Fix Issue #6443: spurrious auto-parsing due to @ class, pair, __pair_base and enums #6445
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ In order to access the name of a class within the ROOT type system, the method T | |
| #include "TDataMember.h" | ||
| #include "TDataType.h" | ||
| #include "TDatime.h" | ||
| #include "TEnum.h" | ||
| #include "TError.h" | ||
| #include "TExMap.h" | ||
| #include "TFunctionTemplate.h" | ||
|
|
@@ -170,6 +171,12 @@ namespace { | |
|
|
||
| std::atomic<Int_t> TClass::fgClassCount; | ||
|
|
||
| static bool IsFromRootCling() { | ||
| // rootcling also uses TCling for generating the dictionary ROOT files. | ||
| const static bool foundSymbol = dlsym(RTLD_DEFAULT, "usedToIdentifyRootClingByDlSym"); | ||
| return foundSymbol; | ||
| } | ||
|
|
||
| // Implementation of the TDeclNameRegistry | ||
|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
@@ -1108,7 +1115,7 @@ TClass::TClass(const char *name, Bool_t silent) : | |
| ::Fatal("TClass::TClass", "gInterpreter not initialized"); | ||
|
|
||
| gInterpreter->SetClassInfo(this); // sets fClassInfo pointer | ||
| if (!silent && !fClassInfo && !TClassEdit::IsArtificial(name)) | ||
| ::Warning("TClass::TClass", "no dictionary for class %s is available", name); | ||
| ResetBit(kLoading); | ||
|
|
||
|
|
@@ -1339,7 +1346,11 @@ void TClass::Init(const char *name, Version_t cversion, | |
| return; | ||
| } | ||
| // Always strip the default STL template arguments (from any template argument or the class name) | ||
| if (TClassEdit::IsArtificial(name)) | ||
| fName = name; // We can assume that the artificial class name is already normalized. | ||
| else | ||
| fName = TClassEdit::ShortType(name, TClassEdit::kDropStlDefault).c_str(); | ||
|
|
||
| fClassVersion = cversion; | ||
| fDeclFileName = dfil ? dfil : ""; | ||
| fImplFileName = ifil ? ifil : ""; | ||
|
|
@@ -1444,7 +1455,7 @@ void TClass::Init(const char *name, Version_t cversion, | |
| // We need to check if the class it is not fwd declared for the cases where we | ||
| // created a TClass directly in the kForwardDeclared state. Indeed in those cases | ||
| // fClassInfo will always be nullptr. | ||
| if (fState!=kForwardDeclared && !fClassInfo && !TClassEdit::IsArtificial(fName)) { | ||
|
|
||
| if (fState == kHasTClassInit) { | ||
| // If the TClass is being generated from a ROOT dictionary, | ||
|
|
@@ -1482,7 +1493,7 @@ void TClass::Init(const char *name, Version_t cversion, | |
| } | ||
| } | ||
| } | ||
| if (!silent && (!fClassInfo && !fCanLoadClassInfo) && !isStl && !TClassEdit::IsArtificial(fName) && | ||
| !TClassEdit::IsInterpreterDetail(fName.Data()) ) { | ||
| if (fState == kHasTClassInit) { | ||
| if (fImplFileLine == -1 && fClassVersion == 0) { | ||
|
|
@@ -1599,7 +1610,7 @@ void TClass::Init(const char *name, Version_t cversion, | |
| fStreamer = TVirtualStreamerInfo::Factory()->GenEmulatedClassStreamer( GetName(), silent ); | ||
| } | ||
| } | ||
| } else if (TClassEdit::IsStdPair(GetName())) { | ||
| // std::pairs have implicit conversions | ||
| GetSchemaRules(kTRUE); | ||
| } | ||
|
|
@@ -2014,7 +2025,7 @@ void TClass::BuildRealData(void* pointer, Bool_t isTransient) | |
| // and those for which the user explicitly requested a dictionary. | ||
| if (!isTransient && GetState() != kHasTClassInit | ||
| && TClassEdit::IsStdClass(GetName()) | ||
| && !TClassEdit::IsStdPair(GetName())) { | ||
| Error("BuildRealData", "Inspection for %s not supported!", GetName()); | ||
| } | ||
|
|
||
|
|
@@ -2304,14 +2315,6 @@ Bool_t TClass::CanSplit() const | |
| if (!valueClass->CanSplit()) { This->fCanSplit = 0; return kFALSE; } | ||
| if (valueClass->GetCollectionProxy() != 0) { This->fCanSplit = 0; return kFALSE; } | ||
|
|
||
| This->fCanSplit = 1; | ||
| return kTRUE; | ||
|
|
||
|
|
@@ -2970,6 +2973,15 @@ TClass *TClass::GetClass(const char *name, Bool_t load, Bool_t silent) | |
| load = kTRUE; | ||
| } | ||
|
|
||
| if (TClassEdit::IsArtificial(name)) { | ||
| // If there is a @ symbol (followed by a version number) then this is a synthetic class name created | ||
| // from an already normalized name for the purpose of supporting schema evolution. | ||
| // There is no dictionary or interpreter information about this kind of class, the only | ||
| // (undesirable) side-effect of doing the search would be a waste of CPU time and potential | ||
| // auto-loading or auto-parsing based on the scope of the name. | ||
| return cl; | ||
| } | ||
|
|
||
| // To avoid spurious auto parsing, let's check if the name as-is is | ||
| // known in the TClassTable. | ||
| DictFuncPtr_t dict = TClassTable::GetDictNorm(name); | ||
|
|
@@ -3030,15 +3042,27 @@ TClass *TClass::GetClass(const char *name, Bool_t load, Bool_t silent) | |
| // altcl->GetName(), name, normalizedName.c_str()); | ||
| // } | ||
|
|
||
| // We want to avoid auto-parsing due to intentionally missing dictionary for std::pair. | ||
| // However, we don't need this special treatement in rootcling (there is no auto-parsing) | ||
| // and we want to make that the TClass for the pair goes through the regular creation | ||
| // mechanism (i.e. in rootcling they should be in kInterpreted state and never in | ||
| // kEmulated state) so that they have proper interpreter (ClassInfo) information which | ||
| // will be used to create the TProtoClass (if one is requested for the pair). | ||
| const bool ispair = TClassEdit::IsStdPair(normalizedName) && !IsFromRootCling(); | ||
| const bool ispairbase = TClassEdit::IsStdPairBase(normalizedName) && !IsFromRootCling(); | ||
|
|
||
| TClass *loadedcl = 0; | ||
| if (checkTable) { | ||
| loadedcl = LoadClassDefault(normalizedName.c_str(),silent); | ||
| } else { | ||
| if (gInterpreter->AutoLoad(normalizedName.c_str(),kTRUE)) { | ||
| loadedcl = LoadClassDefault(normalizedName.c_str(),silent); | ||
| } | ||
| auto e = TEnum::GetEnum(normalizedName.c_str(), TEnum::kNone); | ||
| if (e) | ||
| return nullptr; | ||
| // Maybe this was a typedef: let's try to see if this is the case | ||
| if (!loadedcl && !ispair && !ispairbase) { | ||
| if (TDataType* theDataType = gROOT->GetType(normalizedName.c_str())){ | ||
| // We have a typedef: we get the name of the underlying type | ||
| auto underlyingTypeName = theDataType->GetTypeName(); | ||
|
|
@@ -3061,13 +3085,17 @@ TClass *TClass::GetClass(const char *name, Bool_t load, Bool_t silent) | |
| // TClass if we have one. | ||
| if (cl) return cl; | ||
|
|
||
| if (ispair) { | ||
| auto pairinfo = TVirtualStreamerInfo::Factory()->GenerateInfoForPair(normalizedName); | ||
| return pairinfo ? pairinfo->GetClass() : nullptr; | ||
|
|
||
| } else if (TClassEdit::IsSTLCont( normalizedName.c_str() )) { | ||
|
|
||
| return gInterpreter->GenerateTClass(normalizedName.c_str(), kTRUE, silent); | ||
| } | ||
|
|
||
| // Check the interpreter only after autoparsing the template if any. | ||
| if (!ispairbase) { | ||
| std::string::size_type posLess = normalizedName.find('<'); | ||
| if (posLess != std::string::npos) { | ||
| gCling->AutoParse(normalizedName.substr(0, posLess).c_str()); | ||
|
|
@@ -3632,9 +3660,7 @@ TList *TClass::GetListOfEnums(Bool_t requestListLoading /* = kTRUE */) | |
| return fEnums.load(); | ||
| } | ||
|
|
||
| if (IsFromRootCling()) // rootcling is single thread (this save some space in the rootpcm). | ||
| fEnums = new TListOfEnums(this); | ||
| else | ||
| fEnums = new TListOfEnumsWithLock(this); | ||
|
|
@@ -3997,7 +4023,7 @@ void TClass::GetMissingDictionaries(THashTable& result, bool recurse) | |
|
|
||
| THashTable visited; | ||
|
|
||
| if (TClassEdit::IsStdPair(fName)) { | ||
| GetMissingDictionariesForPairElements(result, visited, recurse); | ||
| return; | ||
| } | ||
|
|
@@ -6339,7 +6365,7 @@ UInt_t TClass::GetCheckSum(ECheckSum code, Bool_t &isvalid) const | |
| // otherwise, on some STL implementations, it can happen that pair has | ||
| // base classes which are an internal implementation detail. | ||
| TList *tlb = ((TClass*)this)->GetListOfBases(); | ||
| if (tlb && !GetCollectionProxy() && !TClassEdit::IsStdPair(GetName())) { | ||
| // Loop over bases if not a proxied collection or a pair | ||
|
|
||
| TIter nextBase(tlb); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -261,7 +261,7 @@ TEnum *TListOfEnums::GetObject(const char *name) const | |
| void TListOfEnums::UnmapObject(TObject *obj) | ||
| { | ||
| TEnum *e = dynamic_cast<TEnum *>(obj); | ||
| if (e && e->GetDeclId()) { | ||
| fIds->Remove((Long64_t)e->GetDeclId()); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -230,8 +230,8 @@ Bool_t TSchemaRuleSet::HasRuleWithSourceClass( const TString &source ) const | |
| } | ||
| } | ||
| } | ||
| } else if (TClassEdit::IsStdPair(fClass->GetName())) { | ||
| if (TClassEdit::IsStdPair(source)) { | ||
| // std::pair can be converted into each other if both its parameter can be converted into | ||
| // each other. | ||
| TClass *src = TClass::GetClass(source); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4031,7 +4031,31 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp | |
|
|
||
| const char *classname = name; | ||
|
|
||
| // RAII to suspend and restore auto-loading and auto-parsing based on some external conditions. | ||
| class MaybeSuspendAutoLoadParse { | ||
| int fStoreAutoLoad = 0; | ||
| int fStoreAutoParse = 0; | ||
| bool fSuspendedAutoParse = false; | ||
| public: | ||
| MaybeSuspendAutoLoadParse(int autoload) { | ||
| fStoreAutoLoad = ((TCling*)gCling)->SetClassAutoLoading(autoload); | ||
| } | ||
|
|
||
| void SuspendAutoParsing() { | ||
| fSuspendedAutoParse = true; | ||
| fStoreAutoParse = ((TCling*)gCling)->SetSuspendAutoParsing(true); | ||
| } | ||
|
|
||
| ~MaybeSuspendAutoLoadParse() { | ||
| if (fSuspendedAutoParse) | ||
| ((TCling*)gCling)->SetSuspendAutoParsing(fStoreAutoParse); | ||
| ((TCling*)gCling)->SetClassAutoLoading(fStoreAutoLoad); | ||
| } | ||
| }; | ||
|
|
||
| MaybeSuspendAutoLoadParse autoLoadParseRAII( autoload ); | ||
| if (TClassEdit::IsStdPair(classname) || TClassEdit::IsStdPairBase(classname)) | ||
| autoLoadParseRAII.SuspendAutoParsing(); | ||
|
|
||
| // First we want to check whether the decl exist, but _without_ | ||
| // generating any template instantiation. However, the lookup | ||
|
|
@@ -4077,14 +4101,12 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp | |
| // findscope. | ||
| if (ROOT::TMetaUtils::IsSTLCont(*tmpltDecl)) { | ||
| // For STL Collection we return kUnknown. | ||
| return kUnknown; | ||
| } | ||
| } | ||
| } | ||
| TClingClassInfo tci(GetInterpreterImpl(), *type); | ||
| if (!tci.IsValid()) { | ||
| return kUnknown; | ||
| } | ||
| auto propertiesMask = isClassOrNamespaceOnly ? kIsClass | kIsStruct | kIsNamespace : | ||
|
|
@@ -4111,19 +4133,16 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp | |
| // , hasClassDefInline); | ||
|
|
||
| // We are now sure that the entry is not in fact an autoload entry. | ||
| if (hasClassDefInline) | ||
| return kWithClassDefInline; | ||
| else | ||
| return kKnown; | ||
| } else { | ||
| // We are now sure that the entry is not in fact an autoload entry. | ||
| return kUnknown; | ||
| } | ||
| } | ||
|
|
||
| if (decl) | ||
| return kKnown; | ||
| else | ||
|
|
@@ -4135,7 +4154,6 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp | |
| TClingTypedefInfo t(fInterpreter, name); | ||
| if (t.IsValid() && !(t.Property() & kIsFundamental)) { | ||
| delete[] classname; | ||
| return kTRUE; | ||
| } | ||
| */ | ||
|
|
@@ -4146,7 +4164,6 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp | |
| // decl = lh.findScope(buf); | ||
| // } | ||
|
|
||
| // return (decl); | ||
| } | ||
|
|
||
|
|
@@ -4364,6 +4381,7 @@ TClass *TCling::GenerateTClass(const char *classname, Bool_t emulation, Bool_t s | |
| if (TClassEdit::IsSTLCont(classname)) { | ||
| version = TClass::GetClass("TVirtualStreamerInfo")->GetClassVersion(); | ||
| } | ||
| R__LOCKGUARD(gInterpreterMutex); | ||
| TClass *cl = new TClass(classname, version, silent); | ||
| if (emulation) { | ||
| cl->SetBit(TClass::kIsEmulation); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not a bit fan of
== 3and== 4without any comment. See also the change in tree/treeplayer/src/TTreeGeneratorBase.cxxThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In TTreeGeneratorBase.cxx, I left 3 or 4 because the likely wrong one was there with no (obvious negative consequence) and (I should add a comment) thus it might actually be the right one .... i.e. to remove it I would have to try to produce a test and the code is used little enough to not be a priority (to spend the time to write the test).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment would be great! :-)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that's part of the documented contract of TClassEdit::TSplitType :)
But nevertheless I will add a comment.