Skip to content
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.

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

Do not complain about the internal of unique_ptr. Do not complain about transient std::tuple. #13900

Merged
merged 15 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion core/base/src/TMemberInspector.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void TMemberInspector::GenericShowMembers(const char *topClassName, const void *
}
}

TClass *top = TClass::GetClass(topClassName);
TClass *top = TClass::GetClass(topClassName, true, isTransient);
pcanal marked this conversation as resolved.
Show resolved Hide resolved
if (top) {
top->CallShowMembers(obj, *this, isTransient);
} else {
Expand Down
10 changes: 10 additions & 0 deletions core/clingutils/res/TClingUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,16 @@ const clang::TypedefNameDecl* GetAnnotatedRedeclarable(const clang::TypedefNameD
// Overload the template for tags, because we only check definitions.
const clang::TagDecl* GetAnnotatedRedeclarable(const clang::TagDecl* TND);

//______________________________________________________________________________
// Return true if the DeclContext is representing an entity reacheable from the
// global namespace
bool IsCtxtReacheable(const clang::DeclContext &ctxt);

//______________________________________________________________________________
// Return true if the decl is representing an entity reacheable from the
// global namespace
bool IsDeclReacheable(const clang::Decl &decl);

//______________________________________________________________________________
// Return true if the decl is part of the std namespace.
bool IsStdClass(const clang::RecordDecl &cl);
Expand Down
38 changes: 38 additions & 0 deletions core/clingutils/src/TClingUtils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -4363,6 +4363,44 @@ const clang::Type *ROOT::TMetaUtils::GetUnderlyingType(clang::QualType type)
return rawtype;
}

////////////////////////////////////////////////////////////////////////////////
/// Return true if the DeclContext is representing an entity reacheable from the
/// global namespace

bool ROOT::TMetaUtils::IsCtxtReacheable(const clang::DeclContext &ctxt)
{
if (ctxt.isNamespace() || ctxt.isTranslationUnit())
return true;
else if(const auto parentdecl = llvm::dyn_cast<clang::CXXRecordDecl>(&ctxt))
return ROOT::TMetaUtils::IsDeclReacheable(*parentdecl);
else
// For example "extern C" context.
return true;
}

////////////////////////////////////////////////////////////////////////////////
/// Return true if the decl is representing an entity reacheable from the
/// global namespace

bool ROOT::TMetaUtils::IsDeclReacheable(const clang::Decl &decl)
{
const clang::DeclContext *ctxt = decl.getDeclContext();
switch (decl.getAccess()) {
case clang::AS_public:
return !ctxt || IsCtxtReacheable(*ctxt);
case clang::AS_protected:
return false;
case clang::AS_private:
return false;
case clang::AS_none:
return !ctxt || IsCtxtReacheable(*ctxt);
default:
// IMPOSSIBLE
pcanal marked this conversation as resolved.
Show resolved Hide resolved
assert(false && "Unexpected value for the access property value in Clang");
return false;
}
}

////////////////////////////////////////////////////////////////////////////////
/// Return true, if the decl is part of the std namespace.

Expand Down
2 changes: 1 addition & 1 deletion core/meta/inc/TDictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ enum EProperty {
kIsCCompiled = 0x00040000,
kIsCPPCompiled = kIsCCompiled,
kIsCompiled = kIsCCompiled,
// 0x00080000 is available
kIsNotReacheable = 0x00080000, // Indicate that the entity can not be used from the Global Namespace
kIsConstant = 0x00100000,
kIsVirtualBase = 0x00200000,
kIsConstPointer = 0x00400000,
Expand Down
2 changes: 1 addition & 1 deletion core/meta/inc/TInterpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ 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) = 0;
virtual void SetClassInfo(TClass *cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE) = 0;

enum ECheckClassInfo {
kUnknown = 0, // backward compatible with false
Expand Down
2 changes: 1 addition & 1 deletion core/meta/src/TClass.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1514,7 +1514,7 @@ void TClass::Init(const char *name, Version_t cversion,
proto->FillTClass(this);
}
if (!fHasRootPcmInfo && gInterpreter->CheckClassInfo(fName, /* autoload = */ kTRUE)) {
gInterpreter->SetClassInfo(this); // sets fClassInfo pointer
gInterpreter->SetClassInfo(this, kFALSE, silent); // 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
9 changes: 8 additions & 1 deletion core/meta/src/TDataMember.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,14 @@ Long_t TDataMember::Property() const
if (!fInfo || !gCling->DataMemberInfo_IsValid(fInfo)) return 0;
int prop = gCling->DataMemberInfo_Property(fInfo);
int propt = gCling->DataMemberInfo_TypeProperty(fInfo);
t->fProperty = prop|propt;
t->fProperty = (prop | propt) & ~(kIsPublic | kIsProtected | kIsPrivate);
// Set to the strictest access of the member and the type
if ((prop | propt) & kIsPrivate)
t->fProperty |= kIsPrivate;
else if ((prop | propt) & kIsProtected)
t->fProperty |= kIsProtected;
else
t->fProperty |= kIsPublic;

t->fFullTypeName = TClassEdit::GetLong64_Name(gCling->DataMemberInfo_TypeName(fInfo));
t->fTrueTypeName = TClassEdit::GetLong64_Name(gCling->DataMemberInfo_TypeTrueName(fInfo));
Expand Down
47 changes: 40 additions & 7 deletions core/metacling/src/TCling.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -2716,6 +2716,15 @@ void TCling::InspectMembers(TMemberInspector& insp, const void* obj,
return;
}

if (TClassEdit::IsUniquePtr(cl->GetName())) {
// Ignore error caused by the inside of std::unique_ptr
pcanal marked this conversation as resolved.
Show resolved Hide resolved
// This is needed solely because of rootclingIO's IsUnsupportedUniquePointer
// which checks the number of elements in the GetListOfRealData.
// If this usage is removed, this can be replaced with a return statement.
// See https://github.com/root-project/root/issues/13574
isTransient = true;
pcanal marked this conversation as resolved.
Show resolved Hide resolved
}

const char* cobj = (const char*) obj; // for ptr arithmetics

// Treat the case of std::complex in a special manner. We want to enforce
Expand Down Expand Up @@ -3944,7 +3953,7 @@ static ETupleOrdering IsTupleAscending()
}
}

static std::string AlternateTuple(const char *classname, const cling::LookupHelper& lh)
static std::string AlternateTuple(const char *classname, const cling::LookupHelper& lh, Bool_t silent)
{
TClassEdit::TSplitType tupleContent(classname);
std::string alternateName = "TEmulatedTuple";
Expand All @@ -3955,6 +3964,26 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp
/*resultType*/nullptr, /* intantiateTemplate= */ false))
return fullname;

{
// Check if we can produce the tuple
auto iter = tupleContent.fElements.begin() + 1; // Skip the template name (tuple).
auto theEnd = tupleContent.fElements.end() - 1; // skip the 'stars'.
auto deleter = [](TypeInfo_t *type) {
gInterpreter->TypeInfo_Delete(type);
};
std::unique_ptr<TypeInfo_t, decltype(deleter)> type{ gInterpreter->TypeInfo_Factory(), deleter };
while (iter != theEnd) {
gInterpreter->TypeInfo_Init(type.get(), iter->c_str());
if (gInterpreter->TypeInfo_Property(type.get()) & kIsNotReacheable) {
if (!silent)
Error("Load","Could not declare alternate type for %s since %s (or one of its context) is private or protected",
classname, iter->c_str());
return "";
}
++iter;
}
}

std::string guard_name;
ROOT::TMetaUtils::GetCppName(guard_name,alternateName.c_str());
std::ostringstream guard;
Expand All @@ -3972,7 +4001,7 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp
switch(IsTupleAscending()) {
case ETupleOrdering::kAscending: {
unsigned int nMember = 0;
auto iter = tupleContent.fElements.begin() + 1; // Skip the template name (tuple)
auto iter = tupleContent.fElements.begin() + 1; // Skip the template name (tuple).
auto theEnd = tupleContent.fElements.end() - 1; // skip the 'stars'.
while (iter != theEnd) {
alternateTuple << " " << *iter << " _" << nMember << ";\n";
Expand All @@ -3983,8 +4012,8 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp
}
case ETupleOrdering::kDescending: {
unsigned int nMember = tupleContent.fElements.size() - 3;
auto iter = tupleContent.fElements.rbegin() + 1; // Skip the template name (tuple)
auto theEnd = tupleContent.fElements.rend() - 1; // skip the 'stars'.
auto iter = tupleContent.fElements.rbegin() + 1; // skip the 'stars'.
auto theEnd = tupleContent.fElements.rend() - 1; // Skip the template name (tuple).
while (iter != theEnd) {
alternateTuple << " " << *iter << " _" << nMember << ";\n";
++iter;
Expand All @@ -4002,7 +4031,10 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp
alternateTuple << "};\n";
alternateTuple << "}}\n";
alternateTuple << "#endif\n";
if (!gCling->Declare(alternateTuple.str().c_str())) {
if (!gCling->Declare(alternateTuple.str().c_str()))
{
// Declare is not silent (yet?), so add an explicit error message
// to indicate the consequence of the syntax errors.
Error("Load","Could not declare %s",alternateName.c_str());
return "";
}
Expand All @@ -4015,7 +4047,7 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp
/// If 'reload' is true, (attempt to) generate a new ClassInfo even if we
/// already have one.

void TCling::SetClassInfo(TClass* cl, Bool_t reload)
void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent)
{
// We are shutting down, there is no point in reloading, it only triggers
// redundant deserializations.
Expand Down Expand Up @@ -4062,7 +4094,7 @@ void TCling::SetClassInfo(TClass* cl, Bool_t reload)
// for the I/O to understand and handle.
if (strncmp(cl->GetName(),"tuple<",strlen("tuple<"))==0) {
if (!reload)
name = AlternateTuple(cl->GetName(), fInterpreter->getLookupHelper());
name = AlternateTuple(cl->GetName(), fInterpreter->getLookupHelper(), silent);
if (reload || name.empty()) {
// We could not generate the alternate
SetWithoutClassInfoState(cl);
Expand Down Expand Up @@ -8868,6 +8900,7 @@ Long_t TCling::FuncTempInfo_Property(FuncTempInfo_t *ft_info) const
break;
default:
// IMPOSSIBLE
assert(false && "Unexpected value for the access property value in Clang");
break;
}

Expand Down
2 changes: 1 addition & 1 deletion core/metacling/src/TCling.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ class TCling final : public TInterpreter {
void UpdateListOfGlobals() final;
void UpdateListOfGlobalFunctions() final;
void UpdateListOfTypes() final;
void SetClassInfo(TClass* cl, Bool_t reload = kFALSE) final;
void SetClassInfo(TClass* cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE) final;

ECheckClassInfo CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE) final;

Expand Down
4 changes: 4 additions & 0 deletions core/metacling/src/TClingBaseClassInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,10 @@ long TClingBaseClassInfo::Property() const
break;
case clang::AS_none:
// IMPOSSIBLE
assert(false && "Unexpected value for the access property value in Clang");
break;
default:
assert(false && "Unexpected value for the access property value in Clang");
break;
}
return property;
Expand Down
9 changes: 6 additions & 3 deletions core/metacling/src/TClingDataMemberInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -453,22 +453,25 @@ long TClingDataMemberInfo::Property() const
};

getParentAccessAndNonTransparentDC();

// TODO: Now that we have the kIsNotReacheable we could return the property
// to be reflecting the local information. However it is unclear if the
// information is used as-is (it appears to not be used in ROOT proper)
switch (strictestAccess) {
case clang::AS_public:
property |= kIsPublic;
break;
case clang::AS_protected:
property |= kIsProtected;
property |= kIsProtected | kIsNotReacheable;
break;
case clang::AS_private:
property |= kIsPrivate;
property |= kIsPrivate | kIsNotReacheable;
break;
case clang::AS_none: //?
property |= kIsPublic;
break;
default:
// IMPOSSIBLE
assert(false && "Unexpected value for the access property value in Clang");
break;
}
if (llvm::isa<clang::UsingShadowDecl>(thisDecl))
Expand Down
10 changes: 8 additions & 2 deletions core/metacling/src/TClingMethodInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -482,20 +482,26 @@ long TClingMethodInfo::Property() const
property |= kIsPublic;
break;
case clang::AS_protected:
property |= kIsProtected;
property |= kIsProtected | kIsNotReacheable;
break;
case clang::AS_private:
property |= kIsPrivate;
property |= kIsPrivate | kIsNotReacheable;
break;
case clang::AS_none:
if (declAccess->getDeclContext()->isNamespace())
property |= kIsPublic;
break;
default:
// IMPOSSIBLE
assert(false && "Unexpected value for the access property value in Clang");
break;
}

if (!(property & kIsNotReacheable)) {
if (! ROOT::TMetaUtils::IsDeclReacheable(*fd))
property |= kIsNotReacheable;
}

if (fd->isConstexpr())
property |= kIsConstexpr;
if (fd->getStorageClass() == clang::SC_Static) {
Expand Down
23 changes: 23 additions & 0 deletions core/metacling/src/TClingTypeInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,29 @@ long TClingTypeInfo::Property() const
const clang::TagDecl *TD = llvm::dyn_cast<clang::TagDecl>(tagQT->getDecl());
if (!TD)
return property;
switch (TD->getAccess()) {
case clang::AS_public:
property |= kIsPublic;
break;
case clang::AS_protected:
property |= kIsProtected | kIsNotReacheable;
break;
case clang::AS_private:
property |= kIsPrivate | kIsNotReacheable;
break;
case clang::AS_none:
if (TD->getDeclContext()->isNamespace())
property |= kIsPublic;
break;
default:
// IMPOSSIBLE
pcanal marked this conversation as resolved.
Show resolved Hide resolved
assert(false && "Unexpected value for the access property value in Clang");
break;
}
if (!(property & kIsNotReacheable)) {
if (! ROOT::TMetaUtils::IsDeclReacheable(*TD))
property |= kIsNotReacheable;
}
if (TD->isEnum()) {
property |= kIsEnum;
} else {
Expand Down
4 changes: 2 additions & 2 deletions core/metacling/test/TClingDataMemberInfoTests.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,11 @@ class Outer {

ASSERT_TRUE(TClass::GetClass("DMLookup::Outer")->GetListOfDataMembers()->FindObject("InnerPrivate"));
auto *dmInnerPrivate = (TDataMember*)TClass::GetClass("DMLookup::Outer")->GetListOfDataMembers()->FindObject("InnerPrivate");
EXPECT_EQ(dmInnerPrivate->Property(), kIsPrivate | kIsClass);
EXPECT_EQ(dmInnerPrivate->Property(), kIsPrivate | kIsClass | kIsNotReacheable);

ASSERT_TRUE(TClass::GetClass("DMLookup::Outer")->GetListOfDataMembers()->FindObject("InnerProtected"));
auto *dmInnerProtected = (TDataMember*)TClass::GetClass("DMLookup::Outer")->GetListOfDataMembers()->FindObject("InnerProtected");
EXPECT_EQ(dmInnerProtected->Property(), kIsProtected | kIsClass);
EXPECT_EQ(dmInnerProtected->Property(), kIsProtected | kIsClass | kIsNotReacheable);
}

TEST(TClingDataMemberInfo, Offset)
Expand Down
4 changes: 3 additions & 1 deletion io/rootpcm/src/rootclingIO.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ static bool IsUnsupportedUniquePointer(const char *normName, TDataMember *dm)
return true;
}

clm->BuildRealData();
// TODO: Is it not clear what situation we are checking for by checking if
// the unique_ptr class has any data members.
clm->BuildRealData(nullptr, /* istransient = */ true);
auto upDms = clm->GetListOfRealData();
if (!upDms) {
Error("CloseStreamerInfoROOTFile", "Cannot determine unique pointer %s data members.", dmTypeName);
Expand Down
Loading