[core] avoid using VLA in TListOfTypes

The current code goes through different codepaths depending on the platform.
One of the codepaths relies on a non-standard extension (variable-length arrays)
and the other manually allocates and frees a char array.
Using a std::string, while losing a bit of performance on Linux/Mac,
simplifies and unifies the codepaths and avoids using non-standard C++
(which in turns enable building with -Werror).
The perf hit is most likely negligible, especially given the use of dynamic_cast
in the same block.

Author: silverweed

core/base/src/TListOfTypes.cxx

@@ -63,28 +63,19 @@ static bool NameExistsElsewhere(const char* name){
       // We have a scope
       const auto enName = lastPos + 1;
       const auto scopeNameSize = ((Long64_t)lastPos - (Long64_t)name) / sizeof(decltype(*lastPos)) - 1;
-#ifdef R__WIN32
-      char *scopeName = new char[scopeNameSize + 1];
-#else
-      char scopeName[scopeNameSize + 1]; // on the stack, +1 for the terminating character '\0'
-#endif
-      strncpy(scopeName, name, scopeNameSize);
-      scopeName[scopeNameSize] = '\0';
+      std::string scopeName{name, scopeNameSize};
       // We have now an enum name and a scope name
       // We look first in the classes
-      if(auto scope = dynamic_cast<TClass*>(gROOT->GetListOfClasses()->FindObject(scopeName))){
+      if (auto scope = dynamic_cast<TClass *>(gROOT->GetListOfClasses()->FindObject(scopeName.c_str()))) {
          theEnum = ((TListOfEnums*)scope->GetListOfEnums(false))->THashList::FindObject(enName);
       }
       // And then if not found in the protoclasses
       if (!theEnum){
-         if (auto scope = TClassTable::GetProtoNorm(scopeName)){
+         if (auto scope = TClassTable::GetProtoNorm(scopeName.c_str())) {
             if (auto listOfEnums = (TListOfEnums*)scope->GetListOfEnums())
                theEnum = listOfEnums->THashList::FindObject(enName);
          }
       }
-#ifdef R__WIN32
-      delete [] scopeName;
-#endif
    } else { // Here we look in the global scope
       theEnum = ((TListOfEnums*)gROOT->GetListOfEnums())->THashList::FindObject(name);
    }

core/meta/src/TEnum.cxx

@@ -296,35 +296,25 @@ TEnum *TEnum::GetEnum(const char *enumName, ESearchAction sa)
 
    if (lastPos != enumName) {
       // We have a scope
-      // All of this C gymnastic is to avoid allocations on the heap (see TClingLookupHelper__ExistingTypeCheck)
       const auto enName = lastPos;
       const auto scopeNameSize = ((Long64_t)lastPos - (Long64_t)enumName) / sizeof(decltype(*lastPos)) - 2;
-#ifdef R__WIN32
-      char *scopeName = new char[scopeNameSize + 1];
-#else
-      char scopeName[scopeNameSize + 1]; // on the stack, +1 for the terminating character '\0'
-#endif
-      strncpy(scopeName, enumName, scopeNameSize);
-      scopeName[scopeNameSize] = '\0';
+      std::string scopeName{enumName, scopeNameSize};
       // Three levels of search
-      theEnum = searchEnum(scopeName, enName, kNone);
+      theEnum = searchEnum(scopeName.c_str(), enName, kNone);
       if (!theEnum && (sa & kAutoload)) {
-         const auto libsLoaded = gInterpreter->AutoLoad(scopeName);
+         const auto libsLoaded = gInterpreter->AutoLoad(scopeName.c_str());
          // It could be an enum in a scope which is not selected
          if (libsLoaded == 0){
             gInterpreter->AutoLoad(enumName);
          }
-         theEnum = searchEnum(scopeName, enName, kAutoload);
+         theEnum = searchEnum(scopeName.c_str(), enName, kAutoload);
       }
       if (!theEnum && (sa & kALoadAndInterpLookup)) {
          if (gDebug > 0) {
             printf("TEnum::GetEnum: Header Parsing - The enumerator %s is not known to the typesystem: an interpreter lookup will be performed. This can imply parsing of headers. This can be avoided selecting the numerator in the linkdef/selection file.\n", enumName);
          }
-         theEnum = searchEnum(scopeName, enName, kALoadAndInterpLookup);
+         theEnum = searchEnum(scopeName.c_str(), enName, kALoadAndInterpLookup);
       }
-#ifdef R__WIN32
-      delete [] scopeName;
-#endif
    } else {
       // We don't have any scope: this is a global enum
       theEnum = findEnumInList(gROOT->GetListOfEnums(), enumName, kNone);

core/metacling/src/TCling.cxx

@@ -960,26 +960,24 @@ bool TClingLookupHelper__ExistingTypeCheck(const std::string &tname,
    if (lastPos != inner)   // Main switch: case 1 - scoped enum, case 2 global enum
    {
       // We have a scope
-      // All of this C gymnastic is to avoid allocations on the heap
       const auto enName = lastPos;
       const auto scopeNameSize = ((Long64_t)lastPos - (Long64_t)inner) / sizeof(decltype(*lastPos)) - 2;
-      char *scopeName = new char[scopeNameSize + 1];
-      strncpy(scopeName, inner, scopeNameSize);
-      scopeName[scopeNameSize] = '\0';
+      std::string scopeName{inner, scopeNameSize};
       // Check if the scope is in the list of classes
-      if (auto scope = static_cast<TClass *>(gROOT->GetListOfClasses()->FindObject(scopeName))) {
+      if (auto scope = static_cast<TClass *>(gROOT->GetListOfClasses()->FindObject(scopeName.c_str()))) {
          auto enumTable = dynamic_cast<const THashList *>(scope->GetListOfEnums(false));
-         if (enumTable && enumTable->THashList::FindObject(enName)) { delete [] scopeName; return true; }
+         if (enumTable && enumTable->THashList::FindObject(enName))
+            return true;
       }
       // It may still be in one of the loaded protoclasses
-      else if (auto scope = static_cast<TProtoClass *>(gClassTable->GetProtoNorm(scopeName))) {
+      else if (auto scope = static_cast<TProtoClass *>(gClassTable->GetProtoNorm(scopeName.c_str()))) {
          auto listOfEnums = scope->GetListOfEnums();
          if (listOfEnums) { // it could be null: no enumerators in the protoclass
             auto enumTable = dynamic_cast<const THashList *>(listOfEnums);
-            if (enumTable && enumTable->THashList::FindObject(enName)) { delete [] scopeName; return true; }
+            if (enumTable && enumTable->THashList::FindObject(enName))
+               return true;
          }
       }
-      delete [] scopeName;
    } else
    {
       // We don't have any scope: this could only be a global enum