From b770487ab2f6de1d22c23797098f89de3964f0bf Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 4 Sep 2019 11:07:01 -0500 Subject: [PATCH 01/12] Correct the TClass state when created from a ClassInfo --- core/meta/src/TClass.cxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/meta/src/TClass.cxx b/core/meta/src/TClass.cxx index 3c140824d066c..b030ee1ba70c8 100644 --- a/core/meta/src/TClass.cxx +++ b/core/meta/src/TClass.cxx @@ -1406,6 +1406,8 @@ void TClass::Init(const char *name, Version_t cversion, fClassInfo = gInterpreter->ClassInfo_Factory(givenInfo); fCanLoadClassInfo = false; // avoids calls to LoadClassInfo() if info is already loaded + if (fState <= kEmulated) + fState = kInterpreted; } // We need to check if the class it is not fwd declared for the cases where we @@ -6053,6 +6055,8 @@ void TClass::SetUnloaded() GetName(),(int)fState); } + InsertTClassInRegistryRAII insertRAII(fState,fName,fNoInfoOrEmuOrFwdDeclNameRegistry); + // Make sure SetClassInfo, re-calculated the state. fState = kForwardDeclared; From 1d98c55cd6a3e34c37a3b1a4563bf8effd09c08e Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 4 Sep 2019 11:10:21 -0500 Subject: [PATCH 02/12] Move declaration next to use --- core/metacling/src/TCling.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index f68e9ee930f20..4fc410943890e 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6485,7 +6485,6 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) // Ignore declaration within a function. return; } - clang::QualType type( td->getTypeForDecl(), 0 ); auto declName=ND->getNameAsString(); if (!TClass::HasNoInfoOrEmuOrFwdDeclaredDecl(declName.c_str())){ @@ -6493,6 +6492,7 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) return; } + clang::QualType type( td->getTypeForDecl(), 0 ); ROOT::TMetaUtils::GetNormalizedName(name, type, *fInterpreter, *fNormalizedCtxt); } else { name = ND->getNameAsString(); From 6f65ddc11621584034e0514ccef0ae4f1b7989b3 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 4 Sep 2019 11:10:58 -0500 Subject: [PATCH 03/12] Add comment explanation the choice of name used in recording. --- core/metacling/src/TCling.cxx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 4fc410943890e..92ae5e8ab4160 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6487,8 +6487,13 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) } auto declName=ND->getNameAsString(); + // Check if we have registered the unqualified name into the list + // of TClass that are in kNoInfo, kEmulated or kFwdDeclaredState. + // Since this is used as heureutistic to avoid spurrious calls to GetNormalizedName + // the unqualified name is sufficient (and the fully qualified name might be + // 'wrong' if there is difference in spelling in the template paramters (for example) if (!TClass::HasNoInfoOrEmuOrFwdDeclaredDecl(declName.c_str())){ -// printf ("Impossible to find a TClassEntry in kNoInfo or kEmulated the decl of which would be called %s. Skip w/o building the normalized name.\n",declName ); + // fprintf (stderr,"WARNING: Impossible to find a TClassEntry in kNoInfo or kEmulated the decl of which would be called %s. Skip w/o building the normalized name.\n",declName.c_str() ); return; } From bcf2864f8c3a4add39545aaf1ff6c694946e739f Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 4 Sep 2019 11:11:21 -0500 Subject: [PATCH 04/12] Use fully qualified name as expected further --- core/metacling/src/TCling.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 92ae5e8ab4160..d2979ffc5cc37 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6500,7 +6500,7 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) clang::QualType type( td->getTypeForDecl(), 0 ); ROOT::TMetaUtils::GetNormalizedName(name, type, *fInterpreter, *fNormalizedCtxt); } else { - name = ND->getNameAsString(); + name = ND->getQualifiedNameAsString(); } // Supposedly we are being called while something is being From ada61666c1b0f4d685b0ad173d13084a0284d75c Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 4 Sep 2019 11:30:25 -0500 Subject: [PATCH 05/12] Don't record an invalid ClassInfo --- core/meta/src/TClass.cxx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/meta/src/TClass.cxx b/core/meta/src/TClass.cxx index b030ee1ba70c8..a2b22d0acc1f0 100644 --- a/core/meta/src/TClass.cxx +++ b/core/meta/src/TClass.cxx @@ -1404,10 +1404,12 @@ void TClass::Init(const char *name, Version_t cversion, } } - fClassInfo = gInterpreter->ClassInfo_Factory(givenInfo); - fCanLoadClassInfo = false; // avoids calls to LoadClassInfo() if info is already loaded - if (fState <= kEmulated) - fState = kInterpreted; + if (!invalid) { + fClassInfo = gInterpreter->ClassInfo_Factory(givenInfo); + fCanLoadClassInfo = false; // avoids calls to LoadClassInfo() if info is already loaded + if (fState <= kEmulated) + fState = kInterpreted; + } } // We need to check if the class it is not fwd declared for the cases where we From 89a85847123ed43de33abf241d4ef74af2db0ff3 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 4 Sep 2019 18:36:56 -0500 Subject: [PATCH 06/12] In UpdateClassInfoWithDecl, avoid lookup when not needed. Also improve error recovery (don't assign/keep an invalid ClassInfo). We avoid the lookup by doing it only when we have a class with an opaque typedef (Double32_t) ... except that we don't know how to find them yet (at least not in a performant way). --- core/metacling/src/TCling.cxx | 80 ++++++++++++++++++++++------------- core/metacling/src/TCling.h | 2 + 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index d2979ffc5cc37..c3c4cfc4621fe 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6465,9 +6465,52 @@ Bool_t TCling::IsAutoLoadNamespaceCandidate(const clang::NamespaceDecl* nsDecl) return fNSFromRootmaps.count(nsDecl) != 0; } + //////////////////////////////////////////////////////////////////////////////// -/// Internal function. Inform a TClass about its new TagDecl or NamespaceDecl. +/// Internal function. Actually do the update of the ClassInfo when seeing +// new TagDecl or NamespaceDecl. +void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alias) { + + TClingClassInfo *cci = ((TClingClassInfo *)cl->fClassInfo); + if (cci) { + // If we only had a forward declaration then update the + // TClingClassInfo with the definition if we have it now. + const TagDecl *tdOld = llvm::dyn_cast_or_null(cci->GetDecl()); + if (!tdOld || (tdDef && tdDef != tdOld)) { + cl->ResetCaches(); + TClass::RemoveClassDeclId(cci->GetDeclId()); + if (tdDef) { + // It's a tag decl, not a namespace decl. + cci->Init(*cci->GetType()); + TClass::AddClassToDeclIdMap(cci->GetDeclId(), cl); + } + } + } else if (!cl->TestBit(TClass::kLoading) && !cl->fHasRootPcmInfo) { + cl->ResetCaches(); + // yes, this is almost a waste of time, but we do need to lookup + // the 'type' corresponding to the TClass anyway in order to + // preserve the opaque typedefs (Double32_t) + if (!alias) + cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, tdDef); + else + cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, cl->GetName()); + if (((TClingClassInfo *)cl->fClassInfo)->IsValid()) { + // We now need to update the state and bits. + if (cl->fState != TClass::kHasTClassInit) { + // if (!cl->fClassInfo->IsValid()) cl->fState = TClass::kForwardDeclared; else + cl->fState = TClass::kInterpreted; + cl->ResetBit(TClass::kIsEmulation); + } + TClass::AddClassToDeclIdMap(((TClingClassInfo *)(cl->fClassInfo))->GetDeclId(), cl); + } else { + delete ((TClingClassInfo *)cl->fClassInfo); + cl->fClassInfo = nullptr; + } + } +} +//////////////////////////////////////////////////////////////////////////////// +/// Internal function. Inform a TClass about its new TagDecl or NamespaceDecl. void TCling::UpdateClassInfoWithDecl(const void* vTD) { const NamedDecl* ND = static_cast(vTD); @@ -6503,6 +6546,8 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) name = ND->getQualifiedNameAsString(); } + + // Supposedly we are being called while something is being // loaded ... let's now tell the autoloader to do the work // yet another time. @@ -6511,35 +6556,12 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) // for example vector and vector TClass* cl = (TClass*)gROOT->GetListOfClasses()->FindObject(name.c_str()); if (cl && GetModTClasses().find(cl) == GetModTClasses().end()) { - TClingClassInfo* cci = ((TClingClassInfo*)cl->fClassInfo); - if (cci) { - // If we only had a forward declaration then update the - // TClingClassInfo with the definition if we have it now. - const TagDecl* tdOld = llvm::dyn_cast_or_null(cci->GetDecl()); - if (!tdOld || (tdDef && tdDef != tdOld)) { - cl->ResetCaches(); - TClass::RemoveClassDeclId(cci->GetDeclId()); - if (td) { - // It's a tag decl, not a namespace decl. - cci->Init(*cci->GetType()); - TClass::AddClassToDeclIdMap(cci->GetDeclId(), cl); - } - } - } else if (!cl->TestBit(TClass::kLoading) && !cl->fHasRootPcmInfo) { - cl->ResetCaches(); - // yes, this is almost a waste of time, but we do need to lookup - // the 'type' corresponding to the TClass anyway in order to - // preserve the opaque typedefs (Double32_t) - cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, cl->GetName()); - // We now need to update the state and bits. - if (cl->fState != TClass::kHasTClassInit) { - // if (!cl->fClassInfo->IsValid()) cl->fState = TClass::kForwardDeclared; else - cl->fState = TClass::kInterpreted; - cl->ResetBit(TClass::kIsEmulation); - } - TClass::AddClassToDeclIdMap(((TClingClassInfo*)(cl->fClassInfo))->GetDeclId(), cl); - } + RefreshClassInfo(cl, tdDef, false); } + // And here we should find the other 'aliases' (eg. vector) + // and update them too: + // foreach(aliascl in gROOT->GetListOfClasses()->FindAliasesOf(name.c_str())) + // RefreshClassInfo(cl, tdDef, true); } //////////////////////////////////////////////////////////////////////////////// diff --git a/core/metacling/src/TCling.h b/core/metacling/src/TCling.h index 2871c32d4cd7b..4bd68f2ff92ab 100644 --- a/core/metacling/src/TCling.h +++ b/core/metacling/src/TCling.h @@ -48,6 +48,7 @@ namespace clang { class EnumDecl; class FunctionDecl; class NamespaceDecl; + class TagDecl; class Type; class QualType; } @@ -313,6 +314,7 @@ class TCling final : public TInterpreter { static void UpdateClassInfo(char* name, Long_t tagnum); static void UpdateClassInfoWork(const char* name); + void RefreshClassInfo(TClass *cl, const clang::TagDecl *td, bool alias); void UpdateClassInfoWithDecl(const void* vTD); static void UpdateAllCanvases(); From efa1c7799358656f61909bbc39af6da60316fa36 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 6 Sep 2019 11:02:07 -0500 Subject: [PATCH 07/12] Expand StartParsingRAII In particular the CachedTokens need to be pushed/pop or otherwise give misleading information. This fixes ROOT-10224. --- .../include/cling/Utils/ParserStateRAII.h | 3 +++ .../cling/lib/Interpreter/LookupHelper.cpp | 23 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/interpreter/cling/include/cling/Utils/ParserStateRAII.h b/interpreter/cling/include/cling/Utils/ParserStateRAII.h index 8ecb1a5a28b5a..a033577e28c59 100644 --- a/interpreter/cling/include/cling/Utils/ParserStateRAII.h +++ b/interpreter/cling/include/cling/Utils/ParserStateRAII.h @@ -47,6 +47,9 @@ namespace cling { ParserStateRAII(clang::Parser& p, bool skipToEOF); ~ParserStateRAII(); + void SetSkipToEOF(bool newvalue) { + SkipToEOF = newvalue; + } }; } // end namespace cling diff --git a/interpreter/cling/lib/Interpreter/LookupHelper.cpp b/interpreter/cling/lib/Interpreter/LookupHelper.cpp index 794d6532052d7..f79e89f877b76 100644 --- a/interpreter/cling/lib/Interpreter/LookupHelper.cpp +++ b/interpreter/cling/lib/Interpreter/LookupHelper.cpp @@ -38,6 +38,13 @@ namespace cling { class StartParsingRAII { LookupHelper& m_LH; llvm::SaveAndRestore SaveIsRecursivelyRunning; + // Save and restore the state of the Parser and lexer. + // Note: ROOT::Internal::ParsingStateRAII also save and restore the state of Sema, + // including pending instantiation for example. It is not clear whether we need + // to do so here too or whether we need to also see the "on-going" semantic information ... + // For now, we leave Sema untouched. + clang::Preprocessor::CleanupAndRestoreCacheRAII fCleanupRAII; + clang::Parser::ParserCurTokRestoreRAII fSavedCurToken; ParserStateRAII ResetParserState; void prepareForParsing(llvm::StringRef code, llvm::StringRef bufferName, LookupHelper::DiagSetting diagOnOff); @@ -45,9 +52,11 @@ namespace cling { StartParsingRAII(LookupHelper& LH, llvm::StringRef code, llvm::StringRef bufferName, LookupHelper::DiagSetting diagOnOff) - : m_LH(LH), SaveIsRecursivelyRunning(LH.IsRecursivelyRunning), - ResetParserState(*LH.m_Parser.get(), - !LH.IsRecursivelyRunning /*skipToEOF*/) { + : m_LH(LH), SaveIsRecursivelyRunning(LH.IsRecursivelyRunning) + , fCleanupRAII(LH.m_Parser.get()->getPreprocessor()) + , fSavedCurToken(*LH.m_Parser.get()) + , ResetParserState(*LH.m_Parser.get(), !LH.IsRecursivelyRunning /*skipToEOF*/) + { LH.IsRecursivelyRunning = true; prepareForParsing(code, bufferName, diagOnOff); } @@ -1237,7 +1246,8 @@ namespace cling { llvm::StringRef funcName, Interpreter* Interp, UnqualifiedId &FuncId, - LookupHelper::DiagSetting diagOnOff) { + LookupHelper::DiagSetting diagOnOff, + ParserStateRAII &ResetParserState) { // Use a very simple parse step that dectect whether the name search (which // is already supposed to be an unqualified name) is a simple identifier, @@ -1336,6 +1346,7 @@ namespace cling { // Create a fake file to parse the function name. // // FIXME:, TODO: Cleanup that complete mess. + ResetParserState.SetSkipToEOF(true); { PP.getDiagnostics().setSuppressAllDiagnostics(diagOnOff == LookupHelper::NoDiagnostics); @@ -1410,9 +1421,9 @@ namespace cling { S.EnterDeclaratorContext(P.getCurScope(), foundDC); UnqualifiedId FuncId; - ParserStateRAII ResetParserState(P, true /*skipToEOF*/); + ParserStateRAII ResetParserState(P, false /*skipToEOF*/); if (!ParseWithShortcuts(foundDC, Context, funcName, Interp, - FuncId, diagOnOff)) { + FuncId, diagOnOff, ResetParserState)) { // Failed parse, cleanup. // Destroy the scope we created first, and // restore the original. From ab7848609c894f120f89aae4a5562a37a9903db9 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 6 Sep 2019 11:55:58 -0500 Subject: [PATCH 08/12] [NFC] Clang-formatting --- core/meta/src/TClass.cxx | 2 +- core/metacling/src/TCling.cxx | 10 ++++----- .../include/cling/Utils/ParserStateRAII.h | 4 +--- .../cling/lib/Interpreter/LookupHelper.cpp | 22 +++++++++---------- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/core/meta/src/TClass.cxx b/core/meta/src/TClass.cxx index a2b22d0acc1f0..65d41c79d6a2d 100644 --- a/core/meta/src/TClass.cxx +++ b/core/meta/src/TClass.cxx @@ -6057,7 +6057,7 @@ void TClass::SetUnloaded() GetName(),(int)fState); } - InsertTClassInRegistryRAII insertRAII(fState,fName,fNoInfoOrEmuOrFwdDeclNameRegistry); + InsertTClassInRegistryRAII insertRAII(fState, fName, fNoInfoOrEmuOrFwdDeclNameRegistry); // Make sure SetClassInfo, re-calculated the state. fState = kForwardDeclared; diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index c3c4cfc4621fe..12c9565765425 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6465,11 +6465,11 @@ Bool_t TCling::IsAutoLoadNamespaceCandidate(const clang::NamespaceDecl* nsDecl) return fNSFromRootmaps.count(nsDecl) != 0; } - //////////////////////////////////////////////////////////////////////////////// /// Internal function. Actually do the update of the ClassInfo when seeing // new TagDecl or NamespaceDecl. -void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alias) { +void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alias) +{ TClingClassInfo *cci = ((TClingClassInfo *)cl->fClassInfo); if (cci) { @@ -6493,7 +6493,7 @@ void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alia if (!alias) cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, tdDef); else - cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, cl->GetName()); + cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, cl->GetName()); if (((TClingClassInfo *)cl->fClassInfo)->IsValid()) { // We now need to update the state and bits. if (cl->fState != TClass::kHasTClassInit) { @@ -6540,14 +6540,12 @@ void TCling::UpdateClassInfoWithDecl(const void* vTD) return; } - clang::QualType type( td->getTypeForDecl(), 0 ); + clang::QualType type(td->getTypeForDecl(), 0); ROOT::TMetaUtils::GetNormalizedName(name, type, *fInterpreter, *fNormalizedCtxt); } else { name = ND->getQualifiedNameAsString(); } - - // Supposedly we are being called while something is being // loaded ... let's now tell the autoloader to do the work // yet another time. diff --git a/interpreter/cling/include/cling/Utils/ParserStateRAII.h b/interpreter/cling/include/cling/Utils/ParserStateRAII.h index a033577e28c59..ed33160a082b2 100644 --- a/interpreter/cling/include/cling/Utils/ParserStateRAII.h +++ b/interpreter/cling/include/cling/Utils/ParserStateRAII.h @@ -47,9 +47,7 @@ namespace cling { ParserStateRAII(clang::Parser& p, bool skipToEOF); ~ParserStateRAII(); - void SetSkipToEOF(bool newvalue) { - SkipToEOF = newvalue; - } + void SetSkipToEOF(bool newvalue) { SkipToEOF = newvalue; } }; } // end namespace cling diff --git a/interpreter/cling/lib/Interpreter/LookupHelper.cpp b/interpreter/cling/lib/Interpreter/LookupHelper.cpp index f79e89f877b76..af1ca3f764230 100644 --- a/interpreter/cling/lib/Interpreter/LookupHelper.cpp +++ b/interpreter/cling/lib/Interpreter/LookupHelper.cpp @@ -39,10 +39,10 @@ namespace cling { LookupHelper& m_LH; llvm::SaveAndRestore SaveIsRecursivelyRunning; // Save and restore the state of the Parser and lexer. - // Note: ROOT::Internal::ParsingStateRAII also save and restore the state of Sema, - // including pending instantiation for example. It is not clear whether we need - // to do so here too or whether we need to also see the "on-going" semantic information ... - // For now, we leave Sema untouched. + // Note: ROOT::Internal::ParsingStateRAII also save and restore the state of + // Sema, including pending instantiation for example. It is not clear + // whether we need to do so here too or whether we need to also see the + // "on-going" semantic information ... For now, we leave Sema untouched. clang::Preprocessor::CleanupAndRestoreCacheRAII fCleanupRAII; clang::Parser::ParserCurTokRestoreRAII fSavedCurToken; ParserStateRAII ResetParserState; @@ -52,11 +52,11 @@ namespace cling { StartParsingRAII(LookupHelper& LH, llvm::StringRef code, llvm::StringRef bufferName, LookupHelper::DiagSetting diagOnOff) - : m_LH(LH), SaveIsRecursivelyRunning(LH.IsRecursivelyRunning) - , fCleanupRAII(LH.m_Parser.get()->getPreprocessor()) - , fSavedCurToken(*LH.m_Parser.get()) - , ResetParserState(*LH.m_Parser.get(), !LH.IsRecursivelyRunning /*skipToEOF*/) - { + : m_LH(LH), SaveIsRecursivelyRunning(LH.IsRecursivelyRunning), + fCleanupRAII(LH.m_Parser.get()->getPreprocessor()), + fSavedCurToken(*LH.m_Parser.get()), + ResetParserState(*LH.m_Parser.get(), + !LH.IsRecursivelyRunning /*skipToEOF*/) { LH.IsRecursivelyRunning = true; prepareForParsing(code, bufferName, diagOnOff); } @@ -1422,8 +1422,8 @@ namespace cling { UnqualifiedId FuncId; ParserStateRAII ResetParserState(P, false /*skipToEOF*/); - if (!ParseWithShortcuts(foundDC, Context, funcName, Interp, - FuncId, diagOnOff, ResetParserState)) { + if (!ParseWithShortcuts(foundDC, Context, funcName, Interp, FuncId, + diagOnOff, ResetParserState)) { // Failed parse, cleanup. // Destroy the scope we created first, and // restore the original. From 12eb1442b81ac5e5736405c6cd3bb526f4611b97 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 6 Sep 2019 15:41:46 -0500 Subject: [PATCH 09/12] Improve type safety --- core/metacling/src/TCling.cxx | 3 +-- core/metacling/src/TCling.h | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 12c9565765425..a598d205516fd 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6511,9 +6511,8 @@ void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alia //////////////////////////////////////////////////////////////////////////////// /// Internal function. Inform a TClass about its new TagDecl or NamespaceDecl. -void TCling::UpdateClassInfoWithDecl(const void* vTD) +void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) { - const NamedDecl* ND = static_cast(vTD); const TagDecl* td = dyn_cast(ND); std::string name; TagDecl* tdDef = 0; diff --git a/core/metacling/src/TCling.h b/core/metacling/src/TCling.h index 4bd68f2ff92ab..04d0c5e497ca0 100644 --- a/core/metacling/src/TCling.h +++ b/core/metacling/src/TCling.h @@ -47,6 +47,7 @@ namespace clang { class DeclContext; class EnumDecl; class FunctionDecl; + class NamedDecl; class NamespaceDecl; class TagDecl; class Type; @@ -315,7 +316,7 @@ class TCling final : public TInterpreter { static void UpdateClassInfo(char* name, Long_t tagnum); static void UpdateClassInfoWork(const char* name); void RefreshClassInfo(TClass *cl, const clang::TagDecl *td, bool alias); - void UpdateClassInfoWithDecl(const void* vTD); + void UpdateClassInfoWithDecl(const clang::NamedDecl* ND); static void UpdateAllCanvases(); // Misc From 59ddae9df0a16245b4003db95146d076f3e82c7e Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 6 Sep 2019 15:44:12 -0500 Subject: [PATCH 10/12] In UpdateClassInfoWithDecl, ignore incomplete definition. --- core/metacling/src/TCling.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index a598d205516fd..b4f36430addf4 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6523,7 +6523,8 @@ void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) td = tdDef; ND = td; - if (llvm::isa(td->getDeclContext())) { + if (!td->isCompleteDefinition() || llvm::isa(td->getDeclContext())) { + // Ignore incomplete definition. // Ignore declaration within a function. return; } From d739e7fd5bdbac74f8e16738e4ec3c4f4d79cad8 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 6 Sep 2019 15:45:39 -0500 Subject: [PATCH 11/12] In UpdateClassInfoWithDecl, improve semantic use of variable name. I.e. don't reuse a variable for a slightly different menaing. --- core/metacling/src/TCling.cxx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index b4f36430addf4..753d6bcb4ec14 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6520,16 +6520,14 @@ void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) tdDef = td->getDefinition(); // Let's pass the decl to the TClass only if it has a definition. if (!tdDef) return; - td = tdDef; - ND = td; - if (!td->isCompleteDefinition() || llvm::isa(td->getDeclContext())) { + if (!tdDef->isCompleteDefinition() || llvm::isa(tdDef->getDeclContext())) { // Ignore incomplete definition. // Ignore declaration within a function. return; } - auto declName=ND->getNameAsString(); + auto declName = tdDef->getNameAsString(); // Check if we have registered the unqualified name into the list // of TClass that are in kNoInfo, kEmulated or kFwdDeclaredState. // Since this is used as heureutistic to avoid spurrious calls to GetNormalizedName @@ -6540,7 +6538,7 @@ void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) return; } - clang::QualType type(td->getTypeForDecl(), 0); + clang::QualType type(tdDef->getTypeForDecl(), 0); ROOT::TMetaUtils::GetNormalizedName(name, type, *fInterpreter, *fNormalizedCtxt); } else { name = ND->getQualifiedNameAsString(); From 609c9655ab7dd2ed62ebe33a8e5ea1990a1c1861 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 6 Sep 2019 15:59:28 -0500 Subject: [PATCH 12/12] In UpdateClassInfoWithDecl fix support for namespace (in RefreshClassInfo) --- core/metacling/src/TCling.cxx | 24 +++++++++++++++--------- core/metacling/src/TCling.h | 2 +- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 753d6bcb4ec14..fdcce7790be32 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -6468,18 +6468,18 @@ Bool_t TCling::IsAutoLoadNamespaceCandidate(const clang::NamespaceDecl* nsDecl) //////////////////////////////////////////////////////////////////////////////// /// Internal function. Actually do the update of the ClassInfo when seeing // new TagDecl or NamespaceDecl. -void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alias) +void TCling::RefreshClassInfo(TClass *cl, const clang::NamedDecl *def, bool alias) { TClingClassInfo *cci = ((TClingClassInfo *)cl->fClassInfo); if (cci) { // If we only had a forward declaration then update the // TClingClassInfo with the definition if we have it now. - const TagDecl *tdOld = llvm::dyn_cast_or_null(cci->GetDecl()); - if (!tdOld || (tdDef && tdDef != tdOld)) { + const NamedDecl *oldDef = llvm::dyn_cast_or_null(cci->GetDecl()); + if (!oldDef || (def && def != oldDef)) { cl->ResetCaches(); TClass::RemoveClassDeclId(cci->GetDeclId()); - if (tdDef) { + if (def) { // It's a tag decl, not a namespace decl. cci->Init(*cci->GetType()); TClass::AddClassToDeclIdMap(cci->GetDeclId(), cl); @@ -6490,8 +6490,8 @@ void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alia // yes, this is almost a waste of time, but we do need to lookup // the 'type' corresponding to the TClass anyway in order to // preserve the opaque typedefs (Double32_t) - if (!alias) - cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, tdDef); + if (!alias && def != nullptr) + cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, def); else cl->fClassInfo = (ClassInfo_t *)new TClingClassInfo(fInterpreter, cl->GetName()); if (((TClingClassInfo *)cl->fClassInfo)->IsValid()) { @@ -6513,11 +6513,14 @@ void TCling::RefreshClassInfo(TClass *cl, const clang::TagDecl *tdDef, bool alia /// Internal function. Inform a TClass about its new TagDecl or NamespaceDecl. void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) { - const TagDecl* td = dyn_cast(ND); + const TagDecl *td = dyn_cast(ND); + const NamespaceDecl *ns = dyn_cast(ND); + const NamedDecl *canon = nullptr; + std::string name; TagDecl* tdDef = 0; if (td) { - tdDef = td->getDefinition(); + canon = tdDef = td->getDefinition(); // Let's pass the decl to the TClass only if it has a definition. if (!tdDef) return; @@ -6540,6 +6543,9 @@ void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) clang::QualType type(tdDef->getTypeForDecl(), 0); ROOT::TMetaUtils::GetNormalizedName(name, type, *fInterpreter, *fNormalizedCtxt); + } else if (ns) { + canon = ns->getCanonicalDecl(); + name = ND->getQualifiedNameAsString(); } else { name = ND->getQualifiedNameAsString(); } @@ -6552,7 +6558,7 @@ void TCling::UpdateClassInfoWithDecl(const NamedDecl* ND) // for example vector and vector TClass* cl = (TClass*)gROOT->GetListOfClasses()->FindObject(name.c_str()); if (cl && GetModTClasses().find(cl) == GetModTClasses().end()) { - RefreshClassInfo(cl, tdDef, false); + RefreshClassInfo(cl, canon, false); } // And here we should find the other 'aliases' (eg. vector) // and update them too: diff --git a/core/metacling/src/TCling.h b/core/metacling/src/TCling.h index 04d0c5e497ca0..b1bd2daf2bd29 100644 --- a/core/metacling/src/TCling.h +++ b/core/metacling/src/TCling.h @@ -315,7 +315,7 @@ class TCling final : public TInterpreter { static void UpdateClassInfo(char* name, Long_t tagnum); static void UpdateClassInfoWork(const char* name); - void RefreshClassInfo(TClass *cl, const clang::TagDecl *td, bool alias); + void RefreshClassInfo(TClass *cl, const clang::NamedDecl *def, bool alias); void UpdateClassInfoWithDecl(const clang::NamedDecl* ND); static void UpdateAllCanvases();