Skip to content

Commit 4d80ea9

Browse files
jblomerjblomer
committed
[ntuple] remove type name from map field constructor
1 parent 6047b7e commit 4d80ea9

File tree

4 files changed

+38
-48
lines changed

4 files changed

+38
-48
lines changed

‎tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,13 @@ public:
281281
/// The generic field for a `std::map<KeyType, ValueType>` and `std::unordered_map<KeyType, ValueType>`
282282
class RMapField : public RProxiedCollectionField {
283283
public:
284-
RMapField(std::string_view fieldName, std::string_view typeName, std::unique_ptr<RFieldBase> itemField);
284+
enum class EMapType {
285+
kMap,
286+
kUnorderedMap,
287+
kMultiMap,
288+
kUnorderedMultiMap
289+
};
290+
RMapField(std::string_view fieldName, EMapType mapType, std::unique_ptr<RFieldBase> itemField);
285291
RMapField(RMapField &&other) = default;
286292
RMapField &operator=(RMapField &&other) = default;
287293
~RMapField() override = default;
@@ -296,7 +302,7 @@ public:
296302
}
297303

298304
explicit RField(std::string_view name)
299-
: RMapField(name, TypeName(), std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
305+
: RMapField(name, EMapType::kMap, std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
300306
{
301307
}
302308
RField(RField &&other) = default;
@@ -313,7 +319,7 @@ public:
313319
}
314320

315321
explicit RField(std::string_view name)
316-
: RMapField(name, TypeName(), std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
322+
: RMapField(name, EMapType::kUnorderedMap, std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
317323
{
318324
}
319325
RField(RField &&other) = default;
@@ -330,7 +336,7 @@ public:
330336
}
331337

332338
explicit RField(std::string_view name)
333-
: RMapField(name, TypeName(), std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
339+
: RMapField(name, EMapType::kMultiMap, std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
334340
{
335341
}
336342
RField(RField &&other) = default;
@@ -347,7 +353,7 @@ public:
347353
}
348354

349355
explicit RField(std::string_view name)
350-
: RMapField(name, TypeName(), std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
356+
: RMapField(name, EMapType::kUnorderedMultiMap, std::make_unique<RField<std::pair<KeyT, ValueT>>>("_0"))
351357
{
352358
}
353359
RField(RField &&other) = default;

‎tree/ntuple/src/RFieldBase.cxx

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -445,67 +445,35 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa
445445
if (innerTypes.size() != 2) {
446446
return R__FORWARD_RESULT(fnFail("the type list for std::map must have exactly two elements"));
447447
}
448-
449448
auto itemField =
450449
Create("_0", "std::pair<" + innerTypes[0] + "," + innerTypes[1] + ">", options, desc, maybeGetChildId(0))
451450
.Unwrap();
452-
453-
// We use the type names of subfields of the newly created item fields to create the map's type name to
454-
// ensure the inner type names are properly normalized.
455-
auto keyTypeName = itemField->GetConstSubfields()[0]->GetTypeName();
456-
auto valueTypeName = itemField->GetConstSubfields()[1]->GetTypeName();
457-
458-
result = std::make_unique<RMapField>(fieldName, "std::map<" + keyTypeName + "," + valueTypeName + ">",
459-
std::move(itemField));
451+
result = std::make_unique<RMapField>(fieldName, RMapField::EMapType::kMap, std::move(itemField));
460452
} else if (resolvedType.substr(0, 19) == "std::unordered_map<") {
461453
auto innerTypes = TokenizeTypeList(resolvedType.substr(19, resolvedType.length() - 20));
462454
if (innerTypes.size() != 2)
463455
return R__FORWARD_RESULT(fnFail("the type list for std::unordered_map must have exactly two elements"));
464-
465456
auto itemField =
466457
Create("_0", "std::pair<" + innerTypes[0] + "," + innerTypes[1] + ">", options, desc, maybeGetChildId(0))
467458
.Unwrap();
468-
469-
// We use the type names of subfields of the newly created item fields to create the map's type name to
470-
// ensure the inner type names are properly normalized.
471-
auto keyTypeName = itemField->GetConstSubfields()[0]->GetTypeName();
472-
auto valueTypeName = itemField->GetConstSubfields()[1]->GetTypeName();
473-
474-
result = std::make_unique<RMapField>(
475-
fieldName, "std::unordered_map<" + keyTypeName + "," + valueTypeName + ">", std::move(itemField));
459+
result = std::make_unique<RMapField>(fieldName, RMapField::EMapType::kUnorderedMap, std::move(itemField));
476460
} else if (resolvedType.substr(0, 14) == "std::multimap<") {
477461
auto innerTypes = TokenizeTypeList(resolvedType.substr(14, resolvedType.length() - 15));
478462
if (innerTypes.size() != 2)
479463
return R__FORWARD_RESULT(fnFail("the type list for std::multimap must have exactly two elements"));
480-
481464
auto itemField =
482465
Create("_0", "std::pair<" + innerTypes[0] + "," + innerTypes[1] + ">", options, desc, maybeGetChildId(0))
483466
.Unwrap();
484-
485-
// We use the type names of subfields of the newly created item fields to create the map's type name to
486-
// ensure the inner type names are properly normalized.
487-
auto keyTypeName = itemField->GetConstSubfields()[0]->GetTypeName();
488-
auto valueTypeName = itemField->GetConstSubfields()[1]->GetTypeName();
489-
490-
result = std::make_unique<RMapField>(fieldName, "std::multimap<" + keyTypeName + "," + valueTypeName + ">",
491-
std::move(itemField));
467+
result = std::make_unique<RMapField>(fieldName, RMapField::EMapType::kMultiMap, std::move(itemField));
492468
} else if (resolvedType.substr(0, 24) == "std::unordered_multimap<") {
493469
auto innerTypes = TokenizeTypeList(resolvedType.substr(24, resolvedType.length() - 25));
494470
if (innerTypes.size() != 2)
495471
return R__FORWARD_RESULT(
496472
fnFail("the type list for std::unordered_multimap must have exactly two elements"));
497-
498473
auto itemField =
499474
Create("_0", "std::pair<" + innerTypes[0] + "," + innerTypes[1] + ">", options, desc, maybeGetChildId(0))
500475
.Unwrap();
501-
502-
// We use the type names of subfields of the newly created item fields to create the map's type name to
503-
// ensure the inner type names are properly normalized.
504-
auto keyTypeName = itemField->GetConstSubfields()[0]->GetTypeName();
505-
auto valueTypeName = itemField->GetConstSubfields()[1]->GetTypeName();
506-
507-
result = std::make_unique<RMapField>(
508-
fieldName, "std::unordered_multimap<" + keyTypeName + "," + valueTypeName + ">", std::move(itemField));
476+
result = std::make_unique<RMapField>(fieldName, RMapField::EMapType::kUnorderedMultiMap, std::move(itemField));
509477
} else if (resolvedType.substr(0, 12) == "std::atomic<") {
510478
std::string itemTypeName = resolvedType.substr(12, resolvedType.length() - 13);
511479
auto itemField = Create("_0", itemTypeName, options, desc, maybeGetChildId(0)).Unwrap();

‎tree/ntuple/src/RFieldMeta.cxx

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,24 @@ std::string BuildSetTypeName(ROOT::RSetField::ESetType setType, const ROOT::RFie
8484
return typePrefix + innerField.GetTypeName() + ">";
8585
}
8686

87+
std::string BuildMapTypeName(ROOT::RMapField::EMapType mapType, const ROOT::RFieldBase *innerField)
88+
{
89+
if (const auto pairField = dynamic_cast<const ROOT::RPairField *>(innerField)) {
90+
std::string typePrefix;
91+
switch (mapType) {
92+
case ROOT::RMapField::EMapType::kMap: typePrefix = "std::map<"; break;
93+
case ROOT::RMapField::EMapType::kUnorderedMap: typePrefix = "std::unordered_map<"; break;
94+
case ROOT::RMapField::EMapType::kMultiMap: typePrefix = "std::multimap<"; break;
95+
case ROOT::RMapField::EMapType::kUnorderedMultiMap: typePrefix = "std::unordered_multimap<"; break;
96+
default: R__ASSERT(false);
97+
}
98+
auto subFields = pairField->GetConstSubfields();
99+
return typePrefix + subFields[0]->GetTypeName() + "," + subFields[1]->GetTypeName() + ">";
100+
}
101+
102+
throw ROOT::RException(R__FAIL("RMapField inner field type must be of RPairField"));
103+
}
104+
87105
} // anonymous namespace
88106

89107
ROOT::RClassField::RClassField(std::string_view fieldName, const RClassField &source)
@@ -820,12 +838,9 @@ void ROOT::RProxiedCollectionField::AcceptVisitor(ROOT::Detail::RFieldVisitor &v
820838

821839
//------------------------------------------------------------------------------
822840

823-
ROOT::RMapField::RMapField(std::string_view fieldName, std::string_view typeName, std::unique_ptr<RFieldBase> itemField)
824-
: RProxiedCollectionField(fieldName, EnsureValidClass(typeName))
841+
ROOT::RMapField::RMapField(std::string_view fieldName, EMapType mapType, std::unique_ptr<RFieldBase> itemField)
842+
: RProxiedCollectionField(fieldName, EnsureValidClass(BuildMapTypeName(mapType, itemField.get())))
825843
{
826-
if (!dynamic_cast<RPairField *>(itemField.get()))
827-
throw RException(R__FAIL("RMapField inner field type must be of RPairField"));
828-
829844
auto *itemClass = fProxy->GetValueClass();
830845
fItemSize = itemClass->GetClassSize();
831846

‎tree/ntuple/test/ntuple_types.cxx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,9 @@ TEST(RNTuple, StdMap)
586586
EXPECT_THROW(RFieldBase::Create("myInvalidMap", "std::map<char, std::string, int>").Unwrap(), ROOT::RException);
587587

588588
auto invalidInnerField = RFieldBase::Create("someIntField", "int").Unwrap();
589-
EXPECT_THROW(std::make_unique<ROOT::RMapField>("myInvalidMap", "std::map<char, int>", std::move(invalidInnerField)),
590-
ROOT::RException);
589+
EXPECT_THROW(
590+
std::make_unique<ROOT::RMapField>("myInvalidMap", ROOT::RMapField::EMapType::kMap, std::move(invalidInnerField)),
591+
ROOT::RException);
591592

592593
FileRaii fileGuard("test_ntuple_rfield_stdmap.root");
593594
{

0 commit comments

Comments
 (0)