From ea134ee0b64caede7190c0fc21644cdb39d00f1b Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Mon, 1 Dec 2025 14:28:15 +0100 Subject: [PATCH 1/8] [ntuple] backport type name normalization infrastructure Backport a minimal subset of the improvements of commit 705376f9340d9d31985dc55d988ba79ff08b89ab We don't backport part of the changes that implement RNTuple normalization of demangled paths. But we do apply some improvements to the renormalization of meta-normalized paths. This is a precondition to cleanly apply the series of patches fixing [U]Long64_t in template parameters. --- tree/ntuple/src/RFieldUtils.cxx | 79 ++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/tree/ntuple/src/RFieldUtils.cxx b/tree/ntuple/src/RFieldUtils.cxx index df7e89f15d15f..2c3f40a447ccd 100644 --- a/tree/ntuple/src/RFieldUtils.cxx +++ b/tree/ntuple/src/RFieldUtils.cxx @@ -58,9 +58,15 @@ const std::unordered_map typeTranslationMap{ {"ULong64_t", "unsigned long long"}, {"uint64_t", "std::uint64_t"}}; +// Natively supported types drop the default template arguments and the CV qualifiers in template arguments. +bool IsUserClass(const std::string &typeName) +{ + return typeName.rfind("std::", 0) != 0 && typeName.rfind("ROOT::VecOps::RVec<", 0) != 0; +} + // Recursively normalizes a template argument using the regular type name normalizer F as a helper. template -std::string GetNormalizedTemplateArg(const std::string &arg, F fnTypeNormalizer) +std::string GetNormalizedTemplateArg(const std::string &arg, bool keepQualifier, F fnTypeNormalizer) { R__ASSERT(!arg.empty()); @@ -69,6 +75,9 @@ std::string GetNormalizedTemplateArg(const std::string &arg, F fnTypeNormalizer) return ROOT::Internal::GetNormalizedInteger(arg); } + if (!keepQualifier) + return fnTypeNormalizer(arg); + std::string qualifier; // Type name template argument; template arguments must keep their CV qualifier if (arg.substr(0, 6) == "const " || (arg.length() > 14 && arg.substr(9, 6) == "const ")) @@ -118,6 +127,39 @@ std::vector FindTemplateAngleBrackets(const std::string &typeName) return result; } +template +void NormalizeTemplateArguments(std::string &templatedTypeName, F fnTypeNormalizer) +{ + const auto angleBrackets = FindTemplateAngleBrackets(templatedTypeName); + R__ASSERT(!angleBrackets.empty()); + + std::string normName; + std::string::size_type currentPos = 0; + for (std::size_t i = 0; i < angleBrackets.size(); i++) { + const auto [posOpen, posClose] = angleBrackets[i]; + // Append the type prefix until the open angle bracket. + normName += templatedTypeName.substr(currentPos, posOpen + 1 - currentPos); + + const auto argList = templatedTypeName.substr(posOpen + 1, posClose - posOpen - 1); + const auto templateArgs = ROOT::Internal::TokenizeTypeList(argList); + R__ASSERT(!templateArgs.empty()); + + const bool isUserClass = IsUserClass(templatedTypeName); + for (const auto &a : templateArgs) { + normName += GetNormalizedTemplateArg(a, isUserClass, fnTypeNormalizer) + ","; + } + + normName[normName.size() - 1] = '>'; + currentPos = posClose + 1; + } + + // Append the rest of the type from the last closing angle bracket. + const auto lastClosePos = angleBrackets.back().second; + normName += templatedTypeName.substr(lastClosePos + 1); + + templatedTypeName = normName; +} + } // namespace std::string ROOT::Internal::GetCanonicalTypePrefix(const std::string &typeName) @@ -233,31 +275,8 @@ std::string ROOT::Internal::GetRenormalizedTypeName(const std::string &metaNorma return canonicalTypePrefix; } - const auto angleBrackets = FindTemplateAngleBrackets(canonicalTypePrefix); - R__ASSERT(!angleBrackets.empty()); - - std::string normName; - std::string::size_type currentPos = 0; - for (std::size_t i = 0; i < angleBrackets.size(); i++) { - const auto [posOpen, posClose] = angleBrackets[i]; - // Append the type prefix until the open angle bracket. - normName += canonicalTypePrefix.substr(currentPos, posOpen + 1 - currentPos); - - const auto argList = canonicalTypePrefix.substr(posOpen + 1, posClose - posOpen - 1); - const auto templateArgs = TokenizeTypeList(argList); - R__ASSERT(!templateArgs.empty()); - - for (const auto &a : templateArgs) { - normName += GetNormalizedTemplateArg(a, GetRenormalizedTypeName) + ","; - } - - normName[normName.size() - 1] = '>'; - currentPos = posClose + 1; - } - - // Append the rest of the type from the last closing angle bracket. - const auto lastClosePos = angleBrackets.back().second; - normName += canonicalTypePrefix.substr(lastClosePos + 1); + std::string normName{canonicalTypePrefix}; + NormalizeTemplateArguments(normName, GetRenormalizedTypeName); return normName; } @@ -280,8 +299,7 @@ std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &o R__ASSERT(!angleBrackets.empty()); // For user-defined class types, we will need to get the default-initialized template arguments. - const bool isUserClass = - (canonicalTypePrefix.substr(0, 5) != "std::") && (canonicalTypePrefix.substr(0, 19) != "ROOT::VecOps::RVec<"); + const bool isUserClass = IsUserClass(canonicalTypePrefix); std::string normName; std::string::size_type currentPos = 0; @@ -295,7 +313,7 @@ std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &o R__ASSERT(!templateArgs.empty()); for (const auto &a : templateArgs) { - normName += GetNormalizedTemplateArg(a, GetNormalizedUnresolvedTypeName) + ","; + normName += GetNormalizedTemplateArg(a, isUserClass, GetNormalizedUnresolvedTypeName) + ","; } // For user-defined classes, append default-initialized template arguments. @@ -314,7 +332,8 @@ std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &o R__ASSERT(expandedTemplateArgs.size() >= templateArgs.size()); for (std::size_t j = templateArgs.size(); j < expandedTemplateArgs.size(); ++j) { - normName += GetNormalizedTemplateArg(expandedTemplateArgs[j], GetNormalizedUnresolvedTypeName) + ","; + normName += + GetNormalizedTemplateArg(expandedTemplateArgs[j], isUserClass, GetNormalizedUnresolvedTypeName) + ","; } } } From 976dcedf167b68216c288906b2f1daa827bea564 Mon Sep 17 00:00:00 2001 From: silverweed Date: Mon, 28 Jul 2025 15:12:02 +0200 Subject: [PATCH 2/8] [ntuple] Don't refuse to create a RProxiedCollectionField w/o dictionary This restriction is actually not required. (cherry picked from commit 0ce3049f550901e2785839ea5dbb75949aab8023) --- tree/ntuple/src/RFieldMeta.cxx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 63ab5ee654205..295a14ffb9da9 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -655,10 +655,6 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam fCollectionType = fProxy->GetCollectionType(); if (fProxy->HasPointers()) throw RException(R__FAIL("collection proxies whose value type is a pointer are not supported")); - if (!fProxy->GetCollectionClass()->HasDictionary()) { - throw RException(R__FAIL("dictionary not available for type " + - GetRenormalizedTypeName(fProxy->GetCollectionClass()->GetName()))); - } fIFuncsRead = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), true /* readFromDisk */); fIFuncsWrite = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), false /* readFromDisk */); From debf77b6a1bbd9ed9733660a2fcb00314d68f570 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sat, 22 Nov 2025 00:36:44 +0100 Subject: [PATCH 3/8] [ntuple] add Internal::NeedsMetaNameAsAlias() (cherry picked from commit 1648058caaa772c368cb9cdcdb4578adbd347034) --- tree/ntuple/inc/ROOT/RFieldUtils.hxx | 9 +++++++ tree/ntuple/src/RFieldUtils.cxx | 35 +++++++++++++++++++++++++++ tree/ntuple/test/ntuple_type_name.cxx | 31 ++++++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/tree/ntuple/inc/ROOT/RFieldUtils.hxx b/tree/ntuple/inc/ROOT/RFieldUtils.hxx index 60a9c7ef740c4..f313883c11069 100644 --- a/tree/ntuple/inc/ROOT/RFieldUtils.hxx +++ b/tree/ntuple/inc/ROOT/RFieldUtils.hxx @@ -29,6 +29,15 @@ std::string GetRenormalizedTypeName(const std::string &metaNormalizedName); /// ensure that e.g. fundamental types are normalized to the type used by RNTuple (e.g. int -> std::int32_t). std::string GetRenormalizedDemangledTypeName(const std::type_info &ti); +/// Checks if the meta normalized name is different from the RNTuple normalized name in a way that would cause +/// the RNTuple normalized name to request different streamer info. This can happen, e.g., if the type name has +/// Long64_t as a template parameter. In this case, RNTuple should use the meta normalized name as a type alias +/// to ensure correct reconstruction of objects from disk. +/// If the function returns true, renormalizedAlias contains the RNTuple normalized name that should be used as +/// type alias. +bool NeedsMetaNameAsAlias(const std::string &metaNormalizedName, std::string &renormalizedAlias, + bool isArgInTemplatedUserClass = false /* used in recursion */); + /// Applies all RNTuple type normalization rules except typedef resolution. std::string GetNormalizedUnresolvedTypeName(const std::string &origName); diff --git a/tree/ntuple/src/RFieldUtils.cxx b/tree/ntuple/src/RFieldUtils.cxx index 2c3f40a447ccd..98fd3e101a55d 100644 --- a/tree/ntuple/src/RFieldUtils.cxx +++ b/tree/ntuple/src/RFieldUtils.cxx @@ -59,6 +59,9 @@ const std::unordered_map typeTranslationMap{ {"uint64_t", "std::uint64_t"}}; // Natively supported types drop the default template arguments and the CV qualifiers in template arguments. +// Any types used as a template argument of user classes will keep [U]Long64_t template arguments for the type alias, +// e.g. MyClass> will normalize to `MyClass>` but keep the original +// spelling in the type alias. bool IsUserClass(const std::string &typeName) { return typeName.rfind("std::", 0) != 0 && typeName.rfind("ROOT::VecOps::RVec<", 0) != 0; @@ -281,6 +284,38 @@ std::string ROOT::Internal::GetRenormalizedTypeName(const std::string &metaNorma return normName; } +bool ROOT::Internal::NeedsMetaNameAsAlias(const std::string &metaNormalizedName, std::string &renormalizedAlias, + bool isArgInTemplatedUserClass) +{ + const auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName); + if (canonicalTypePrefix.find('<') == std::string::npos) { + // If there are no templates, the function is done. + return false; + } + + bool result = false; + const bool hasTemplatedUserClassParent = isArgInTemplatedUserClass || IsUserClass(canonicalTypePrefix); + auto fnCheckLong64 = [&](const std::string &arg) -> std::string { + if ((arg == "Long64_t" || arg == "ULong64_t") && hasTemplatedUserClassParent) { + result = true; + return arg; + } + + std::string renormalizedArgAlias; + if (NeedsMetaNameAsAlias(arg, renormalizedArgAlias, hasTemplatedUserClassParent)) { + result = true; + return renormalizedArgAlias; + } + + return GetRenormalizedTypeName(arg); + }; + + renormalizedAlias = canonicalTypePrefix; + NormalizeTemplateArguments(renormalizedAlias, fnCheckLong64); + + return result; +} + std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &origName) { const TClassEdit::EModType modType = static_cast( diff --git a/tree/ntuple/test/ntuple_type_name.cxx b/tree/ntuple/test/ntuple_type_name.cxx index bcc4d56412c03..ef085a4e3117b 100644 --- a/tree/ntuple/test/ntuple_type_name.cxx +++ b/tree/ntuple/test/ntuple_type_name.cxx @@ -286,3 +286,34 @@ TEST(RNTuple, ContextDependentTypeNames) } } } + +TEST(RNTuple, NeedsMetaNameAsAlias) +{ + using ROOT::Internal::NeedsMetaNameAsAlias; + + std::string renormalizedAlias; + EXPECT_FALSE(NeedsMetaNameAsAlias("bool", renormalizedAlias)); + EXPECT_FALSE(NeedsMetaNameAsAlias("std::vector", renormalizedAlias)); + EXPECT_FALSE(NeedsMetaNameAsAlias("std::vector", renormalizedAlias)); + EXPECT_TRUE(NeedsMetaNameAsAlias("::MyClass", renormalizedAlias)); + EXPECT_EQ("MyClass", renormalizedAlias); + EXPECT_TRUE(NeedsMetaNameAsAlias("MyClass", renormalizedAlias)); + EXPECT_TRUE(NeedsMetaNameAsAlias("std::vector >", renormalizedAlias)); + EXPECT_EQ("std::vector>", renormalizedAlias); + EXPECT_TRUE(NeedsMetaNameAsAlias("MyClass>", renormalizedAlias)); + EXPECT_EQ("MyClass>", renormalizedAlias); + EXPECT_TRUE(NeedsMetaNameAsAlias("MyClass>", renormalizedAlias)); + EXPECT_EQ("MyClass>", renormalizedAlias); + + EXPECT_TRUE(NeedsMetaNameAsAlias("::CL::TP::CL2", renormalizedAlias)); + EXPECT_EQ("CL::TP::CL2", renormalizedAlias); + EXPECT_TRUE(NeedsMetaNameAsAlias("::CL::CL2::TP", renormalizedAlias)); + EXPECT_EQ("CL::CL2::TP", renormalizedAlias); + + EXPECT_FALSE(NeedsMetaNameAsAlias("std::map", renormalizedAlias)); + EXPECT_FALSE(NeedsMetaNameAsAlias("std::map", renormalizedAlias)); + EXPECT_TRUE(NeedsMetaNameAsAlias("std::map>", renormalizedAlias)); + EXPECT_EQ("std::map>", renormalizedAlias); + EXPECT_TRUE(NeedsMetaNameAsAlias("MyClass>", renormalizedAlias)); + EXPECT_EQ("MyClass>", renormalizedAlias); +} From 7ba4b5dbc6db66fc95ab20762c4449468c8103cb Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sat, 22 Nov 2025 00:37:22 +0100 Subject: [PATCH 4/8] [ntuple] fix type names with [U]Long64_t template args Custom classes with [U]Long64_t template arguments need to use their meta-normalized name as type alias. Otherwise, during reconstruction with the RNTuple normalized name, the streamer info for the `std::[u]int64_t` argument will be requested (typically `long` instead of `long long`). (cherry picked from commit 6124192fa687003d227d4628906ec4aab8051059) --- tree/ntuple/src/RFieldMeta.cxx | 4 +++ tree/ntuple/test/CustomStruct.hxx | 13 ++++++- tree/ntuple/test/CustomStructLinkDef.h | 4 ++- tree/ntuple/test/rfield_class.cxx | 47 ++++++++++++++++---------- 4 files changed, 49 insertions(+), 19 deletions(-) diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 295a14ffb9da9..0b3107ea41156 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -127,6 +127,10 @@ ROOT::RClassField::RClassField(std::string_view fieldName, TClass *classp) if (!(fClass->ClassProperty() & kClassHasExplicitDtor)) fTraits |= kTraitTriviallyDestructible; + std::string renormalizedAlias; + if (Internal::NeedsMetaNameAsAlias(classp->GetName(), renormalizedAlias)) + fTypeAlias = renormalizedAlias; + int i = 0; const auto *bases = fClass->GetListOfBases(); assert(bases); diff --git a/tree/ntuple/test/CustomStruct.hxx b/tree/ntuple/test/CustomStruct.hxx index b0f85559be5c8..3f9e6a72d7709 100644 --- a/tree/ntuple/test/CustomStruct.hxx +++ b/tree/ntuple/test/CustomStruct.hxx @@ -92,14 +92,25 @@ struct alignas(std::uint64_t) TestEBO : public EmptyStruct { template class EdmWrapper { public: + struct Inner { + T fX; + }; + bool fIsPresent = true; T fMember; }; +template +struct EdmContent { + U fU; + V fV; +}; + class EdmContainer { public: + using EdmWrapperLong64_t = EdmWrapper; // Used to test that the streamer info for fWrapper will use long long - EdmWrapper fWrapper; + EdmWrapperLong64_t fWrapper; }; template diff --git a/tree/ntuple/test/CustomStructLinkDef.h b/tree/ntuple/test/CustomStructLinkDef.h index fe9c76be0fa3c..177494f0cefdd 100644 --- a/tree/ntuple/test/CustomStructLinkDef.h +++ b/tree/ntuple/test/CustomStructLinkDef.h @@ -26,10 +26,12 @@ #pragma link C++ class std::map+ ; #pragma link C++ class std::map+ ; +#pragma link C++ class EdmContent+; #pragma link C++ class EdmWrapper +; #pragma link C++ class EdmHash < 1> + ; #pragma link C++ class EdmWrapper+; -#pragma link C++ class EdmContainer; +#pragma link C++ class EdmWrapper>>+; +#pragma link C++ class EdmContainer+; #pragma link C++ class DataVector < int, double> + ; #pragma link C++ class DataVector < int, float> + ; diff --git a/tree/ntuple/test/rfield_class.cxx b/tree/ntuple/test/rfield_class.cxx index 8e8a51548e0d4..16e7f2bde600d 100644 --- a/tree/ntuple/test/rfield_class.cxx +++ b/tree/ntuple/test/rfield_class.cxx @@ -368,29 +368,35 @@ TEST(RNTuple, TClassMetaName) TEST(RNTuple, StreamerInfoRecords) { - // Every testee consists of the type stored on disk and the expected streamer info records - std::vector>> testees{ - {"float", {}}, - {"std::vector", {}}, - {"std::pair", {}}, - {"std::map", {}}, - {"CustomStruct", {"CustomStruct"}}, - {"std::vector", {"CustomStruct"}}, - {"std::map", {"CustomStruct"}}, - {"DerivedA", {"DerivedA", "CustomStruct"}}, - {"std::pair", {"DerivedA", "CustomStruct"}}, - {"EdmWrapper", {"EdmWrapper"}}, - {"TRotation", {"TRotation"}}}; + // Every testee consists of the type stored on disk, the expected streamer info records, and the expected type alias + std::vector, std::string>> testees{ + {"float", {}, ""}, + {"std::vector", {}, ""}, + {"std::pair", {}, ""}, + {"std::map", {}, ""}, + {"CustomStruct", {"CustomStruct"}, ""}, + {"std::vector", {"CustomStruct"}, ""}, + {"std::map", {"CustomStruct"}, ""}, + {"DerivedA", {"DerivedA", "CustomStruct"}, ""}, + {"std::pair", {"DerivedA", "CustomStruct"}, ""}, + {"EdmWrapper", {"EdmWrapper"}, "EdmWrapper"}, + {"EdmWrapper > >", + {"EdmWrapper > >", "EdmContent"}, + "EdmWrapper>>"}, + {"EdmContainer", {"EdmContainer", "EdmWrapper"}, ""}, + {"EdmWrapper::Inner", {"EdmWrapper::Inner"}, "EdmWrapper::Inner"}, + {"EdmContainer::EdmWrapperLong64_t", {"EdmWrapper"}, "EdmContainer::EdmWrapperLong64_t"}, + {"TRotation", {"TRotation"}, ""}}; for (const auto &t : testees) { FileRaii fileGuard("test_ntuple_streamer_info_records.root"); { auto model = ROOT::RNTupleModel::Create(); - if (t.first == "TRotation") { - model->AddField(std::make_unique("f", t.first)); + if (std::get<0>(t) == "TRotation") { + model->AddField(std::make_unique("f", std::get<0>(t))); } else { - model->AddField(ROOT::RFieldBase::Create("f", t.first).Unwrap()); + model->AddField(ROOT::RFieldBase::Create("f", std::get<0>(t)).Unwrap()); } auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); } @@ -398,7 +404,7 @@ TEST(RNTuple, StreamerInfoRecords) auto f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str())); ASSERT_TRUE(f && !f->IsZombie()); - std::unordered_set expectedInfos{t.second.begin(), t.second.end()}; + std::unordered_set expectedInfos{std::get<1>(t).begin(), std::get<1>(t).end()}; expectedInfos.insert("ROOT::RNTuple"); for (const auto info : TRangeDynCast(*f->GetStreamerInfoList())) { auto itr = expectedInfos.find(info->GetName()); @@ -409,5 +415,12 @@ TEST(RNTuple, StreamerInfoRecords) expectedInfos.erase(itr); } EXPECT_TRUE(expectedInfos.empty()); + + // Make sure we can reconstruct the fields + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + EXPECT_EQ(std::get<2>(t), reader->GetModel().GetConstField("f").GetTypeAlias()); + if (auto field = dynamic_cast(&reader->GetModel().GetConstField("f"))) { + EXPECT_EQ(std::get<1>(t)[0], field->GetClass()->GetName()); + } } } From f0f5e076faefe375d4b668a12d97407bd68c1d57 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sun, 23 Nov 2025 23:33:24 +0100 Subject: [PATCH 5/8] [ntuple] fix collection proxy with [U]Long64_t template arg (cherry picked from commit 571411b7b17b7d5125c149b2f1c0dad355e2e9d4) --- tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx | 2 +- tree/ntuple/src/RFieldMeta.cxx | 8 ++++++-- tree/ntuple/test/CustomStructLinkDef.h | 1 + tree/ntuple/test/SimpleCollectionProxy.hxx | 7 +++++-- tree/ntuple/test/rfield_class.cxx | 9 +++++++++ 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx b/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx index d5b40f6883771..9ff9756b88d88 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx @@ -256,7 +256,7 @@ template class RField::value>::type> final : public RProxiedCollectionField { public: static std::string TypeName() { return ROOT::Internal::GetRenormalizedDemangledTypeName(typeid(T)); } - RField(std::string_view name) : RProxiedCollectionField(name, TypeName()) + RField(std::string_view name) : RProxiedCollectionField(name, Internal::GetDemangledTypeName(typeid(T))) { static_assert(std::is_class::value, "collection proxy unsupported for fundamental types"); } diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 0b3107ea41156..6c9de20259d7d 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -638,7 +638,7 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam fNWritten(0) { if (!classp->GetCollectionProxy()) - throw RException(R__FAIL(std::string(GetTypeName()) + " has no associated collection proxy")); + throw RException(R__FAIL(std::string(classp->GetName()) + " has no associated collection proxy")); if (classp->Property() & kIsDefinedInStd) { static const std::vector supportedStdTypes = { "std::set<", "std::unordered_set<", "std::multiset<", "std::unordered_multiset<", @@ -654,6 +654,10 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam throw RException(R__FAIL(std::string(GetTypeName()) + " is not supported")); } + std::string renormalizedAlias; + if (Internal::NeedsMetaNameAsAlias(classp->GetName(), renormalizedAlias)) + fTypeAlias = renormalizedAlias; + fProxy.reset(classp->GetCollectionProxy()->Generate()); fProperties = fProxy->GetProperties(); fCollectionType = fProxy->GetCollectionType(); @@ -699,7 +703,7 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam case EDataType::kFloat_t: itemField = std::make_unique>("_0"); break; case EDataType::kDouble_t: itemField = std::make_unique>("_0"); break; case EDataType::kBool_t: itemField = std::make_unique>("_0"); break; - default: throw RException(R__FAIL("unsupported value type")); + default: throw RException(R__FAIL("unsupported value type: " + std::to_string(fProxy->GetType()))); } } diff --git a/tree/ntuple/test/CustomStructLinkDef.h b/tree/ntuple/test/CustomStructLinkDef.h index 177494f0cefdd..0cc1fcb2228c4 100644 --- a/tree/ntuple/test/CustomStructLinkDef.h +++ b/tree/ntuple/test/CustomStructLinkDef.h @@ -64,6 +64,7 @@ #pragma link C++ class StructUsingCollectionProxy + ; #pragma link C++ class StructUsingCollectionProxy> + ; #pragma link C++ class StructUsingCollectionProxy + ; +#pragma link C++ class StructUsingCollectionProxy+; #pragma link C++ class TrivialTraitsBase + ; #pragma link C++ class TrivialTraits + ; diff --git a/tree/ntuple/test/SimpleCollectionProxy.hxx b/tree/ntuple/test/SimpleCollectionProxy.hxx index 39024f141fee7..d8032972c51b3 100644 --- a/tree/ntuple/test/SimpleCollectionProxy.hxx +++ b/tree/ntuple/test/SimpleCollectionProxy.hxx @@ -72,6 +72,8 @@ public: if constexpr (std::is_same::value) return EDataType::kFloat_t; ; + if constexpr (std::is_same::value) + return EDataType::kLong64_t; return EDataType::kOther_t; } @@ -122,8 +124,9 @@ template <> struct IsCollectionProxy> : std::true_type { }; template <> -struct IsCollectionProxy> : std::true_type { -}; +struct IsCollectionProxy> : std::true_type {}; +template <> +struct IsCollectionProxy> : std::true_type {}; template <> struct IsCollectionProxy>> : std::true_type { diff --git a/tree/ntuple/test/rfield_class.cxx b/tree/ntuple/test/rfield_class.cxx index 16e7f2bde600d..f5af45565c96e 100644 --- a/tree/ntuple/test/rfield_class.cxx +++ b/tree/ntuple/test/rfield_class.cxx @@ -1,4 +1,5 @@ #include "ntuple_test.hxx" +#include "SimpleCollectionProxy.hxx" #include #include @@ -364,6 +365,14 @@ TEST(RNTuple, TClassMetaName) auto f4 = RFieldBase::Create("f", "EdmContainer").Unwrap(); EXPECT_STREQ("EdmWrapper", static_cast(f4->GetConstSubfields()[0])->GetClass()->GetName()); + + SimpleCollectionProxy> proxy; + auto cl = TClass::GetClass("StructUsingCollectionProxy"); + cl->CopyCollectionProxy(proxy); + + auto f5 = std::make_unique>>("f"); + EXPECT_TRUE(dynamic_cast(f5.get())); + EXPECT_EQ("StructUsingCollectionProxy", f5->GetTypeAlias()); } TEST(RNTuple, StreamerInfoRecords) From 502aef8d4c8201ed1a09e82cf32b17a42a618931 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sun, 23 Nov 2025 23:39:19 +0100 Subject: [PATCH 6/8] [ntuple] strip unneeded type alias arg from streamer field constructor (cherry picked from commit 9a821aaaff42ca7be99bf01f01fe056396c4ce0a) --- tree/ntuple/inc/ROOT/RField.hxx | 2 +- tree/ntuple/src/RFieldMeta.cxx | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index d0939002bc783..47c3b5b37b1f8 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -243,7 +243,7 @@ protected: void BeforeConnectPageSource(ROOT::Internal::RPageSource &pageSource) final; public: - RStreamerField(std::string_view fieldName, std::string_view className, std::string_view typeAlias = ""); + RStreamerField(std::string_view fieldName, std::string_view className); RStreamerField(RStreamerField &&other) = default; RStreamerField &operator=(RStreamerField &&other) = default; ~RStreamerField() final = default; diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 6c9de20259d7d..f30658807f656 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -859,10 +859,9 @@ class TBufferRecStreamer : public TBufferFile { } // anonymous namespace -ROOT::RStreamerField::RStreamerField(std::string_view fieldName, std::string_view className, std::string_view typeAlias) +ROOT::RStreamerField::RStreamerField(std::string_view fieldName, std::string_view className) : RStreamerField(fieldName, EnsureValidClass(className)) { - fTypeAlias = typeAlias; } ROOT::RStreamerField::RStreamerField(std::string_view fieldName, TClass *classp) @@ -888,7 +887,7 @@ void ROOT::RStreamerField::BeforeConnectPageSource(ROOT::Internal::RPageSource & std::unique_ptr ROOT::RStreamerField::CloneImpl(std::string_view newName) const { - return std::unique_ptr(new RStreamerField(newName, GetTypeName(), GetTypeAlias())); + return std::unique_ptr(new RStreamerField(newName, GetTypeName())); } std::size_t ROOT::RStreamerField::AppendImpl(const void *from) From 006f6c7cbc98c616c7c096ed7ab5d96140359ffb Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sun, 23 Nov 2025 23:44:04 +0100 Subject: [PATCH 7/8] [ntuple] fix streamer field with [U]Long64_t template arg (cherry picked from commit cc32bb65f4f6c050aa30777e6d6a462c28a7ed4f) --- tree/ntuple/src/RFieldMeta.cxx | 4 ++++ tree/ntuple/test/rfield_class.cxx | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index f30658807f656..20a0fc02c5c69 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -870,6 +870,10 @@ ROOT::RStreamerField::RStreamerField(std::string_view fieldName, TClass *classp) fClass(classp), fIndex(0) { + std::string renormalizedAlias; + if (Internal::NeedsMetaNameAsAlias(classp->GetName(), renormalizedAlias)) + fTypeAlias = renormalizedAlias; + fTraits |= kTraitTypeChecksum; // For RClassField, we only check for explicit constructors and destructors and then recursively combine traits from // all member subfields. For RStreamerField, we treat the class as a black box and additionally need to check for diff --git a/tree/ntuple/test/rfield_class.cxx b/tree/ntuple/test/rfield_class.cxx index f5af45565c96e..5cc9ab4a94e0a 100644 --- a/tree/ntuple/test/rfield_class.cxx +++ b/tree/ntuple/test/rfield_class.cxx @@ -373,6 +373,10 @@ TEST(RNTuple, TClassMetaName) auto f5 = std::make_unique>>("f"); EXPECT_TRUE(dynamic_cast(f5.get())); EXPECT_EQ("StructUsingCollectionProxy", f5->GetTypeAlias()); + + ROOT::RStreamerField f6("f", "EdmWrapper"); + EXPECT_STREQ("EdmWrapper", f6.GetClass()->GetName()); + EXPECT_EQ("EdmWrapper", f6.GetTypeAlias()); } TEST(RNTuple, StreamerInfoRecords) From 285eaed44c929b74adca31ff98ece6d52bbdf0f2 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 26 Nov 2025 14:57:33 +0100 Subject: [PATCH 8/8] [ntuple] bump spec to version 1.0.0.2 Clarify treatment of ROOT fundamental typedefs, including special treatment of `[U]Long64_t`. (cherry picked from commit b1406671ca89f562315d130061d50c98d87fd2f5) --- tree/ntuple/doc/BinaryFormatSpecification.md | 19 ++++++++++++++++++- tree/ntuple/inc/ROOT/RNTuple.hxx | 4 ++-- tree/ntuple/test/ntuple_minifile.cxx | 12 ++++++------ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/tree/ntuple/doc/BinaryFormatSpecification.md b/tree/ntuple/doc/BinaryFormatSpecification.md index 341316b375481..404477d0461fe 100644 --- a/tree/ntuple/doc/BinaryFormatSpecification.md +++ b/tree/ntuple/doc/BinaryFormatSpecification.md @@ -1,4 +1,4 @@ -# RNTuple Binary Format Specification 1.0.0.1 +# RNTuple Binary Format Specification 1.0.0.2 ## Versioning Notes @@ -826,6 +826,23 @@ For example, the type name `const pair>` will be `std::pair>` with an alias type name `std::pair>`. +#### Treatment of ROOT Typedefs + +A noteworthy implementation detail of the ROOT implemetation is the treatment of ROOT typedefs for fundamental types. +The types `Bool_t`, `Float_t`, `Double_t`, `[U]Char_t`, `[U]Short_t`, `[U]Int_t`, `[U]Long[64]_t` +are not treated as typedefs but they are directly mapped to the corresponding normalized RNTuple fundamental types. + +An exception is `[U]Long64_t` that appears as a template parameter of a user-defined class, +either directly or indirectly. +This case is treated as a normal typedef, +i.e. the RNTuple alias type will contain the type spelling that uses `[U]Long64_t`. +For example, the type `MyClass>` will be normalized to `MyClass>` +with a type alias that is equal to the original spelling. +This treatment is necessary for compatibility between the RNTuple schema and the ROOT streamer info records. + +The typedef `Double32_t` is treated as a normal typedef. +It is always normalized to `double` and the type name containing `Double32_t` is stored as type alias. + ### Fundamental Types The following fundamental types are stored as `leaf` fields with a single column each. diff --git a/tree/ntuple/inc/ROOT/RNTuple.hxx b/tree/ntuple/inc/ROOT/RNTuple.hxx index 4fca67730ac1d..240cc4ef3cc18 100644 --- a/tree/ntuple/inc/ROOT/RNTuple.hxx +++ b/tree/ntuple/inc/ROOT/RNTuple.hxx @@ -76,7 +76,7 @@ public: static constexpr std::uint16_t kVersionEpoch = 1; static constexpr std::uint16_t kVersionMajor = 0; static constexpr std::uint16_t kVersionMinor = 0; - static constexpr std::uint16_t kVersionPatch = 1; + static constexpr std::uint16_t kVersionPatch = 2; private: /// Version of the RNTuple binary format that the writer supports (see specification). @@ -88,7 +88,7 @@ private: std::uint16_t fVersionMajor = kVersionMajor; /// Changing the minor version indicates new optional fields added to the RNTuple metadata std::uint16_t fVersionMinor = kVersionMinor; - /// Changing the patch version indicates new backported features from newer binary format versions + /// Changing the patch version indicates clarifications or new backported features from newer binary format versions std::uint16_t fVersionPatch = kVersionPatch; /// The file offset of the header excluding the TKey part std::uint64_t fSeekHeader = 0; diff --git a/tree/ntuple/test/ntuple_minifile.cxx b/tree/ntuple/test/ntuple_minifile.cxx index 9c3f9bfd0a023..cda67fb8fcc65 100644 --- a/tree/ntuple/test/ntuple_minifile.cxx +++ b/tree/ntuple/test/ntuple_minifile.cxx @@ -79,7 +79,7 @@ TEST(MiniFile, Stream) EXPECT_EQ(1u, ntuple.GetVersionEpoch()); EXPECT_EQ(0u, ntuple.GetVersionMajor()); EXPECT_EQ(0u, ntuple.GetVersionMinor()); - EXPECT_EQ(1u, ntuple.GetVersionPatch()); + EXPECT_EQ(2u, ntuple.GetVersionPatch()); EXPECT_EQ(offHeader, ntuple.GetSeekHeader()); EXPECT_EQ(offFooter, ntuple.GetSeekFooter()); @@ -118,7 +118,7 @@ TEST(MiniFile, Proper) EXPECT_EQ(1u, ntuple.GetVersionEpoch()); EXPECT_EQ(0u, ntuple.GetVersionMajor()); EXPECT_EQ(0u, ntuple.GetVersionMinor()); - EXPECT_EQ(1u, ntuple.GetVersionPatch()); + EXPECT_EQ(2u, ntuple.GetVersionPatch()); EXPECT_EQ(offHeader, ntuple.GetSeekHeader()); EXPECT_EQ(offFooter, ntuple.GetSeekFooter()); @@ -817,7 +817,7 @@ TEST(MiniFile, UUID) TEST(MiniFile, FreeSlots) { FileRaii fileGuard("test_ntuple_minifile_freeslot.root"); - + // Write a TFile, delete some objects in it to create free slots, then write a RNTuple into it with a RBlob // fitting into a free slot with some free bytes left. Verify that we properly write the new free slot header // and don't end up with a corrupted TFile. @@ -828,8 +828,8 @@ TEST(MiniFile, FreeSlots) file->WriteObject(&dummyObj, "dummy2"); file->WriteObject(&dummyObj, "dummy3"); // These deletes will create a free slot about 200 B wide. - file->Delete("dummy;*"); - file->Delete("dummy2;*"); + file->Delete("dummy;*"); + file->Delete("dummy2;*"); } int maxKeySize = -1; @@ -880,7 +880,7 @@ TEST(MiniFile, FreeSlots) auto file = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); auto keysInfoIter = file->WalkTKeys(); std::vector keysInfo(keysInfoIter.begin(), keysInfoIter.end()); - + // Indices into `keysInfo` with the "interesting" free slots, i.e. those that have likely been shrunk by // RMiniFileWriter allocating an RBlob into them. std::vector gapIndices;