From 4a16b19b444e225d3ae57443464a0791c68ddd09 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 3 Dec 2025 10:13:01 +0100 Subject: [PATCH 1/2] [ntuple] factor out GetTypeList() field methods The variant field, the tuple field, and the pair field all use approximately the same GetTypeList() private static method. Factor this out in a single method in an anonymous namespace. --- tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx | 6 -- tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx | 1 - tree/ntuple/src/RFieldMeta.cxx | 58 ++++++++----------- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx b/tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx index 921d724c2cfc9..6e8437eb46175 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx @@ -134,9 +134,6 @@ public: /// The generic field for `std::pair` types class RPairField : public RRecordField { -private: - static std::string GetTypeList(const std::array, 2> &itemFields); - protected: RPairField(std::string_view fieldName, std::array, 2> itemFields, const std::array &offsets); @@ -188,9 +185,6 @@ public: /// The generic field for `std::tuple` types class RTupleField : public RRecordField { -private: - static std::string GetTypeList(const std::vector> &itemFields); - protected: RTupleField(std::string_view fieldName, std::vector> itemFields, const std::vector &offsets); diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx index e60198c72f8ac..eeeca91d20501 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx @@ -404,7 +404,6 @@ private: size_t fVariantOffset = 0; std::vector fNWritten; - static std::string GetTypeList(const std::vector> &itemFields); /// Extracts the index from an `std::variant` and transforms it into the 1-based index used for the switch column /// The implementation supports two memory layouts that are in use: a trailing unsigned byte, zero-indexed, /// having the exception caused empty state encoded by the max tag value, diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 5fdfd99aad56a..a648fd332b90b 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -71,6 +71,24 @@ TEnum *EnsureValidEnum(std::string_view enumName) return e; } +std::string GetTypeList(std::span> itemFields, bool useTypeAliases) +{ + std::string result; + for (size_t i = 0; i < itemFields.size(); ++i) { + if (useTypeAliases && !itemFields[i]->GetTypeAlias().empty()) { + result += itemFields[i]->GetTypeAlias(); + } else { + result += itemFields[i]->GetTypeName(); + } + result.push_back(','); + } + if (result.empty()) { + throw ROOT::RException(R__FAIL("invalid empty type list provided as template argument")); + } + result.pop_back(); // remove trailing comma + return result; +} + std::string BuildSetTypeName(ROOT::RSetField::ESetType setType, const ROOT::RFieldBase &innerField) { std::string typePrefix; @@ -667,14 +685,9 @@ void ROOT::REnumField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const //------------------------------------------------------------------------------ -std::string ROOT::RPairField::RPairField::GetTypeList(const std::array, 2> &itemFields) -{ - return itemFields[0]->GetTypeName() + "," + itemFields[1]->GetTypeName(); -} - ROOT::RPairField::RPairField(std::string_view fieldName, std::array, 2> itemFields, const std::array &offsets) - : ROOT::RRecordField(fieldName, "std::pair<" + GetTypeList(itemFields) + ">") + : ROOT::RRecordField(fieldName, "std::pair<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { AttachItemFields(std::move(itemFields)); fOffsets.push_back(offsets[0]); @@ -682,7 +695,7 @@ ROOT::RPairField::RPairField(std::string_view fieldName, std::array, 2> itemFields) - : ROOT::RRecordField(fieldName, "std::pair<" + GetTypeList(itemFields) + ">") + : ROOT::RRecordField(fieldName, "std::pair<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { AttachItemFields(std::move(itemFields)); @@ -1268,28 +1281,16 @@ void ROOT::RField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) //------------------------------------------------------------------------------ -std::string ROOT::RTupleField::RTupleField::GetTypeList(const std::vector> &itemFields) -{ - std::string result; - if (itemFields.empty()) - throw RException(R__FAIL("the type list for std::tuple must have at least one element")); - for (size_t i = 0; i < itemFields.size(); ++i) { - result += itemFields[i]->GetTypeName() + ","; - } - result.pop_back(); // remove trailing comma - return result; -} - ROOT::RTupleField::RTupleField(std::string_view fieldName, std::vector> itemFields, const std::vector &offsets) - : ROOT::RRecordField(fieldName, "std::tuple<" + GetTypeList(itemFields) + ">") + : ROOT::RRecordField(fieldName, "std::tuple<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { AttachItemFields(std::move(itemFields)); fOffsets = offsets; } ROOT::RTupleField::RTupleField(std::string_view fieldName, std::vector> itemFields) - : ROOT::RRecordField(fieldName, "std::tuple<" + GetTypeList(itemFields) + ">") + : ROOT::RRecordField(fieldName, "std::tuple<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { AttachItemFields(std::move(itemFields)); @@ -1362,17 +1363,6 @@ struct RVariantTag { } // anonymous namespace -std::string ROOT::RVariantField::GetTypeList(const std::vector> &itemFields) -{ - std::string result; - for (size_t i = 0; i < itemFields.size(); ++i) { - result += itemFields[i]->GetTypeName() + ","; - } - R__ASSERT(!result.empty()); // there is always at least one variant - result.pop_back(); // remove trailing comma - return result; -} - ROOT::RVariantField::RVariantField(std::string_view name, const RVariantField &source) : ROOT::RFieldBase(name, source.GetTypeName(), ROOT::ENTupleStructure::kVariant, false /* isSimple */), fMaxItemSize(source.fMaxItemSize), @@ -1387,8 +1377,8 @@ ROOT::RVariantField::RVariantField(std::string_view name, const RVariantField &s } ROOT::RVariantField::RVariantField(std::string_view fieldName, std::vector> itemFields) - : ROOT::RFieldBase(fieldName, "std::variant<" + GetTypeList(itemFields) + ">", ROOT::ENTupleStructure::kVariant, - false /* isSimple */) + : ROOT::RFieldBase(fieldName, "std::variant<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">", + ROOT::ENTupleStructure::kVariant, false /* isSimple */) { // The variant needs to initialize its own tag member fTraits |= kTraitTriviallyDestructible & ~kTraitTriviallyConstructible; From 934d6afef713cd685f5dbd74e8f9e8e52e4d8b52 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 3 Dec 2025 11:27:55 +0100 Subject: [PATCH 2/2] [ntuple] Propagate type aliases from item fields When constructing compound types from item fields, propagate alias types from the item fields to the parent field. This is already done globally in RFieldBase::Create() because the entire type name is compared to the unresolved type name. However, the type alias propagation was missing when a field is directly constructed with an item field. --- tree/ntuple/src/RField.cxx | 7 + tree/ntuple/src/RFieldMeta.cxx | 57 +++++++- tree/ntuple/src/RFieldSequenceContainer.cxx | 13 ++ tree/ntuple/test/ntuple_type_name.cxx | 136 ++++++++++++++++++++ 4 files changed, 206 insertions(+), 7 deletions(-) diff --git a/tree/ntuple/src/RField.cxx b/tree/ntuple/src/RField.cxx index c565c9c8778c5..b5250382b2dc6 100644 --- a/tree/ntuple/src/RField.cxx +++ b/tree/ntuple/src/RField.cxx @@ -842,6 +842,9 @@ ROOT::RNullableField::RNullableField(std::string_view fieldName, const std::stri : ROOT::RFieldBase(fieldName, typePrefix + "<" + itemField->GetTypeName() + ">", ROOT::ENTupleStructure::kCollection, false /* isSimple */) { + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = typePrefix + "<" + itemField->GetTypeAlias() + ">"; + Attach(std::move(itemField)); } @@ -1183,6 +1186,10 @@ ROOT::RAtomicField::RAtomicField(std::string_view fieldName, std::unique_ptrGetTraits() & kTraitTriviallyDestructible) fTraits |= kTraitTriviallyDestructible; + + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = "std::atomic<" + itemField->GetTypeAlias() + ">"; + Attach(std::move(itemField)); } diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index a648fd332b90b..41a57d732aa31 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -89,7 +89,7 @@ std::string GetTypeList(std::span> itemFields, return result; } -std::string BuildSetTypeName(ROOT::RSetField::ESetType setType, const ROOT::RFieldBase &innerField) +std::string BuildSetTypeName(ROOT::RSetField::ESetType setType, const ROOT::RFieldBase &innerField, bool useTypeAlias) { std::string typePrefix; switch (setType) { @@ -99,10 +99,13 @@ std::string BuildSetTypeName(ROOT::RSetField::ESetType setType, const ROOT::RFie case ROOT::RSetField::ESetType::kUnorderedMultiSet: typePrefix = "std::unordered_multiset<"; break; default: R__ASSERT(false); } - return typePrefix + innerField.GetTypeName() + ">"; + return typePrefix + + ((useTypeAlias && !innerField.GetTypeAlias().empty()) ? innerField.GetTypeAlias() + : innerField.GetTypeName()) + + ">"; } -std::string BuildMapTypeName(ROOT::RMapField::EMapType mapType, const ROOT::RFieldBase *innerField) +std::string BuildMapTypeName(ROOT::RMapField::EMapType mapType, const ROOT::RFieldBase *innerField, bool useTypeAliases) { if (const auto pairField = dynamic_cast(innerField)) { std::string typePrefix; @@ -113,8 +116,18 @@ std::string BuildMapTypeName(ROOT::RMapField::EMapType mapType, const ROOT::RFie case ROOT::RMapField::EMapType::kUnorderedMultiMap: typePrefix = "std::unordered_multimap<"; break; default: R__ASSERT(false); } - auto subFields = pairField->GetConstSubfields(); - return typePrefix + subFields[0]->GetTypeName() + "," + subFields[1]->GetTypeName() + ">"; + const auto &items = pairField->GetConstSubfields(); + std::string type = typePrefix; + for (int i : {0, 1}) { + if (useTypeAliases && !items[i]->GetTypeAlias().empty()) { + type += items[i]->GetTypeAlias(); + } else { + type += items[i]->GetTypeName(); + } + if (i == 0) + type.push_back(','); + } + return type + ">"; } throw ROOT::RException(R__FAIL("RMapField inner field type must be of RPairField")); @@ -689,6 +702,10 @@ ROOT::RPairField::RPairField(std::string_view fieldName, std::array &offsets) : ROOT::RRecordField(fieldName, "std::pair<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { + const std::string typeAlias = "std::pair<" + GetTypeList(itemFields, true /* useTypeAliases */) + ">"; + if (typeAlias != GetTypeName()) + fTypeAlias = typeAlias; + AttachItemFields(std::move(itemFields)); fOffsets.push_back(offsets[0]); fOffsets.push_back(offsets[1]); @@ -697,6 +714,10 @@ ROOT::RPairField::RPairField(std::string_view fieldName, std::array, 2> itemFields) : ROOT::RRecordField(fieldName, "std::pair<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { + const std::string typeAlias = "std::pair<" + GetTypeList(itemFields, true /* useTypeAliases */) + ">"; + if (typeAlias != GetTypeName()) + fTypeAlias = typeAlias; + AttachItemFields(std::move(itemFields)); // ISO C++ does not guarantee any specific layout for `std::pair`; query TClass for the member offsets @@ -942,8 +963,13 @@ void ROOT::RProxiedCollectionField::AcceptVisitor(ROOT::Detail::RFieldVisitor &v //------------------------------------------------------------------------------ ROOT::RMapField::RMapField(std::string_view fieldName, EMapType mapType, std::unique_ptr itemField) - : RProxiedCollectionField(fieldName, EnsureValidClass(BuildMapTypeName(mapType, itemField.get()))), fMapType(mapType) + : RProxiedCollectionField(fieldName, + EnsureValidClass(BuildMapTypeName(mapType, itemField.get(), false /* useTypeAliases */))), + fMapType(mapType) { + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = BuildMapTypeName(mapType, itemField.get(), true /* useTypeAliases */); + auto *itemClass = fProxy->GetValueClass(); fItemSize = itemClass->GetClassSize(); @@ -973,10 +999,15 @@ void ROOT::RMapField::ReconcileOnDiskField(const RNTupleDescriptor &desc) //------------------------------------------------------------------------------ ROOT::RSetField::RSetField(std::string_view fieldName, ESetType setType, std::unique_ptr itemField) - : ROOT::RProxiedCollectionField(fieldName, EnsureValidClass(BuildSetTypeName(setType, *itemField))), + : ROOT::RProxiedCollectionField(fieldName, + EnsureValidClass(BuildSetTypeName(setType, *itemField, false /* useTypeAlias */))), fSetType(setType) { + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = BuildSetTypeName(setType, *itemField, true /* useTypeAlias */); + fItemSize = itemField->GetValueSize(); + Attach(std::move(itemField)); } @@ -1285,6 +1316,10 @@ ROOT::RTupleField::RTupleField(std::string_view fieldName, std::vector &offsets) : ROOT::RRecordField(fieldName, "std::tuple<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { + const std::string typeAlias = "std::tuple<" + GetTypeList(itemFields, true /* useTypeAliases */) + ">"; + if (typeAlias != GetTypeName()) + fTypeAlias = typeAlias; + AttachItemFields(std::move(itemFields)); fOffsets = offsets; } @@ -1292,6 +1327,10 @@ ROOT::RTupleField::RTupleField(std::string_view fieldName, std::vector> itemFields) : ROOT::RRecordField(fieldName, "std::tuple<" + GetTypeList(itemFields, false /* useTypeAliases */) + ">") { + const std::string typeAlias = "std::tuple<" + GetTypeList(itemFields, true /* useTypeAliases */) + ">"; + if (typeAlias != GetTypeName()) + fTypeAlias = typeAlias; + AttachItemFields(std::move(itemFields)); auto *c = TClass::GetClass(GetTypeName().c_str()); @@ -1383,6 +1422,10 @@ ROOT::RVariantField::RVariantField(std::string_view fieldName, std::vector"; + if (typeAlias != GetTypeName()) + fTypeAlias = typeAlias; + auto nFields = itemFields.size(); if (nFields == 0 || nFields > kMaxVariants) { throw RException(R__FAIL("invalid number of variant fields (outside [1.." + std::to_string(kMaxVariants) + ")")); diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index d780ec31ade08..03789767f1e34 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -124,6 +124,10 @@ ROOT::RArrayField::RArrayField(std::string_view fieldName, std::unique_ptrGetTraits() & ~kTraitMappable; + if (!itemField->GetTypeAlias().empty()) { + fTypeAlias = "std::array<" + itemField->GetTypeAlias() + "," + + Internal::GetNormalizedInteger(static_cast(arrayLength)) + ">"; + } Attach(std::move(itemField)); } @@ -245,6 +249,8 @@ ROOT::RRVecField::RRVecField(std::string_view fieldName, std::unique_ptrGetTraits() & kTraitTriviallyDestructible)) fItemDeleter = GetDeleterOf(*itemField); + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = "ROOT::VecOps::RVec<" + itemField->GetTypeAlias() + ">"; Attach(std::move(itemField)); fValueSize = EvalRVecValueSize(fSubfields[0]->GetAlignment(), fSubfields[0]->GetValueSize(), GetAlignment()); @@ -549,6 +555,9 @@ ROOT::RVectorField::RVectorField(std::string_view fieldName, std::unique_ptrempty()) fTraits |= kTraitEmulatedField; + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = "std::vector<" + itemField->GetTypeAlias() + ">"; + if (!(itemField->GetTraits() & kTraitTriviallyDestructible)) fItemDeleter = GetDeleterOf(*itemField); Attach(std::move(itemField)); @@ -859,6 +868,8 @@ ROOT::RArrayAsRVecField::RArrayAsRVecField(std::string_view fieldName, std::uniq fItemSize(itemField->GetValueSize()), fArrayLength(arrayLength) { + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = "ROOT::VecOps::RVec<" + itemField->GetTypeAlias() + ">"; Attach(std::move(itemField)); fValueSize = EvalRVecValueSize(fSubfields[0]->GetAlignment(), fSubfields[0]->GetValueSize(), GetAlignment()); if (!(fSubfields[0]->GetTraits() & kTraitTriviallyDestructible)) @@ -961,6 +972,8 @@ ROOT::RArrayAsVectorField::RArrayAsVectorField(std::string_view fieldName, std:: fItemSize(itemField->GetValueSize()), fArrayLength(arrayLength) { + if (!itemField->GetTypeAlias().empty()) + fTypeAlias = "std::vector<" + itemField->GetTypeAlias() + ">"; Attach(std::move(itemField)); if (!(fSubfields[0]->GetTraits() & kTraitTriviallyDestructible)) fItemDeleter = GetDeleterOf(*fSubfields[0]); diff --git a/tree/ntuple/test/ntuple_type_name.cxx b/tree/ntuple/test/ntuple_type_name.cxx index c9748aa7bbffe..b1b92aa3b695b 100644 --- a/tree/ntuple/test/ntuple_type_name.cxx +++ b/tree/ntuple/test/ntuple_type_name.cxx @@ -411,3 +411,139 @@ TEST(RNTuple, NeedsMetaNameAsAlias) EXPECT_TRUE(NeedsMetaNameAsAlias("MyClass>", renormalizedAlias)); EXPECT_EQ("MyClass>", renormalizedAlias); } + +TEST(RNTuple, PropagateTypeAlias) +{ + { + auto f = ROOT::RFieldBase::Create("f", "std::vector").Unwrap(); + EXPECT_EQ("std::vector", f->GetTypeName()); + EXPECT_EQ("std::vector", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "ROOT::RVec").Unwrap(); + EXPECT_EQ("ROOT::VecOps::RVec", f->GetTypeName()); + EXPECT_EQ("ROOT::VecOps::RVec", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::array").Unwrap(); + EXPECT_EQ("std::array", f->GetTypeName()); + EXPECT_EQ("std::array", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::variant").Unwrap(); + EXPECT_EQ("std::variant", f->GetTypeName()); + EXPECT_EQ("std::variant", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::pair").Unwrap(); + EXPECT_EQ("std::pair", f->GetTypeName()); + EXPECT_EQ("std::pair", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::tuple").Unwrap(); + EXPECT_EQ("std::tuple", f->GetTypeName()); + EXPECT_EQ("std::tuple", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::optional").Unwrap(); + EXPECT_EQ("std::optional", f->GetTypeName()); + EXPECT_EQ("std::optional", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::multiset").Unwrap(); + EXPECT_EQ("std::multiset", f->GetTypeName()); + EXPECT_EQ("std::multiset", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::multimap").Unwrap(); + EXPECT_EQ("std::multimap", f->GetTypeName()); + EXPECT_EQ("std::multimap", f->GetTypeAlias()); + } + + { + auto f = ROOT::RFieldBase::Create("f", "std::atomic").Unwrap(); + EXPECT_EQ("std::atomic", f->GetTypeName()); + EXPECT_EQ("std::atomic", f->GetTypeAlias()); + } + + auto GetDouble32Item = []() { + auto item = std::make_unique>("_0"); + item->SetDouble32(); + return item; + }; + + { + auto f = std::make_unique("f", GetDouble32Item()); + EXPECT_EQ("std::vector", f->GetTypeName()); + EXPECT_EQ("std::vector", f->GetTypeAlias()); + } + + { + auto f = std::make_unique("f", GetDouble32Item()); + EXPECT_EQ("ROOT::VecOps::RVec", f->GetTypeName()); + EXPECT_EQ("ROOT::VecOps::RVec", f->GetTypeAlias()); + } + + { + auto f = std::make_unique("f", GetDouble32Item(), 2); + EXPECT_EQ("std::array", f->GetTypeName()); + EXPECT_EQ("std::array", f->GetTypeAlias()); + } + + { + std::vector> items; + items.emplace_back(GetDouble32Item()); + items.emplace_back(std::make_unique>("f")); + auto f = std::make_unique("f", std::move(items)); + EXPECT_EQ("std::variant", f->GetTypeName()); + EXPECT_EQ("std::variant", f->GetTypeAlias()); + } + + { + std::array, 2> items; + items[0] = GetDouble32Item(); + items[1] = std::make_unique>("f"); + auto f = std::make_unique("f", std::move(items)); + EXPECT_EQ("std::pair", f->GetTypeName()); + EXPECT_EQ("std::pair", f->GetTypeAlias()); + } + + { + std::vector> items; + items.emplace_back(GetDouble32Item()); + items.emplace_back(std::make_unique>("f")); + auto f = std::make_unique("f", std::move(items)); + EXPECT_EQ("std::tuple", f->GetTypeName()); + EXPECT_EQ("std::tuple", f->GetTypeAlias()); + } + + { + auto f = std::make_unique("f", GetDouble32Item()); + EXPECT_EQ("std::optional", f->GetTypeName()); + EXPECT_EQ("std::optional", f->GetTypeAlias()); + } + + { + std::array, 2> items; + items[0] = GetDouble32Item(); + items[1] = std::make_unique>("f"); + auto f = std::make_unique("f", ROOT::RMapField::EMapType::kMultiMap, + std::make_unique("f", std::move(items))); + EXPECT_EQ("std::multimap", f->GetTypeName()); + EXPECT_EQ("std::multimap", f->GetTypeAlias()); + } + + { + auto f = std::make_unique("f", GetDouble32Item()); + EXPECT_EQ("std::atomic", f->GetTypeName()); + EXPECT_EQ("std::atomic", f->GetTypeAlias()); + } +}