Skip to content

Commit 47d2528

Browse files
jblomerjblomer
committed
[ntuple] use RResult in field reconciliation
Let EnsureMatchingOnDiskField() and EnsureMatchingTypePrefix() return an RResult<void> instead of directly throwing.
1 parent 50551c3 commit 47d2528

File tree

5 files changed

+35
-32
lines changed

5 files changed

+35
-32
lines changed

‎tree/ntuple/inc/ROOT/RFieldBase.hxx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -532,10 +532,11 @@ protected:
532532
std::uint32_t CompareOnDiskField(const RFieldDescriptor &fieldDesc, std::uint32_t ignoreBits) const;
533533
/// Compares the field to the provieded on-disk field descriptor. Throws an exception if the fields don't match.
534534
/// Optionally, a set of bits can be provided that should be ignored in the comparison.
535-
void EnsureMatchingOnDiskField(const RFieldDescriptor &fieldDesc, std::uint32_t ignoreBits = 0) const;
535+
RResult<void> EnsureMatchingOnDiskField(const RFieldDescriptor &fieldDesc, std::uint32_t ignoreBits = 0) const;
536536
/// Many fields accept a range of type prefixes for schema evolution,
537537
/// e.g. std::unique_ptr< and std::optional< for nullable fields
538-
void EnsureMatchingTypePrefix(const RFieldDescriptor &fieldDesc, const std::vector<std::string> &prefixes) const;
538+
RResult<void>
539+
EnsureMatchingTypePrefix(const RFieldDescriptor &fieldDesc, const std::vector<std::string> &prefixes) const;
539540

540541
/// Factory method to resurrect a field from the stored on-disk type information. This overload takes an already
541542
/// normalized type name and type alias.

‎tree/ntuple/src/RField.cxx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ void ROOT::RCardinalityField::GenerateColumns(const ROOT::RNTupleDescriptor &des
7878
void ROOT::RCardinalityField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
7979
{
8080
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
81-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeVersion | kDiffStructure | kDiffTypeName);
81+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeVersion | kDiffStructure | kDiffTypeName).ThrowOnError();
8282
if (fieldDesc.GetStructure() == ENTupleStructure::kPlain) {
8383
if (fieldDesc.GetTypeName().rfind("ROOT::RNTupleCardinality<", 0) != 0) {
8484
throw RException(R__FAIL("RCardinalityField " + GetQualifiedFieldName() +
@@ -664,7 +664,7 @@ void ROOT::RRecordField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
664664
R__ASSERT(GetTypeName().empty());
665665

666666
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
667-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName | kDiffTypeVersion);
667+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName | kDiffTypeVersion).ThrowOnError();
668668

669669
// The on-disk ID of subfields is matched by field name. So we inherently support reordering of fields
670670
// and we will ignore extra on-disk fields.
@@ -879,8 +879,8 @@ void ROOT::RNullableField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
879879
static const std::vector<std::string> prefixes = {"std::optional<", "std::unique_ptr<"};
880880

881881
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
882-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
883-
EnsureMatchingTypePrefix(fieldDesc, prefixes);
882+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();
883+
EnsureMatchingTypePrefix(fieldDesc, prefixes).ThrowOnError();
884884
}
885885

886886
ROOT::RNTupleLocalIndex ROOT::RNullableField::GetItemIndex(ROOT::NTupleSize_t globalIndex)
@@ -1099,8 +1099,8 @@ void ROOT::RAtomicField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
10991099
static const std::vector<std::string> prefixes = {"std::atomic<"};
11001100

11011101
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
1102-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
1103-
EnsureMatchingTypePrefix(fieldDesc, prefixes);
1102+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();
1103+
EnsureMatchingTypePrefix(fieldDesc, prefixes).ThrowOnError();
11041104
}
11051105

11061106
std::vector<ROOT::RFieldBase::RValue> ROOT::RAtomicField::SplitValue(const RValue &value) const

‎tree/ntuple/src/RFieldBase.cxx‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,14 +1018,15 @@ void ROOT::RFieldBase::ReconcileOnDiskField(const RNTupleDescriptor &desc)
10181018
// The default implementation throws an exception if the on-disk ID is set and there are any meaningful differences
10191019
// to the on-disk field. Derived classes may overwrite this and relax the checks to support automatic schema
10201020
// evolution.
1021-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(fOnDiskId));
1021+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(fOnDiskId)).ThrowOnError();
10221022
}
10231023

1024-
void ROOT::RFieldBase::EnsureMatchingOnDiskField(const RFieldDescriptor &fieldDesc, std::uint32_t ignoreBits) const
1024+
ROOT::RResult<void>
1025+
ROOT::RFieldBase::EnsureMatchingOnDiskField(const RFieldDescriptor &fieldDesc, std::uint32_t ignoreBits) const
10251026
{
10261027
const std::uint32_t diffBits = CompareOnDiskField(fieldDesc, ignoreBits);
10271028
if (diffBits == 0)
1028-
return;
1029+
return RResult<void>::Success();
10291030

10301031
std::ostringstream errMsg;
10311032
errMsg << "in-memory field " << GetQualifiedFieldName() << " of type " << GetTypeName() << " is incompatible "
@@ -1045,17 +1046,17 @@ void ROOT::RFieldBase::EnsureMatchingOnDiskField(const RFieldDescriptor &fieldDe
10451046
if (diffBits & kDiffNRepetitions) {
10461047
errMsg << " repetition count " << GetNRepetitions() << " vs. " << fieldDesc.GetNRepetitions() << ";";
10471048
}
1048-
throw RException(R__FAIL(errMsg.str()));
1049+
return R__FAIL(errMsg.str());
10491050
}
10501051

1051-
void ROOT::RFieldBase::EnsureMatchingTypePrefix(const RFieldDescriptor &fieldDesc,
1052-
const std::vector<std::string> &prefixes) const
1052+
ROOT::RResult<void> ROOT::RFieldBase::EnsureMatchingTypePrefix(const RFieldDescriptor &fieldDesc,
1053+
const std::vector<std::string> &prefixes) const
10531054
{
10541055
for (const auto &p : prefixes) {
10551056
if (fieldDesc.GetTypeName().rfind(p, 0) == 0)
1056-
return;
1057+
return RResult<void>::Success();
10571058
}
1058-
throw RException(R__FAIL("incompatible type " + fieldDesc.GetTypeName() + " for field " + GetQualifiedFieldName()));
1059+
return R__FAIL("incompatible type " + fieldDesc.GetTypeName() + " for field " + GetQualifiedFieldName());
10591060
}
10601061

10611062
std::uint32_t ROOT::RFieldBase::CompareOnDiskField(const RFieldDescriptor &fieldDesc, std::uint32_t ignoreBits) const

‎tree/ntuple/src/RFieldMeta.cxx‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ std::unique_ptr<ROOT::RFieldBase> ROOT::RClassField::BeforeConnectPageSource(ROO
513513

514514
void ROOT::RClassField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
515515
{
516-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeVersion | kDiffTypeName);
516+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeVersion | kDiffTypeName).ThrowOnError();
517517
}
518518

519519
void ROOT::RClassField::ConstructValue(void *where) const
@@ -611,7 +611,7 @@ std::unique_ptr<ROOT::RFieldBase> ROOT::REnumField::CloneImpl(std::string_view n
611611
void ROOT::REnumField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
612612
{
613613
// TODO(jblomer): allow enum to enum conversion only by rename rule
614-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName | kDiffTypeVersion);
614+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName | kDiffTypeVersion).ThrowOnError();
615615
}
616616

617617
std::vector<ROOT::RFieldBase::RValue> ROOT::REnumField::SplitValue(const RValue &value) const
@@ -677,8 +677,8 @@ void ROOT::RPairField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
677677
static const std::vector<std::string> prefixes = {"std::pair<", "std::tuple<"};
678678

679679
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
680-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
681-
EnsureMatchingTypePrefix(fieldDesc, prefixes);
680+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();
681+
EnsureMatchingTypePrefix(fieldDesc, prefixes).ThrowOnError();
682682

683683
const auto nOnDiskSubfields = fieldDesc.GetLinkIds().size();
684684
if (nOnDiskSubfields != 2) {
@@ -822,7 +822,7 @@ void ROOT::RProxiedCollectionField::GenerateColumns(const ROOT::RNTupleDescripto
822822

823823
void ROOT::RProxiedCollectionField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
824824
{
825-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName);
825+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName).ThrowOnError();
826826
}
827827

828828
void ROOT::RProxiedCollectionField::ConstructValue(void *where) const
@@ -989,7 +989,7 @@ std::unique_ptr<ROOT::RFieldBase> ROOT::RStreamerField::BeforeConnectPageSource(
989989

990990
void ROOT::RStreamerField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
991991
{
992-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName | kDiffTypeVersion);
992+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName | kDiffTypeVersion).ThrowOnError();
993993
}
994994

995995
void ROOT::RStreamerField::ConstructValue(void *where) const
@@ -1224,8 +1224,8 @@ void ROOT::RTupleField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
12241224
static const std::vector<std::string> prefixes = {"std::pair<", "std::tuple<"};
12251225

12261226
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
1227-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
1228-
EnsureMatchingTypePrefix(fieldDesc, prefixes);
1227+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();
1228+
EnsureMatchingTypePrefix(fieldDesc, prefixes).ThrowOnError();
12291229

12301230
const auto nOnDiskSubfields = fieldDesc.GetLinkIds().size();
12311231
const auto nSubfields = fSubfields.size();
@@ -1386,8 +1386,8 @@ void ROOT::RVariantField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
13861386
static const std::vector<std::string> prefixes = {"std::variant<"};
13871387

13881388
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
1389-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
1390-
EnsureMatchingTypePrefix(fieldDesc, prefixes);
1389+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();
1390+
EnsureMatchingTypePrefix(fieldDesc, prefixes).ThrowOnError();
13911391

13921392
if (fSubfields.size() != fieldDesc.GetLinkIds().size()) {
13931393
throw RException(R__FAIL("number of variants on-disk do not match for " + GetQualifiedFieldName()));

‎tree/ntuple/src/RFieldSequenceContainer.cxx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ void ROOT::RArrayField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
8080
static const std::vector<std::string> prefixes = {"std::array<"};
8181

8282
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
83-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName);
84-
EnsureMatchingTypePrefix(fieldDesc, prefixes);
83+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName).ThrowOnError();
84+
EnsureMatchingTypePrefix(fieldDesc, prefixes).ThrowOnError();
8585
}
8686

8787
void ROOT::RArrayField::ConstructValue(void *where) const
@@ -442,7 +442,7 @@ std::unique_ptr<ROOT::RFieldBase> ROOT::RRVecField::BeforeConnectPageSource(Inte
442442

443443
void ROOT::RRVecField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
444444
{
445-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName);
445+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName).ThrowOnError();
446446
}
447447

448448
void ROOT::RRVecField::ConstructValue(void *where) const
@@ -624,7 +624,7 @@ void ROOT::RVectorField::GenerateColumns(const ROOT::RNTupleDescriptor &desc)
624624

625625
void ROOT::RVectorField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
626626
{
627-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName);
627+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName).ThrowOnError();
628628
}
629629

630630
void ROOT::RVectorField::RVectorDeleter::operator()(void *objPtr, bool dtorOnly)
@@ -728,7 +728,7 @@ void ROOT::RField<std::vector<bool>>::GenerateColumns(const ROOT::RNTupleDescrip
728728

729729
void ROOT::RField<std::vector<bool>>::ReconcileOnDiskField(const RNTupleDescriptor &desc)
730730
{
731-
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName);
731+
EnsureMatchingOnDiskField(desc.GetFieldDescriptor(GetOnDiskId()), kDiffTypeName).ThrowOnError();
732732
}
733733

734734
std::vector<ROOT::RFieldBase::RValue> ROOT::RField<std::vector<bool>>::SplitValue(const RValue &value) const
@@ -828,7 +828,8 @@ void ROOT::RArrayAsRVecField::ReadInClusterImpl(RNTupleLocalIndex localIndex, vo
828828
void ROOT::RArrayAsRVecField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
829829
{
830830
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
831-
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName | kDiffTypeVersion | kDiffStructure | kDiffNRepetitions);
831+
EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName | kDiffTypeVersion | kDiffStructure | kDiffNRepetitions)
832+
.ThrowOnError();
832833
if (fieldDesc.GetTypeName().rfind("std::array<", 0) != 0) {
833834
throw RException(R__FAIL("RArrayAsRVecField " + GetQualifiedFieldName() + " expects an on-disk array field"));
834835
}

0 commit comments

Comments
 (0)