Skip to content

[core] use one instead of two lookups within TClass::Init() #14760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[core] use one instead of two lookups within TClass::Init() #14760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/meta/inc/TInterpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,14 @@ class TInterpreter : public TNamed {
virtual void UpdateListOfGlobals() = 0;
virtual void UpdateListOfGlobalFunctions() = 0;
virtual void UpdateListOfTypes() = 0;
virtual void SetClassInfo(TClass *cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE) = 0;
virtual void SetClassInfo(TClass *cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE, TDictionary::DeclId_t decl = nullptr, Bool_t preChecked = kFALSE) = 0;

enum ECheckClassInfo {
kUnknown = 0, // backward compatible with false
kKnown = 1,
kWithClassDefInline = 2
};
virtual ECheckClassInfo CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE) = 0;
virtual ECheckClassInfo CheckClassInfo(const char *name, TDictionary::DeclId_t &decl, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE, Bool_t instantiateTemplate = kFALSE) = 0;

virtual Bool_t CheckClassTemplate(const char *name) = 0;
virtual Longptr_t Calc(const char *line, EErrorCode* error = nullptr) = 0;
Expand Down
8 changes: 5 additions & 3 deletions core/meta/src/TClass.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1586,8 +1586,9 @@ void TClass::Init(const char *name, Version_t cversion,
if (proto)
proto->FillTClass(this);
}
if (!fHasRootPcmInfo && gInterpreter->CheckClassInfo(fName, /* autoload = */ kTRUE)) {
gInterpreter->SetClassInfo(this, kFALSE, silent); // sets fClassInfo pointer
TDictionary::DeclId_t decl {};
if (!fHasRootPcmInfo && gInterpreter->CheckClassInfo(fName, decl, /* autoload = */ kTRUE, /* instantiateTemplate */ kTRUE)) {
gInterpreter->SetClassInfo(this, kFALSE, silent, decl, kTRUE); // sets fClassInfo pointer
if (fClassInfo) {
// This should be moved out of GetCheckSum itself however the last time
// we tried this cause problem, in particular in the end-of-process operation.
Expand Down Expand Up @@ -3305,7 +3306,8 @@ TClass *TClass::GetClass(const char *name, Bool_t load, Bool_t silent, size_t hi
printf("TClass::GetClass: Header Parsing - The representation of %s was not found in the type system. A lookup in the interpreter is about to be tried: this can cause parsing. This can be avoided selecting %s in the linkdef/selection file.\n",normalizedName.c_str(), normalizedName.c_str());
}
if (normalizedName.length()) {
auto cci = gInterpreter->CheckClassInfo(normalizedName.c_str(), kTRUE /* autoload */,
TDictionary::DeclId_t decl {};
auto cci = gInterpreter->CheckClassInfo(normalizedName.c_str(), decl, kTRUE /* autoload */,
kTRUE /*Only class, structs and ns*/);

// We could have an interpreted class with an inline ClassDef, in this case we do not
Expand Down
25 changes: 19 additions & 6 deletions core/metacling/src/TCling.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4061,8 +4061,10 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp
/// Set pointer to the TClingClassInfo in TClass.
/// If 'reload' is true, (attempt to) generate a new ClassInfo even if we
/// already have one.
/// \param decl parameter passed by copy, to be used if preChecked is true for creating TClingClassInfo
/// \param preChecked is true if we already called CheckClassInfo and got a valid decl

void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent)
void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent, TDictionary::DeclId_t decl, Bool_t preChecked)
{
// We are shutting down, there is no point in reloading, it only triggers
// redundant deserializations.
Expand Down Expand Up @@ -4122,7 +4124,7 @@ void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent)
// that is currently in the caller (like SetUnloaded) that disable AutoLoading and AutoParsing and
// code is in the callee (disabling template instantiation) and end up with a more explicit class:
// TClingClassInfoReadOnly.
TClingClassInfo* info = new TClingClassInfo(GetInterpreterImpl(), name.c_str(), instantiateTemplate);
TClingClassInfo* info = preChecked ? new TClingClassInfo(GetInterpreterImpl(), decl) : new TClingClassInfo(GetInterpreterImpl(), name.c_str(), instantiateTemplate);
if (!info->IsValid()) {
SetWithoutClassInfoState(cl);
delete info;
Expand Down Expand Up @@ -4196,9 +4198,11 @@ void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent)
/// specifically check that each level of nesting is already loaded.
/// In case of templates the idea is that everything between the outer
/// '<' and '>' has to be skipped, e.g.: `aap<pippo<noot>::klaas>::a_class`
///
/// \param decl parameter passed by reference and set to the found decl, if any

TInterpreter::ECheckClassInfo
TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly /* = kFALSE*/)
TCling::CheckClassInfo(const char *name, TDictionary::DeclId_t &decl, Bool_t autoload, Bool_t isClassOrNamespaceOnly /* = kFALSE*/, Bool_t instantiateTemplate /*= kFALSE*/)
{
R__LOCKGUARD(gInterpreterMutex);
static const char *anonEnum = "anonymous enum ";
Expand Down Expand Up @@ -4266,17 +4270,26 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp
// this forward declaration.
const cling::LookupHelper& lh = fInterpreter->getLookupHelper();
const clang::Type *type = nullptr;
const clang::Decl *decl
decl
= lh.findScope(classname,
gDebug > 5 ? cling::LookupHelper::WithDiagnostics
: cling::LookupHelper::NoDiagnostics,
&type, /* intantiateTemplate= */ false );
&type, instantiateTemplate);
if (!decl) {
std::string buf = TClassEdit::InsertStd(classname);
decl = lh.findScope(buf,
gDebug > 5 ? cling::LookupHelper::WithDiagnostics
: cling::LookupHelper::NoDiagnostics,
&type,false);
&type, instantiateTemplate);
}
if (!decl && type) {
const TagType *tagtype =type->getAs<TagType>();
if (tagtype) {
decl = tagtype->getDecl();
}
}
if (decl && ((const clang::Decl*)decl)->isInvalidDecl()) {
decl = nullptr;
}

if (type) {
Expand Down
4 changes: 2 additions & 2 deletions core/metacling/src/TCling.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ class TCling final : public TInterpreter {
void UpdateListOfGlobals() final;
void UpdateListOfGlobalFunctions() final;
void UpdateListOfTypes() final;
void SetClassInfo(TClass* cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE) final;
void SetClassInfo(TClass* cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE, TDictionary::DeclId_t decl = nullptr, Bool_t preChecked = kFALSE) final;

ECheckClassInfo CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE) final;
ECheckClassInfo CheckClassInfo(const char *name, TDictionary::DeclId_t &decl, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE, Bool_t instantiateTemplate = kFALSE) final;

Bool_t CheckClassTemplate(const char *name) final;
Longptr_t Calc(const char* line, EErrorCode* error = nullptr) final;
Expand Down
6 changes: 3 additions & 3 deletions core/metacling/src/TClingClassInfo.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, Bool_t all)
SetDecl(TU);
}

TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, const char *name, bool intantiateTemplate /* = true */)
TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, const char *name, bool instantiateTemplate /* = true */)
: TClingDeclInfo(nullptr), fInterp(interp), fFirstTime(true), fDescend(false), fIterAll(kTRUE), fIsIter(false),
fOffsetCache(0)
{
Expand All @@ -87,14 +87,14 @@ TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, const char *name, b
const Decl *decl = lh.findScope(name,
gDebug > 5 ? cling::LookupHelper::WithDiagnostics
: cling::LookupHelper::NoDiagnostics,
&type, intantiateTemplate);
&type, instantiateTemplate);
if (!decl) {
std::string buf = TClassEdit::InsertStd(name);
if (buf != name) {
decl = lh.findScope(buf,
gDebug > 5 ? cling::LookupHelper::WithDiagnostics
: cling::LookupHelper::NoDiagnostics,
&type, intantiateTemplate);
&type, instantiateTemplate);
}
}
if (!decl && type) {
Expand Down
2 changes: 1 addition & 1 deletion core/metacling/src/TClingClassInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class TClingClassInfo final : public TClingDeclInfo {
fDeclFileName(rhs.fDeclFileName), fOffsetCache(rhs.fOffsetCache)
{}
explicit TClingClassInfo(cling::Interpreter *, Bool_t all = kTRUE);
explicit TClingClassInfo(cling::Interpreter *, const char *classname, bool intantiateTemplate = kTRUE);
explicit TClingClassInfo(cling::Interpreter *, const char *classname, bool instantiateTemplate = kTRUE);
explicit TClingClassInfo(cling::Interpreter *interp, const clang::Type &tag);
explicit TClingClassInfo(cling::Interpreter *interp, const clang::Decl *D);
TClingClassInfo &operator=(const TClingClassInfo &rhs)
Expand Down
3 changes: 2 additions & 1 deletion core/textinput/src/Getline_color.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,10 @@ void ROOT::TextInputColorizer::ProcessTextChange(EditorRange& Modification,
}
std::string word = text.substr(i, wordLen);
char color = kColorNone;
TDictionary::DeclId_t decl;
if (gClassTable->GetDict(word.c_str())
|| gInterpreter->GetClassSharedLibs(word.c_str())
|| gInterpreter->CheckClassInfo(word.c_str(), false /*autoload*/)
|| gInterpreter->CheckClassInfo(word.c_str(), decl, false /*autoload*/)
|| ((THashTable*)gROOT->GetListOfTypes())
->THashTable::FindObject(word.c_str())) {
color = kColorType;
Expand Down
Loading