From 054886dfcc848f9a2648c28123d9c45d4a1a50ce Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 25 Feb 2026 10:53:00 +0100 Subject: [PATCH 1/2] [ntuple] use only "true type name" for class members Do not set the full type name as type alias. Pass only the true type name to RFieldBase::Create(). The true type name will contain Long64_t and Double32_t type name tokens, so we are still able to treat those correctly. The full type name would provide additional information. However, normalizing the full type name can cause header auto parsing. Also, the full type name, in general, cannot be used directly because it may be a type that is only valid within the class context. --- tree/ntuple/src/RFieldMeta.cxx | 14 +--------- tree/ntuple/test/CustomStruct.hxx | 13 +++++++++- tree/ntuple/test/CustomStructLinkDef.h | 1 + tree/ntuple/test/ntuple_type_name.cxx | 36 ++++++++++++++------------ tree/ntuple/test/ntuple_types.cxx | 23 +++++++++++----- 5 files changed, 50 insertions(+), 37 deletions(-) diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index df2abd2317a85..7d543175743ea 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -226,28 +226,16 @@ ROOT::RClassField::RClassField(std::string_view fieldName, TClass *classp) // context-dependent types (e.g. typedefs defined in the class itself - which will not be fully qualified in // the string returned by dataMember->GetFullTypeName()) std::string typeName{dataMember->GetTrueTypeName()}; - // RFieldBase::Create() set subField->fTypeAlias based on the assumption that the user specified typeName, which - // already went through one round of type resolution. - std::string origTypeName{dataMember->GetFullTypeName()}; // For C-style arrays, complete the type name with the size for each dimension, e.g. `int[4][2]` if (dataMember->Property() & kIsArray) { for (int dim = 0, n = dataMember->GetArrayDim(); dim < n; ++dim) { - const auto addedStr = "[" + std::to_string(dataMember->GetMaxIndex(dim)) + "]"; - typeName += addedStr; - origTypeName += addedStr; + typeName += "[" + std::to_string(dataMember->GetMaxIndex(dim)) + "]"; } } auto subField = RFieldBase::Create(dataMember->GetName(), typeName).Unwrap(); - const auto normTypeName = ROOT::Internal::GetNormalizedUnresolvedTypeName(origTypeName); - if (normTypeName == subField->GetTypeName()) { - SetTypeAliasOf(*subField, ""); - } else { - SetTypeAliasOf(*subField, normTypeName); - } - fTraits &= subField->GetTraits(); Attach(std::move(subField), RSubfieldInfo{kDataMember, static_cast(dataMember->GetOffset())}); } diff --git a/tree/ntuple/test/CustomStruct.hxx b/tree/ntuple/test/CustomStruct.hxx index 797aa8cd96ebb..6fed8437613af 100644 --- a/tree/ntuple/test/CustomStruct.hxx +++ b/tree/ntuple/test/CustomStruct.hxx @@ -43,9 +43,17 @@ struct CustomAtomicNotLockFree { }; struct CustomStruct { + template + struct VectorWrapper { + std::vector fVec; + }; + template using MyVec = std::vector; + template + using MyVectorWrapper = VectorWrapper; + float a = 0.0; std::vector v1; std::vector> v2; @@ -72,7 +80,10 @@ struct DerivedA2 : public CustomStruct { }; struct DerivedWithTypedef : public CustomStruct { - MyVec m; + MyVec m1; + MyVec m2; + MyVec m3; + MyVectorWrapper m4; }; struct DerivedB : public DerivedA { diff --git a/tree/ntuple/test/CustomStructLinkDef.h b/tree/ntuple/test/CustomStructLinkDef.h index 5469a664314b2..86ebcaac2c9b0 100644 --- a/tree/ntuple/test/CustomStructLinkDef.h +++ b/tree/ntuple/test/CustomStructLinkDef.h @@ -16,6 +16,7 @@ #pragma link C++ class CustomAtomicNotLockFree+; #pragma link C++ class CustomStruct+; +#pragma link C++ class CustomStruct::VectorWrapper+; #pragma link C++ class DerivedA+; #pragma link C++ class DerivedA2+; #pragma link C++ class DerivedWithTypedef + ; diff --git a/tree/ntuple/test/ntuple_type_name.cxx b/tree/ntuple/test/ntuple_type_name.cxx index 30172d03b7788..b13d7ce9f943d 100644 --- a/tree/ntuple/test/ntuple_type_name.cxx +++ b/tree/ntuple/test/ntuple_type_name.cxx @@ -310,13 +310,11 @@ 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("", hashSubfields[0]->GetTypeAlias()); EXPECT_EQ("fHash2", hashSubfields[1]->GetFieldName()); EXPECT_EQ("std::string", hashSubfields[1]->GetTypeName()); - // FIXME: This should really be EdmHash<1>::value_typeT::value_type>, but this is the value we get from - // TDataMember::GetFullTypeName right now... - EXPECT_EQ("value_typeT::value_type>", hashSubfields[1]->GetTypeAlias()); + EXPECT_EQ("", hashSubfields[1]->GetTypeAlias()); } TEST(RNTuple, ContextDependentTypeNames) @@ -330,28 +328,34 @@ TEST(RNTuple, ContextDependentTypeNames) auto model = RNTupleModel::Create(); auto fieldBase = RFieldBase::Create("foo", "DerivedWithTypedef").Unwrap(); model->AddField(std::move(fieldBase)); - auto ntuple = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); - auto entry = ntuple->GetModel().CreateBareEntry(); - auto ptr = std::make_unique(); - entry->BindRawPtr("foo", ptr.get()); - for (auto i = 0; i < 10; ++i) { - ptr->m.push_back(i); - ntuple->Fill(*entry); - ptr->m.clear(); - } + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); } { auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); - EXPECT_EQ(reader->GetNEntries(), 10); const auto &desc = reader->GetDescriptor(); const auto fooId = desc.FindFieldId("foo"); const auto baseId = desc.GetFieldDescriptor(fooId).GetLinkIds()[0]; { - const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("m", fooId)); + const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("m1", fooId)); + EXPECT_EQ(fdesc.GetTypeName(), "std::vector"); + EXPECT_EQ(fdesc.GetTypeAlias(), "std::vector"); + } + { + const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("m2", fooId)); + EXPECT_EQ(fdesc.GetTypeName(), "std::vector"); + EXPECT_EQ(fdesc.GetTypeAlias(), ""); + } + { + const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("m3", fooId)); EXPECT_EQ(fdesc.GetTypeName(), "std::vector"); - EXPECT_EQ(fdesc.GetTypeAlias(), "MyVec"); + EXPECT_EQ(fdesc.GetTypeAlias(), ""); + } + { + const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("m4", fooId)); + EXPECT_EQ(fdesc.GetTypeName(), "CustomStruct::VectorWrapper"); + EXPECT_EQ(fdesc.GetTypeAlias(), "CustomStruct::VectorWrapper"); } { const auto &fdesc = desc.GetFieldDescriptor(desc.FindFieldId("a", baseId)); diff --git a/tree/ntuple/test/ntuple_types.cxx b/tree/ntuple/test/ntuple_types.cxx index 56d6cd0940621..aa9870ba5702b 100644 --- a/tree/ntuple/test/ntuple_types.cxx +++ b/tree/ntuple/test/ntuple_types.cxx @@ -2250,9 +2250,15 @@ TEST(RNTuple, ContextDependentTypes) auto ptr = std::make_unique(); entry->BindRawPtr("foo", ptr.get()); for (auto i = 0; i < 10; ++i) { - ptr->m.push_back(i); + ptr->m1.push_back(i); + ptr->m2.push_back(i); + ptr->m3.push_back(i); + ptr->m4.fVec.push_back(i); ntuple->Fill(*entry); - ptr->m.clear(); + ptr->m1.clear(); + ptr->m2.clear(); + ptr->m3.clear(); + ptr->m4.fVec.clear(); } } @@ -2263,13 +2269,16 @@ TEST(RNTuple, ContextDependentTypes) const auto &model = reader->GetModel(); const auto &field = model.GetConstField("foo"); EXPECT_EQ(field.GetTypeName(), "DerivedWithTypedef"); - const auto &vec = model.GetConstField("foo.m"); - EXPECT_EQ(vec.GetTypeName(), "std::vector"); - EXPECT_EQ(vec.GetTypeAlias(), "MyVec"); - auto vecView = reader->GetView>("foo.m"); + auto m1View = reader->GetView>("foo.m1"); + auto m2View = reader->GetView>("foo.m2"); + auto m3View = reader->GetView>("foo.m3"); + auto m4View = reader->GetView>("foo.m4"); for (const auto &i : reader->GetEntryRange()) { - EXPECT_EQ(vecView(i), std::vector{static_cast(i)}); + EXPECT_DOUBLE_EQ(m1View(i)[0], std::vector{static_cast(i)}[0]); + EXPECT_EQ(m2View(i), std::vector{static_cast(i)}); + EXPECT_EQ(m3View(i), std::vector{static_cast(i)}); + EXPECT_EQ(m4View(i).fVec, std::vector{static_cast(i)}); } } } From 1bf27d9fccfacbcc7c8c9196b4bff5528416f7f6 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 25 Feb 2026 22:10:14 +0100 Subject: [PATCH 2/2] [ntuple] remove unused RFieldBase::SetTypeAliasOf() --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 0eac258e91357..c608ba8aa480b 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -418,8 +418,6 @@ protected: /// Allow parents to mark their childs as artificial fields (used in class and record fields) static void CallSetArtificialOn(RFieldBase &other) { other.SetArtificial(); } - /// Allow class fields to adjust the type alias of their members - static void SetTypeAliasOf(RFieldBase &other, const std::string &alias) { other.fTypeAlias = alias; } /// Operations on values of complex types, e.g. ones that involve multiple columns or for which no direct /// column type exists.