Improvements based on code review

makortel · May 7, 2024 · c8db8b0 · c8db8b0
1 parent 49db039
commit c8db8b0
Show file tree

Hide file tree

Showing 14 changed files with 50 additions and 100 deletions.
diff --git a/DQMOffline/Trigger/src/EgHLTTrigTools.cc b/DQMOffline/Trigger/src/EgHLTTrigTools.cc
@@ -55,7 +55,7 @@ std::vector<int> trigTools::getMinNrObjsRequiredByFilter(const std::vector<std::
     return retVal;
   }
   for (auto& psetIt : *psetRegistry) {  //loop over every pset for every module ever run
-    const std::map<std::string, edm::Entry>& mapOfPara =
+    const auto& mapOfPara =
         psetIt.second.tbl();  //contains the parameter name and value for all the parameters of the pset
     const auto itToModLabel = mapOfPara.find(mag0);
     if (itToModLabel != mapOfPara.end()) {

diff --git a/FWCore/ParameterSet/interface/Entry.h b/FWCore/ParameterSet/interface/Entry.h
@@ -177,7 +177,7 @@ namespace edm {
 
     friend std::ostream& operator<<(std::ostream& ost, Entry const& entry);
 
-    //empt string view denotes failure to find bounds
+    //empty string view denotes failure to find bounds
     static std::string_view bounds(std::string_view, std::size_t iEndHint);
 
   private:

diff --git a/FWCore/ParameterSet/interface/ParameterDescriptionNode.h b/FWCore/ParameterSet/interface/ParameterDescriptionNode.h
@@ -42,8 +42,8 @@ namespace edm {
     k_double = 'D',
     k_vdouble = 'd',
     k_bool = 'B',
-    k_string = 'Z',
-    k_vstring = 'z',
+    k_stringRaw = 'Z',
+    k_vstringRaw = 'z',
     k_stringHex = 'S',
     k_vstringHex = 's',
     k_EventID = 'E',

diff --git a/FWCore/ParameterSet/interface/ParameterSet.h b/FWCore/ParameterSet/interface/ParameterSet.h
@@ -230,13 +230,13 @@ namespace edm {
 
     std::vector<ParameterSet> popVParameterSet(std::string const& name);
 
-    typedef std::map<std::string, Entry> table;
+    typedef std::map<std::string, Entry, std::less<>> table;
     table const& tbl() const { return tbl_; }
 
-    typedef std::map<std::string, ParameterSetEntry> psettable;
+    typedef std::map<std::string, ParameterSetEntry, std::less<>> psettable;
     psettable const& psetTable() const { return psetTable_; }
 
-    typedef std::map<std::string, VParameterSetEntry> vpsettable;
+    typedef std::map<std::string, VParameterSetEntry, std::less<>> vpsettable;
     vpsettable const& vpsetTable() const { return vpsetTable_; }
 
     ParameterSet* getPSetForUpdate(std::string const& name, bool& isTracked);

diff --git a/FWCore/ParameterSet/interface/types.h b/FWCore/ParameterSet/interface/types.h
@@ -84,11 +84,11 @@ namespace edm {
   bool encode_element(std::string&, std::string const&);
   std::optional<std::string_view> decode_vstring_extent(std::string_view from);
 
-  // String old
+  // String old, kept for backwards compatibility
   bool decode_deprecated(std::string&, std::string_view);
   bool encode_deprecated(std::string&, std::string const&);
 
-  // vString old
+  // vString old, kept for backwards compatibility
   bool decode_deprecated(std::vector<std::string>&, std::string_view);
   bool encode_deprecated(std::string&, std::vector<std::string> const&);
 

diff --git a/FWCore/ParameterSet/src/Entry.cc b/FWCore/ParameterSet/src/Entry.cc
@@ -1222,11 +1222,11 @@ namespace edm {
       }
       case kTvstringRaw: {
         os << "{";
-        std::string start = "'";
-        std::string const between(",'");
+        std::string_view start = "'";
+        std::string_view const between(",'");
         std::vector<std::string> strings = entry.getVString();
-        for (std::vector<std::string>::const_iterator it = strings.begin(), itEnd = strings.end(); it != itEnd; ++it) {
-          os << start << *it << "'";
+        for (auto const& s : strings) {
+          os << start << s << "'";
           start = between;
         }
         os << "}";
@@ -1238,11 +1238,11 @@ namespace edm {
       }
       case kTvstringHex: {
         os << "{";
-        std::string start = "'";
-        std::string const between(",'");
+        std::string_view start = "'";
+        std::string_view const between(",'");
         std::vector<std::string> strings = entry.getVString();
-        for (std::vector<std::string>::const_iterator it = strings.begin(), itEnd = strings.end(); it != itEnd; ++it) {
-          os << start << *it << "'";
+        for (auto const& s : strings) {
+          os << start << s << "'";
           start = between;
         }
         os << "}";

diff --git a/FWCore/ParameterSet/src/ParameterDescriptionNode.cc b/FWCore/ParameterSet/src/ParameterDescriptionNode.cc
@@ -18,6 +18,9 @@
 #define TYPE_TO_NAME(type) \
   case k_##type:           \
     return #type
+#define TYPE_TO_NAME2(e_val, type) \
+  case e_val:                      \
+    return #type
 
 namespace edm {
 
@@ -40,8 +43,8 @@ namespace edm {
   TYPE_TO_ENUM(double, k_double)
   TYPE_TO_ENUM(std::vector<double>, k_vdouble)
   TYPE_TO_ENUM(bool, k_bool)
-  TYPE_TO_ENUM(std::string, k_string)
-  TYPE_TO_ENUM(std::vector<std::string>, k_vstring)
+  TYPE_TO_ENUM(std::string, k_stringRaw)
+  TYPE_TO_ENUM(std::vector<std::string>, k_vstringRaw)
   TYPE_TO_ENUM(EventID, k_EventID)
   TYPE_TO_ENUM(std::vector<EventID>, k_VEventID)
   TYPE_TO_ENUM(LuminosityBlockID, k_LuminosityBlockID)
@@ -74,8 +77,8 @@ namespace edm {
       TYPE_TO_NAME(double);
       TYPE_TO_NAME(vdouble);
       TYPE_TO_NAME(bool);
-      TYPE_TO_NAME(string);
-      TYPE_TO_NAME(vstring);
+      TYPE_TO_NAME2(k_stringRaw, string);
+      TYPE_TO_NAME2(k_vstringRaw, vstring);
       TYPE_TO_NAME(EventID);
       TYPE_TO_NAME(VEventID);
       TYPE_TO_NAME(LuminosityBlockID);

diff --git a/FWCore/ParameterSet/src/ParameterSet.cc b/FWCore/ParameterSet/src/ParameterSet.cc
@@ -605,7 +605,7 @@ namespace edm {
       if (q == remaining.npos)
         return false;
       // form name unique to this ParameterSet
-      std::string name = std::string(remaining.substr(0, q));
+      auto name = remaining.substr(0, q);
       if (tbl_.find(name) != tbl_.end())
         return false;
 
@@ -614,12 +614,12 @@ namespace edm {
       // entries are generically of the form tracked-type-rep
       if (rep[1] == 'Q') {
         ParameterSetEntry psetEntry(rep);
-        if (!psetTable_.insert(std::make_pair(name, psetEntry)).second) {
+        if (!psetTable_.emplace(name, psetEntry).second) {
           return false;
         }
       } else if (rep[1] == 'q') {
         VParameterSetEntry vpsetEntry(rep);
-        if (!vpsetTable_.insert(std::make_pair(name, vpsetEntry)).second) {
+        if (!vpsetTable_.emplace(name, vpsetEntry).second) {
           return false;
         }
       } else {
@@ -632,19 +632,19 @@ namespace edm {
         if (not remaining.empty() and remaining.front() != ';') {
           return false;
         }
-        Entry value(name, bounds);
+        Entry value(std::string(name), bounds);
         if (rep[1] == 'P') {
           ParameterSetEntry psetEntry(value.getPSet(), value.isTracked());
-          if (!psetTable_.insert(std::make_pair(name, psetEntry)).second) {
+          if (!psetTable_.emplace(name, psetEntry).second) {
             return false;
           }
         } else if (rep[1] == 'p') {
           VParameterSetEntry vpsetEntry(value.getVPSet(), value.isTracked());
-          if (!vpsetTable_.insert(std::make_pair(name, vpsetEntry)).second) {
+          if (!vpsetTable_.emplace(name, vpsetEntry).second) {
             return false;
           }
         } else {
-          if (!tbl_.insert(std::make_pair(name, value)).second) {
+          if (!tbl_.emplace(name, value).second) {
             return false;
           }
         }

diff --git a/FWCore/ParameterSet/src/ParameterSetConverter.cc b/FWCore/ParameterSet/src/ParameterSetConverter.cc
@@ -160,19 +160,19 @@ namespace edm {
       for (auto const& k : parameterSets_) {
         std::vector<std::string_view> pieces;
         split(std::back_inserter(pieces), k.first, '<', ';', '>');
-        for (std::vector<std::string_view>::iterator i = pieces.begin(), e = pieces.end(); i != e; ++i) {
-          std::string_view removeName = i->substr(i->find('+'));
+        for (auto const& i : pieces) {
+          std::string_view removeName = i.substr(i.find('+'));
           if (removeName.size() >= 4) {
             if (removeName[1] == 'P') {
               std::string psetString(removeName.begin() + 3, removeName.end() - 1);
-              parameterSets_.push_back(std::make_pair(psetString, ParameterSetID()));
+              parameterSets_.emplace_back(std::move(psetString), ParameterSetID());
               doItAgain = true;
             } else if (removeName[1] == 'p') {
-              std::string pvec = std::string(removeName.begin() + 3, removeName.end() - 1);
+              std::string_view pvec = removeName.substr(3, removeName.size() - 4);
               std::vector<std::string_view> temp;
               split(std::back_inserter(temp), pvec, '{', ',', '}');
-              for (std::vector<std::string_view>::const_iterator j = temp.begin(), f = temp.end(); j != f; ++j) {
-                parameterSets_.push_back(std::make_pair(std::string(*j), ParameterSetID()));
+              for (auto const& j : temp) {
+                parameterSets_.emplace_back(j, ParameterSetID());
               }
               doItAgain = true;
             }

diff --git a/FWCore/ParameterSet/src/VParameterSetEntry.cc b/FWCore/ParameterSet/src/VParameterSetEntry.cc
@@ -12,11 +12,7 @@ namespace edm {
   VParameterSetEntry::VParameterSetEntry() : tracked_(false), theVPSet_(), theIDs_() {}
 
   VParameterSetEntry::VParameterSetEntry(std::vector<ParameterSet> const& vpset, bool isTracked)
-      : tracked_(isTracked), theVPSet_(new std::vector<ParameterSet>), theIDs_() {
-    for (std::vector<ParameterSet>::const_iterator i = vpset.begin(), e = vpset.end(); i != e; ++i) {
-      theVPSet_->push_back(*i);
-    }
-  }
+      : tracked_(isTracked), theVPSet_(new std::vector<ParameterSet>(vpset)), theIDs_() {}
 
   VParameterSetEntry::VParameterSetEntry(std::string_view rep)
       : tracked_(rep[0] == '+'), theVPSet_(), theIDs_(new std::vector<ParameterSetID>) {
@@ -26,8 +22,8 @@ namespace edm {
     std::string_view bracketedRepr = rep.substr(2);
     split(std::back_inserter(temp), bracketedRepr, '{', ',', '}');
     theIDs_->reserve(temp.size());
-    for (std::vector<std::string_view>::const_iterator i = temp.begin(), e = temp.end(); i != e; ++i) {
-      theIDs_->push_back(ParameterSetID(std::string(*i)));
+    for (auto const& id : temp) {
+      theIDs_->emplace_back(std::string(id));
     }
   }
 

diff --git a/FWCore/ParameterSet/src/split.h b/FWCore/ParameterSet/src/split.h
@@ -118,15 +118,9 @@ bool edm::split(std::string_view s, char first, char sep, char last, FUNC f) {
     return false;
 
   // invariant:  we've found all items in [b..boi)
-  for (str_c_iter  //boi = std::find_if(b, e, is_not_a(sep))
-           boi = contextual_find_not(b, e, first, sep, last),
-           eoi;
-       boi != e
-       //; boi = std::find_if(eoi, e, is_not_a(sep))
-       ;
+  for (str_c_iter boi = contextual_find_not(b, e, first, sep, last), eoi; boi != e;
        boi = contextual_find_not(eoi, e, first, sep, last)) {
     // find end of current item:
-    //eoi = std::find_if(boi, e, is_a(sep));
     eoi = contextual_find(boi, e, first, sep, last);
 
     // copy the item formed from characters in [boi..eoi):

diff --git a/FWCore/ParameterSet/src/types.cc b/FWCore/ParameterSet/src/types.cc
@@ -1215,7 +1215,6 @@ std::optional<std::string_view> edm::decode_pset_extent(std::string_view from) {
 // ----------------------------------------------------------------------
 // vPSet
 // ----------------------------------------------------------------------
-#include <iostream>
 bool edm::decode(std::vector<ParameterSet>& to, std::string_view from) {
   to.clear();
   if (from.size() < 2) {
@@ -1273,7 +1272,6 @@ bool edm::encode(std::string& to, std::vector<ParameterSet> const& from) {
 
 // ----------------------------------------------------------------------
 
-#include <iostream>
 std::optional<std::string_view> edm::decode_vpset_extent(std::string_view from) {
   if (from.size() < 2) {
     return {};

diff --git a/FWCore/ParameterSet/test/parameterSetDescription_t.cc b/FWCore/ParameterSet/test/parameterSetDescription_t.cc
@@ -1541,7 +1541,7 @@ int main(int, char**) try {
   pset.addUntrackedParameter<std::string>("svalue", g);
   assert(par != 0);
   assert(par->label() == std::string("svalue"));
-  assert(par->type() == edm::k_string);
+  assert(par->type() == edm::k_stringRaw);
   assert(par->isTracked() == false);
   assert(edm::parameterTypeEnumToString(par->type()) == std::string("string"));
 
@@ -1605,7 +1605,7 @@ int main(int, char**) try {
   std::vector<std::string> v6;
   par = psetDesc.add<std::vector<std::string>>("v6", v6);
   pset.addParameter<std::vector<std::string>>("v6", v6);
-  assert(par->type() == edm::k_vstring);
+  assert(par->type() == edm::k_vstringRaw);
   assert(edm::parameterTypeEnumToString(par->type()) == std::string("vstring"));
 
   std::vector<edm::EventID> v7;