Skip to content

[cling] Update policy for GetFullyQualifiedName in cling utils #19292

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

Closed
guitargeek wants to merge 1 commit into from
Closed

[cling] Update policy for GetFullyQualifiedName in cling utils #19292

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
[cling] Update policy for GetFullyQualifiedName in cling utils 
Just like in CppInterOp, set the `SuppressElaboration` and
`FullyQualifiedName` policies when getting the fully qualified name of a
QualType.

Closes the following JIRA ticket:

https://its.cern.ch/jira/browse/ROOT-10150
  • Loading branch information
@guitargeek
guitargeek committed Jul 5, 2025
commit ea1957a975d78f8f822051432cea0f99f1f54382
2 changes: 2 additions & 0 deletions interpreter/cling/lib/Utils/AST.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1762,6 +1762,8 @@ namespace utils {
PrintingPolicy Policy(Ctx.getPrintingPolicy());
Policy.SuppressScope = false;
Policy.AnonymousTagLocations = false;
Policy.SuppressElaboration = true;
Policy.FullyQualifiedName = true;
return FQQT.getAsString(Policy);
}

Expand Down
4 changes: 4 additions & 0 deletions roottest/root/meta/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ ROOTTEST_ADD_TEST(loadernotapp
OUTREF loadernotapp.ref
DEPENDS ${GENERATE_EXECUTABLE_TEST})

ROOTTEST_ADD_TEST(reproduceROOT10150
MACRO reproduceROOT10150.C+
FAILREGEX "error")

ROOTTEST_COMPILE_MACRO(fornamespace.C)

#---Copy from source to binary some of the files
Expand Down
28 changes: 28 additions & 0 deletions roottest/root/meta/reproduceROOT10150.C
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// reproducer from https://its.cern.ch/jira/browse/ROOT-10150

#include <TInterpreter.h>
#include <functional>
#include <iostream>

namespace Foo {
struct Particle {
float m_pt;
float pt() const { return m_pt; }
};
} // namespace Foo

template <typename FType>
FType get_functor(std::string const &functor_string)
{
auto intern = gInterpreter->MakeInterpreterValue();
gInterpreter->Evaluate(functor_string.c_str(), *intern);
return *static_cast<FType *>(intern->GetAsPointer());
}

void reproduceROOT10150()
{
auto functor = get_functor<std::function<bool(Foo::Particle const &)>>(
"std::function<bool(Foo::Particle const&)>( [](Foo::Particle const& particle){return particle.pt() > 15;} );");
Foo::Particle low_pT_particle{1.f}, high_pT_particle{40.f};
std::cout << functor(high_pT_particle) << " " << functor(low_pT_particle) << std::endl;
}
6 changes: 3 additions & 3 deletions tree/ntuple/test/ntuple_type_name.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ TEST(RNTuple, TypeNameTemplatesNestedAlias)
ASSERT_EQ(2, hashSubfields.size());
EXPECT_EQ("fHash", hashSubfields[0]->GetFieldName());
EXPECT_EQ("std::string", hashSubfields[0]->GetTypeName());
EXPECT_EQ("EdmHash<1>::value_type", hashSubfields[0]->GetTypeAlias());
EXPECT_EQ("EdmHash::value_type", hashSubfields[0]->GetTypeAlias());
Copy link
Member

@hahnjo hahnjo Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is incorrect: EdmHash is a template class and requires template arguments. Clang stores the inner class outside the template instantiation as an optimization, but the type must have arguments.

Copy link
Contributor Author

@guitargeek guitargeek Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it's incorrect :( I realized the printing policies are quite brittle in general, and I can't make all cases work. I'll report on the problems I found soon, in a separate GitHub issue I guess.


EXPECT_EQ("fHash2", hashSubfields[1]->GetFieldName());
EXPECT_EQ("std::string", hashSubfields[1]->GetTypeName());
// FIXME: This should really be EdmHash<1>::value_typeT<EdmHash<1>::value_type>, but this is the value we get from
// TDataMember::GetFullTypeName right now...
EXPECT_EQ("value_typeT<EdmHash<1>::value_type>", hashSubfields[1]->GetTypeAlias());
EXPECT_EQ("EdmHash::value_typeT<EdmHash::value_type>", hashSubfields[1]->GetTypeAlias());
Comment on lines 223 to +225
Copy link
Member

@hahnjo hahnjo Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equally wrong, the expected outcome is even documented in the comment.

}

TEST(RNTuple, ContextDependentTypeNames)
Expand Down Expand Up @@ -257,7 +257,7 @@ TEST(RNTuple, ContextDependentTypeNames)
{
const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("m", fooId));
EXPECT_EQ(fdesc.GetTypeName(), "std::vector<std::int32_t>");
EXPECT_EQ(fdesc.GetTypeAlias(), "MyVec<std::int32_t>");
EXPECT_EQ(fdesc.GetTypeAlias(), "CustomStruct::MyVec<std::int32_t>");
Copy link
Member

@hahnjo hahnjo Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old version may be correct, depending on how you argue: it's the type alias as the user wrote it, inside of CustomStruct which happens to have a typedef...

}
{
const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("a", baseId));
Expand Down
2 changes: 1 addition & 1 deletion tree/ntuple/test/ntuple_types.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2261,7 +2261,7 @@ TEST(RNTuple, ContextDependentTypes)
EXPECT_EQ(field.GetTypeName(), "DerivedWithTypedef");
const auto &vec = model.GetConstField("foo.m");
EXPECT_EQ(vec.GetTypeName(), "std::vector<std::int32_t>");
EXPECT_EQ(vec.GetTypeAlias(), "MyVec<std::int32_t>");
EXPECT_EQ(vec.GetTypeAlias(), "CustomStruct::MyVec<std::int32_t>");

auto vecView = reader->GetView<DerivedWithTypedef::MyVec<int>>("foo.m");
for (const auto &i : reader->GetEntryRange()) {
Expand Down
Loading