ConfigurableParam: Support for std::string parameter values

This commit enables support for std::string members in parameter classes:
```
struct Param : ConfigurableParamHelper<Param> {
  std::string name = "defaultString";
};
```
will now work and be modifyable through the offered APIs.

std::string is the only way to represent strings. char* will not work.

Author: sawenzel

Common/SimConfig/doc/ConfigurableParam.md

@@ -86,7 +86,7 @@ Thereafter, the parameter `ParamA` is automatically registered in a parameter re
 
 * Parameter classes may only contain simple members! Currently the following types are supported
     * simple pods (for example `double x; char y;`)
-    * simple char strings (`char *`)
+    * std::string
     * fixed size arrays of pods using the ROOT way to serialize:
        ```c++
        static constexpr int N=3; //!

Common/SimConfig/include/SimConfig/ConfigurableParam.h

@@ -105,9 +105,9 @@ class ConfigurableParam
   static void printAllKeyValuePairs();
 
   // writes a human readable JSON file of all parameters
-  static void writeJSON(std::string filename);
+  static void writeJSON(std::string const& filename);
   // writes a human readable INI file of all parameters
-  static void writeINI(std::string filename);
+  static void writeINI(std::string const& filename);
 
   // can be used instead of using API on concrete child classes
   template <typename T>
@@ -120,7 +120,7 @@ class ConfigurableParam
   }
 
   template <typename T>
-  static void setValue(std::string mainkey, std::string subkey, T x)
+  static void setValue(std::string const& mainkey, std::string const& subkey, T x)
   {
     auto key = mainkey + "." + subkey;
     if (sPtree->get_optional<std::string>(key).is_initialized()) {
@@ -134,7 +134,7 @@ class ConfigurableParam
 
   // specialized for std::string
   // which means that the type will be converted internally
-  static void setValue(std::string key, std::string valuestring)
+  static void setValue(std::string const& key, std::string const& valuestring)
   {
     if (sPtree->get_optional<std::string>(key).is_initialized()) {
       sPtree->put(key, valuestring);
@@ -157,7 +157,7 @@ class ConfigurableParam
   // (certain) key-values
   // propagates changes down to each registered configuration
   // might be useful to get stuff from the command line
-  static void updateFromString(std::string);
+  static void updateFromString(std::string const&);
 
  protected:
   // constructor is doing nothing else but
@@ -166,7 +166,7 @@ class ConfigurableParam
 
   static void initPropertyTree();
   static bool updateThroughStorageMap(std::string, std::string, std::type_info const&, void*);
-  static bool updateThroughStorageMapWithConversion(std::string, std::string);
+  static bool updateThroughStorageMapWithConversion(std::string const&, std::string const&);
 
   virtual ~ConfigurableParam() = default;
 
@@ -177,7 +177,7 @@ class ConfigurableParam
 
   // static map keeping, for each configuration key, its memory location and type
   // (internal use to easily sync updates, this is ok since parameter classes are singletons)
-  static std::map<std::string, std::pair<int, void*>>* sKeyToStorageMap;
+  static std::map<std::string, std::pair<std::type_info const&, void*>>* sKeyToStorageMap;
 
   // keep track of provenance of parameters and values
   static std::map<std::string, ConfigurableParam::EParamProvenance>* sValueProvenanceMap;

Common/SimConfig/include/SimConfig/ConfigurableParamHelper.h

@@ -33,7 +33,7 @@ class _ParamHelper
                                   std::map<std::string, ConfigurableParam::EParamProvenance> const* provmap);
 
   static void fillKeyValuesImpl(std::string mainkey, TClass* cl, void*, boost::property_tree::ptree*,
-                                std::map<std::string, std::pair<int, void*>>*);
+                                std::map<std::string, std::pair<std::type_info const&, void*>>*);
 
   static void printWarning(std::type_info const&);
 

Common/SimConfig/src/ConfigurableParam.cxx

@@ -30,19 +30,19 @@ namespace conf
 
 std::vector<ConfigurableParam*>* ConfigurableParam::sRegisteredParamClasses = nullptr;
 boost::property_tree::ptree* ConfigurableParam::sPtree = nullptr;
-std::map<std::string, std::pair<int, void*>>* ConfigurableParam::sKeyToStorageMap = nullptr;
+std::map<std::string, std::pair<std::type_info const&, void*>>* ConfigurableParam::sKeyToStorageMap = nullptr;
 std::map<std::string, ConfigurableParam::EParamProvenance>* ConfigurableParam::sValueProvenanceMap = nullptr;
 
 bool ConfigurableParam::sIsFullyInitialized = false;
 bool ConfigurableParam::sRegisterMode = true;
 
-void ConfigurableParam::writeINI(std::string filename)
+void ConfigurableParam::writeINI(std::string const& filename)
 {
   initPropertyTree(); // update the boost tree before writing
   boost::property_tree::write_ini(filename, *sPtree);
 }
 
-void ConfigurableParam::writeJSON(std::string filename)
+void ConfigurableParam::writeJSON(std::string const& filename)
 {
   initPropertyTree(); // update the boost tree before writing
   boost::property_tree::write_json(filename, *sPtree);
@@ -58,6 +58,9 @@ void ConfigurableParam::initPropertyTree()
 
 void ConfigurableParam::printAllKeyValuePairs()
 {
+  if (!sIsFullyInitialized) {
+    initialize();
+  }
   std::cout << "####\n";
   for (auto p : *sRegisteredParamClasses) {
     p->printKeyValues(true);
@@ -69,6 +72,9 @@ void ConfigurableParam::printAllKeyValuePairs()
 // ... we need to generalize this ... but ok for demonstration purposes
 void ConfigurableParam::toCCDB(std::string filename)
 {
+  if (!sIsFullyInitialized) {
+    initialize();
+  }
   TFile file(filename.c_str(), "RECREATE");
   for (auto p : *sRegisteredParamClasses) {
     p->serializeTo(&file);
@@ -97,7 +103,7 @@ ConfigurableParam::ConfigurableParam()
     sPtree = new boost::property_tree::ptree;
   }
   if (sKeyToStorageMap == nullptr) {
-    sKeyToStorageMap = new std::map<std::string, std::pair<int, void*>>;
+    sKeyToStorageMap = new std::map<std::string, std::pair<std::type_info const&, void*>>;
   }
   if (sValueProvenanceMap == nullptr) {
     sValueProvenanceMap = new std::map<std::string, ConfigurableParam::EParamProvenance>;
@@ -125,7 +131,7 @@ void ConfigurableParam::printAllRegisteredParamNames()
   }
 }
 
-void ConfigurableParam::updateFromString(std::string configstring)
+void ConfigurableParam::updateFromString(std::string const& configstring)
 {
   if (!sIsFullyInitialized) {
     initialize();
@@ -213,7 +219,7 @@ bool ConfigurableParam::updateThroughStorageMap(std::string mainkey, std::string
   int type = TDataType::GetType(tinfo);
 
   // check that type matches
-  if (iter->second.first != type) {
+  if (iter->second.first != tinfo) {
     LOG(WARN) << "Types do not match; cannot update value";
     return false;
   }
@@ -339,8 +345,21 @@ bool ConvertAndCopy(std::string const& valuestring, void* targetaddr)
   }
   return false;
 }
+// special version for std::string
+template <>
+bool ConvertAndCopy<std::string>(std::string const& valuestring, void* targetaddr)
+{
+  std::string& target = *((std::string*)targetaddr);
+  if (target.compare(valuestring) != 0) {
+    // the targetaddr is a std::string to which we can simply assign
+    // and all the magic will happen internally
+    target = valuestring;
+    return true;
+  }
+  return false;
+}
 
-bool ConfigurableParam::updateThroughStorageMapWithConversion(std::string key, std::string valuestring)
+bool ConfigurableParam::updateThroughStorageMapWithConversion(std::string const& key, std::string const& valuestring)
 {
   // check if key_exists
   auto iter = sKeyToStorageMap->find(key);
@@ -349,10 +368,17 @@ bool ConfigurableParam::updateThroughStorageMapWithConversion(std::string key, s
     return false;
   }
 
+  auto targetaddress = iter->second.second;
+
+  // treat some special cases first:
+  // the type is actually a std::string
+  if (iter->second.first == typeid(std::string)) {
+    return ConvertAndCopy<std::string>(valuestring, targetaddress);
+  }
+
   // the type (aka ROOT::EDataType which the type identification in the map) we need to convert to
-  int targettype = iter->second.first;
+  int targettype = TDataType::GetType(iter->second.first);
 
-  auto targetaddress = iter->second.second;
   switch (targettype) {
     case kChar_t: {
       return ConvertAndCopy<char>(valuestring, targetaddress);

Common/SimConfig/src/ConfigurableParamHelper.cxx

@@ -24,6 +24,11 @@
 
 using namespace o2::conf;
 
+bool isString(TDataMember const& dm)
+{
+  return strcmp(dm.GetTrueTypeName(), "string") == 0;
+}
+
 // a generic looper of data members of a TClass; calling a callback
 // reused in various functions below
 void loopOverMembers(TClass* cl, void* obj,
@@ -33,6 +38,10 @@ void loopOverMembers(TClass* cl, void* obj,
   for (int i = 0; i < memberlist->GetEntries(); ++i) {
     auto dm = (TDataMember*)memberlist->At(i);
 
+    auto isValidComplex = [dm]() {
+      return isString(*dm);
+    };
+
     // filter out static members for now
     if (dm->Property() & kIsStatic) {
       continue;
@@ -41,8 +50,8 @@ void loopOverMembers(TClass* cl, void* obj,
       LOG(WARNING) << "Pointer types not supported in ConfigurableParams";
       continue;
     }
-    if (!dm->IsBasic()) {
-      LOG(WARNING) << "Complex types not supported in ConfigurableParams";
+    if (!dm->IsBasic() && !isValidComplex()) {
+      LOG(WARNING) << "Generic complex types not supported in ConfigurableParams";
       continue;
     }
     const auto dim = dm->GetArrayDim();
@@ -69,15 +78,34 @@ std::string getName(const TDataMember* dm, int index, int size)
   return namestream.str();
 }
 
+const char* asString(TDataMember const& dm, char* pointer)
+{
+  // first check if this is a basic data type, in which case
+  // we let ROOT do the work
+  if (auto dt = dm.GetDataType()) {
+    return dt->AsString(pointer);
+  }
+
+  // if data member is a std::string just return
+  else if (isString(dm)) {
+    return ((std::string*)pointer)->c_str();
+  }
+  // potentially other cases to be added here
+
+  LOG(ERROR) << "COULD NOT REPRESENT AS STRING";
+  return nullptr;
+}
+
 void _ParamHelper::printParametersImpl(std::string mainkey, TClass* cl, void* obj,
                                        std::map<std::string, ConfigurableParam::EParamProvenance> const* provmap)
 {
   auto printMembers = [&mainkey, obj, provmap](const TDataMember* dm, int index, int size) {
     // pointer to object
     auto dt = dm->GetDataType();
-    char* pointer = ((char*)obj) + dm->GetOffset() + index * dt->Size();
+    auto TS = dt ? dt->Size() : 0;
+    char* pointer = ((char*)obj) + dm->GetOffset() + index * TS;
     const auto name = getName(dm, index, size);
-    std::cout << name << " : " << dt->AsString(pointer);
+    std::cout << name << " : " << asString(*dm, pointer);
     if (provmap != nullptr) {
       auto iter = provmap->find(mainkey + "." + name);
       if (iter != provmap->end()) {
@@ -89,19 +117,84 @@ void _ParamHelper::printParametersImpl(std::string mainkey, TClass* cl, void* ob
   loopOverMembers(cl, obj, printMembers);
 }
 
+// a function converting a string representing a type to the type_info
+// because unfortunately typeid(double) != typeid("double")
+// but we can take the TDataType (if it exists) as a hint in order to
+// minimize string comparisons
+std::type_info const& nameToTypeInfo(const char* tname, TDataType const* dt)
+{
+  if (dt) {
+    switch (dt->GetType()) {
+      case kChar_t: {
+        return typeid(char);
+      }
+      case kUChar_t: {
+        return typeid(unsigned char);
+      }
+      case kShort_t: {
+        return typeid(short);
+      }
+      case kUShort_t: {
+        return typeid(unsigned short);
+      }
+      case kInt_t: {
+        return typeid(int);
+      }
+      case kUInt_t: {
+        return typeid(unsigned int);
+      }
+      case kLong_t: {
+        return typeid(long);
+      }
+      case kULong_t: {
+        return typeid(unsigned long);
+      }
+      case kFloat_t: {
+        return typeid(float);
+      }
+      case kDouble_t: {
+        return typeid(double);
+      }
+      case kDouble32_t: {
+        return typeid(double);
+      }
+      case kBool_t: {
+        return typeid(bool);
+      }
+      case kLong64_t: {
+        return typeid(long long);
+      }
+      case kULong64_t: {
+        return typeid(unsigned long long);
+      }
+      default: {
+        break;
+      }
+    }
+  }
+  // if we get here none of the above worked
+  if (strcmp(tname, "string") == 0 || strcmp(tname, "std::string")) {
+    return typeid(std::string);
+  }
+  LOG(ERROR) << "ENCOUNTERED AN UNSUPPORTED TYPE " << tname << "IN A CONFIGURABLE PARAMETER";
+  return typeid("ERROR");
+}
+
 void _ParamHelper::fillKeyValuesImpl(std::string mainkey, TClass* cl, void* obj, boost::property_tree::ptree* tree,
-                                     std::map<std::string, std::pair<int, void*>>* keytostoragemap)
+                                     std::map<std::string, std::pair<std::type_info const&, void*>>* keytostoragemap)
 {
   boost::property_tree::ptree localtree;
   auto fillMap = [obj, &mainkey, &localtree, &keytostoragemap](const TDataMember* dm, int index, int size) {
     const auto name = getName(dm, index, size);
     auto dt = dm->GetDataType();
-    char* pointer = ((char*)obj) + dm->GetOffset() + index * dt->Size();
-    localtree.put(name, dt->AsString(pointer));
+    auto TS = dt ? dt->Size() : 0;
+    char* pointer = ((char*)obj) + dm->GetOffset() + index * TS;
+    localtree.put(name, asString(*dm, pointer));
 
     auto key = mainkey + "." + name;
-    using mapped_t = std::pair<int, void*>;
-    keytostoragemap->insert(std::pair<std::string, mapped_t>(key, mapped_t(dt->GetType(), pointer)));
+    using mapped_t = std::pair<std::type_info const&, void*>;
+    auto& ti = nameToTypeInfo(dm->GetTrueTypeName(), dt);
+    keytostoragemap->insert(std::pair<std::string, mapped_t>(key, mapped_t(ti, pointer)));
   };
   loopOverMembers(cl, obj, fillMap);
   tree->add_child(mainkey, localtree);
@@ -124,16 +217,38 @@ void _ParamHelper::assignmentImpl(std::string mainkey, TClass* cl, void* to, voi
   auto assignifchanged = [to, from, &mainkey, provmap](const TDataMember* dm, int index, int size) {
     const auto name = getName(dm, index, size);
     auto dt = dm->GetDataType();
-    char* pointerto = ((char*)to) + dm->GetOffset() + index * dt->Size();
-    char* pointerfrom = ((char*)from) + dm->GetOffset() + index * dt->Size();
-    if (!isMemblockDifferent(pointerto, pointerfrom, dt->Size())) {
+    auto TS = dt ? dt->Size() : 0;
+    char* pointerto = ((char*)to) + dm->GetOffset() + index * TS;
+    char* pointerfrom = ((char*)from) + dm->GetOffset() + index * TS;
+
+    // lambda to update the provenance
+    auto updateProv = [&mainkey, name, provmap]() {
       auto key = mainkey + "." + name;
       auto iter = provmap->find(key);
       if (iter != provmap->end()) {
         iter->second = ConfigurableParam::EParamProvenance::kCCDB; // TODO: change to "current STATE"??
       } else {
         LOG(WARN) << "KEY " << key << " NOT FOUND WHILE UPDATING PARAMETER PROVENANCE";
       }
+    };
+
+    // TODO: this could dispatch to the same method used in ConfigurableParam::setValue
+    // but will be slower
+
+    // test if a complicated case
+    if (isString(*dm)) {
+      std::string& target = *(std::string*)pointerto;
+      std::string const& origin = *(std::string*)pointerfrom;
+      if (target.compare(origin) != 0) {
+        updateProv();
+        target = origin;
+      }
+      return;
+    }
+
+    //
+    if (!isMemblockDifferent(pointerto, pointerfrom, TS)) {
+      updateProv();
       // actually copy
       std::memcpy(pointerto, pointerfrom, dt->Size());
     }