From b40d5de8488d2bc771758f25f3cc65324981058b Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Tue, 26 May 2026 23:12:47 +0200 Subject: [PATCH 01/37] [ntuple] fix argument comment --- tree/ntuple/src/RMiniFile.cxx | 12 ++++++------ tree/ntuple/src/RPageStorageFile.cxx | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 35f28cb666b71..fa4ae6d3c014b 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -1221,8 +1221,8 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::RImplRFile::ReserveBlobKey(size //////////////////////////////////////////////////////////////////////////////// -ROOT::Internal::RNTupleFileWriter::RNTupleFileWriter(std::string_view name, std::uint64_t maxKeySize, bool hidden) - : fIsHidden(hidden), fNTupleName(name) +ROOT::Internal::RNTupleFileWriter::RNTupleFileWriter(std::string_view name, std::uint64_t maxKeySize, bool isHidden) + : fIsHidden(isHidden), fNTupleName(name) { auto &fileSimple = fFile.emplace(); fileSimple.fControlBlock = std::make_unique(); @@ -1269,8 +1269,8 @@ ROOT::Internal::RNTupleFileWriter::Recreate(std::string_view ntupleName, std::st // RNTupleFileWriter::RImplSimple does its own buffering, turn off additional buffering from C stdio. std::setvbuf(fileStream, nullptr, _IONBF, 0); - auto writer = - std::unique_ptr(new RNTupleFileWriter(ntupleName, options.GetMaxKeySize(), /*hidden=*/false)); + auto writer = std::unique_ptr( + new RNTupleFileWriter(ntupleName, options.GetMaxKeySize(), /*isHidden=*/false)); RImplSimple &fileSimple = std::get(writer->fFile); fileSimple.fFile = fileStream; fileSimple.fDirectIO = options.GetUseDirectIO(); @@ -1309,7 +1309,7 @@ std::unique_ptr ROOT::Internal::RNTupleFileWriter::Append(std::string_view ntupleName, ROOT::Experimental::RFile &file, std::string_view ntupleDir, std::uint64_t maxKeySize) { - auto writer = std::unique_ptr(new RNTupleFileWriter(ntupleName, maxKeySize, /*hidden=*/false)); + auto writer = std::unique_ptr(new RNTupleFileWriter(ntupleName, maxKeySize, /*isHidden=*/false)); auto &rfile = writer->fFile.emplace(); rfile.fFile = &file; R__ASSERT(ntupleDir.empty() || ntupleDir[ntupleDir.size() - 1] == '/'); @@ -1321,7 +1321,7 @@ std::unique_ptr ROOT::Internal::RNTupleFileWriter::CloneAsHidden(std::string_view ntupleName) const { if (auto *file = std::get_if(&fFile)) { - return Append(ntupleName, *file->fDirectory, fNTupleAnchor.fMaxKeySize, /* hidden= */ true); + return Append(ntupleName, *file->fDirectory, fNTupleAnchor.fMaxKeySize, /* isHidden= */ true); } // TODO: support also non-TFile-based writers throw ROOT::RException(R__FAIL("cannot clone a non-TFile-based RNTupleFileWriter.")); diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 522727e52d089..2d0c1b6681f9d 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -74,7 +74,7 @@ ROOT::Internal::RPageSinkFile::RPageSinkFile(std::string_view ntupleName, TDirec const ROOT::RNTupleWriteOptions &options) : RPageSinkFile(ntupleName, options) { - fWriter = RNTupleFileWriter::Append(ntupleName, fileOrDirectory, options.GetMaxKeySize(), /*hidden=*/false); + fWriter = RNTupleFileWriter::Append(ntupleName, fileOrDirectory, options.GetMaxKeySize(), /*isHidden=*/false); } ROOT::Internal::RPageSinkFile::RPageSinkFile(std::string_view ntupleName, ROOT::Experimental::RFile &file, From 5a005fced8824866c11373e61abdd0cbd5ef66ab Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Tue, 26 May 2026 23:28:10 +0200 Subject: [PATCH 02/37] [io] fix casting through void --- core/base/inc/TBuffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/base/inc/TBuffer.h b/core/base/inc/TBuffer.h index 7e90a50178ef9..eb25fc6f84649 100644 --- a/core/base/inc/TBuffer.h +++ b/core/base/inc/TBuffer.h @@ -390,7 +390,7 @@ template TBuffer &operator>>(TBuffer &buf, Tmpl *&obj) // since the pointer could be zero (so typeid(*obj) is not usable). auto cl = TClass::GetClass(); - obj = (Tmpl *) ( (void*) buf.ReadObjectAny(cl) ); + obj = reinterpret_cast(buf.ReadObjectAny(cl)); return buf; } From e522b41e9b3808bd7d18343124825347c2c595e0 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 10:33:19 +0200 Subject: [PATCH 03/37] [RVec] avoid unnecessary capacity check in internal set_size --- math/vecops/inc/ROOT/RVec.hxx | 100 +++++++++++++++++----------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index fdf9d42d011f0..4e6bf5063bb5d 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -172,6 +172,8 @@ protected: /// If false, the RVec is in "memory adoption" mode, i.e. it is acting as a view on a memory buffer it does not own. bool Owns() const { return fCapacity != -1; } + void SetSizeUnchecked(std::size_t N) { fSize = N; } + public: size_t size() const { return fSize; } size_t capacity() const noexcept { return Owns() ? fCapacity : fSize; } @@ -192,7 +194,7 @@ public: if (N > capacity()) { throw std::runtime_error("Setting size to a value greater than capacity."); } - fSize = N; + SetSizeUnchecked(N); } }; @@ -366,7 +368,7 @@ public: if (R__unlikely(this->size() >= this->capacity())) this->grow(); ::new ((void *)this->end()) T(Elt); - this->set_size(this->size() + 1); + this->SetSizeUnchecked(this->size() + 1); } void push_back(T &&Elt) @@ -374,12 +376,12 @@ public: if (R__unlikely(this->size() >= this->capacity())) this->grow(); ::new ((void *)this->end()) T(::std::move(Elt)); - this->set_size(this->size() + 1); + this->SetSizeUnchecked(this->size() + 1); } void pop_back() { - this->set_size(this->size() - 1); + this->SetSizeUnchecked(this->size() - 1); this->end()->~T(); } }; @@ -487,10 +489,10 @@ public: if (R__unlikely(this->size() >= this->capacity())) this->grow(); memcpy(reinterpret_cast(this->end()), &Elt, sizeof(T)); - this->set_size(this->size() + 1); + this->SetSizeUnchecked(this->size() + 1); } - void pop_back() { this->set_size(this->size() - 1); } + void pop_back() { this->SetSizeUnchecked(this->size() - 1); } }; /// Storage for the SmallVector elements. This is specialized for the N=0 case @@ -595,13 +597,13 @@ public: if (N < this->size()) { if (this->Owns()) this->destroy_range(this->begin() + N, this->end()); - this->set_size(N); + this->SetSizeUnchecked(N); } else if (N > this->size()) { if (this->capacity() < N) this->grow(N); for (auto I = this->end(), E = this->begin() + N; I != E; ++I) new (&*I) T(); - this->set_size(N); + this->SetSizeUnchecked(N); } } @@ -610,12 +612,12 @@ public: if (N < this->size()) { if (this->Owns()) this->destroy_range(this->begin() + N, this->end()); - this->set_size(N); + this->SetSizeUnchecked(N); } else if (N > this->size()) { if (this->capacity() < N) this->grow(N); std::uninitialized_fill(this->end(), this->begin() + N, NV); - this->set_size(N); + this->SetSizeUnchecked(N); } } @@ -632,7 +634,7 @@ public: } if (this->Owns()) this->destroy_range(this->end() - NumItems, this->end()); - this->set_size(this->size() - NumItems); + this->SetSizeUnchecked(this->size() - NumItems); } R__RVEC_NODISCARD T pop_back_val() @@ -655,7 +657,7 @@ public: this->grow(this->size() + NumInputs); this->uninitialized_copy(in_start, in_end, this->end()); - this->set_size(this->size() + NumInputs); + this->SetSizeUnchecked(this->size() + NumInputs); } /// Append \p NumInputs copies of \p Elt to the end. @@ -665,7 +667,7 @@ public: this->grow(this->size() + NumInputs); std::uninitialized_fill_n(this->end(), NumInputs, Elt); - this->set_size(this->size() + NumInputs); + this->SetSizeUnchecked(this->size() + NumInputs); } void append(std::initializer_list IL) { append(IL.begin(), IL.end()); } @@ -679,7 +681,7 @@ public: clear(); if (this->capacity() < NumElts) this->grow(NumElts); - this->set_size(NumElts); + this->SetSizeUnchecked(NumElts); std::uninitialized_fill(this->begin(), this->end(), Elt); } @@ -731,7 +733,7 @@ public: // Drop the last elts. if (this->Owns()) this->destroy_range(I, this->end()); - this->set_size(I - this->begin()); + this->SetSizeUnchecked(I - this->begin()); return (N); } @@ -755,7 +757,7 @@ public: ::new ((void *)this->end()) T(::std::move(this->back())); // Push everything else over. std::move_backward(I, this->end() - 1, this->end()); - this->set_size(this->size() + 1); + this->SetSizeUnchecked(this->size() + 1); // If we just moved the element we're inserting, be sure to update // the reference. @@ -786,7 +788,7 @@ public: ::new ((void *)this->end()) T(std::move(this->back())); // Push everything else over. std::move_backward(I, this->end() - 1, this->end()); - this->set_size(this->size() + 1); + this->SetSizeUnchecked(this->size() + 1); // If we just moved the element we're inserting, be sure to update // the reference. @@ -838,7 +840,7 @@ public: // Move over the elements that we're about to overwrite. T *OldEnd = this->end(); - this->set_size(this->size() + NumToInsert); + this->SetSizeUnchecked(this->size() + NumToInsert); size_t NumOverwritten = OldEnd - I; this->uninitialized_move(I, OldEnd, this->end() - NumOverwritten); @@ -895,7 +897,7 @@ public: // Move over the elements that we're about to overwrite. T *OldEnd = this->end(); - this->set_size(this->size() + NumToInsert); + this->SetSizeUnchecked(this->size() + NumToInsert); size_t NumOverwritten = OldEnd - I; this->uninitialized_move(I, OldEnd, this->end() - NumOverwritten); @@ -919,7 +921,7 @@ public: if (R__unlikely(this->size() >= this->capacity())) this->grow(); ::new ((void *)this->end()) T(std::forward(Args)...); - this->set_size(this->size() + 1); + this->SetSizeUnchecked(this->size() + 1); return this->back(); } @@ -974,17 +976,17 @@ void RVecImpl::swap(RVecImpl &RHS) if (this->size() > RHS.size()) { size_t EltDiff = this->size() - RHS.size(); this->uninitialized_copy(this->begin() + NumShared, this->end(), RHS.end()); - RHS.set_size(RHS.size() + EltDiff); + RHS.SetSizeUnchecked(RHS.size() + EltDiff); if (this->Owns()) this->destroy_range(this->begin() + NumShared, this->end()); - this->set_size(NumShared); + this->SetSizeUnchecked(NumShared); } else if (RHS.size() > this->size()) { size_t EltDiff = RHS.size() - this->size(); this->uninitialized_copy(RHS.begin() + NumShared, RHS.end(), this->end()); - this->set_size(this->size() + EltDiff); + this->SetSizeUnchecked(this->size() + EltDiff); if (RHS.Owns()) this->destroy_range(RHS.begin() + NumShared, RHS.end()); - RHS.set_size(NumShared); + RHS.SetSizeUnchecked(NumShared); } } @@ -1012,7 +1014,7 @@ RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) this->destroy_range(NewEnd, this->end()); // Trim. - this->set_size(RHSSize); + this->SetSizeUnchecked(RHSSize); return *this; } @@ -1025,7 +1027,7 @@ RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) // Destroy current elements. this->destroy_range(this->begin(), this->end()); } - this->set_size(0); + this->SetSizeUnchecked(0); CurSize = 0; this->grow(RHSSize); } else if (CurSize) { @@ -1037,7 +1039,7 @@ RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) this->uninitialized_copy(RHS.begin() + CurSize, RHS.end(), this->begin() + CurSize); // Set end. - this->set_size(RHSSize); + this->SetSizeUnchecked(RHSSize); return *this; } @@ -1075,7 +1077,7 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) // Destroy excess elements and trim the bounds. if (this->Owns()) this->destroy_range(NewEnd, this->end()); - this->set_size(RHSSize); + this->SetSizeUnchecked(RHSSize); // Clear the RHS. RHS.clear(); @@ -1093,7 +1095,7 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) // Destroy current elements. this->destroy_range(this->begin(), this->end()); } - this->set_size(0); + this->SetSizeUnchecked(0); CurSize = 0; this->grow(RHSSize); } else if (CurSize) { @@ -1105,7 +1107,7 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) this->uninitialized_move(RHS.begin() + CurSize, RHS.end(), this->begin() + CurSize); // Set end. - this->set_size(RHSSize); + this->SetSizeUnchecked(RHSSize); RHS.clear(); return *this; @@ -1399,7 +1401,7 @@ RVec v2 {5.f,6.f,7.f,8.f}; auto v3 = v1+v2; auto v4 = 3 * v1; ~~~ -The supported operators are +The supported operators are - +, -, *, / - +=, -=, *=, /= - <, >, ==, !=, <=, >=, &&, || @@ -1408,7 +1410,7 @@ The supported operators are - &=, |=, ^= - <<=, >>= -The most common mathematical functions are supported. It is possible to invoke them passing +The most common mathematical functions are supported. It is possible to invoke them passing RVecs as arguments. - abs, fdim, fmod, remainder - floor, ceil, trunc, round, lround, llround @@ -3027,13 +3029,13 @@ Common_t Angle(T0 x1, T1 y1, T2 z1, T3 x2, T4 y2, T5 z2){ const auto cx = y1 * z2 - y2 * z1; const auto cy = x1 * z2 - x2 * z1; const auto cz = x1 * y2 - x2 * y1; - + // norm of cross product const auto c = std::sqrt(cx * cx + cy * cy + cz * cz); - + // dot product const auto d = x1 * x2 + y1 * y2 + z1 * z2; - + return std::atan2(c, d); } @@ -3057,7 +3059,7 @@ Common_t InvariantMasses_PxPyPzM( return (mass1 + mass2); if (p1_sq <= 0) { auto mm = mass1 + std::sqrt(mass2*mass2 + p2_sq); - auto m2 = mm*mm - p2_sq; + auto m2 = mm*mm - p2_sq; if (m2 >= 0) return std::sqrt( m2 ); else @@ -3065,7 +3067,7 @@ Common_t InvariantMasses_PxPyPzM( } if (p2_sq <= 0) { auto mm = mass2 + std::sqrt(mass1*mass1 + p1_sq); - auto m2 = mm*mm - p1_sq; + auto m2 = mm*mm - p1_sq; if (m2 >= 0) return std::sqrt( m2 ); else @@ -3142,11 +3144,11 @@ RVec InvariantMasses( const auto x1 = pt1[i] * std::cos(phi1[i]); const auto y1 = pt1[i] * std::sin(phi1[i]); const auto z1 = pt1[i] * std::sinh(eta1[i]); - + const auto x2 = pt2[i] * std::cos(phi2[i]); const auto y2 = pt2[i] * std::sin(phi2[i]); const auto z2 = pt2[i] * std::sinh(eta2[i]); - + // Numerically stable computation of Invariant Masses inv_masses[i] = InvariantMasses_PxPyPzM(x1, y1, z1, mass1[i], x2, y2, z2, mass2[i]); } @@ -3300,11 +3302,11 @@ inline RVec Linspace(T start, T end, unsigned long long n = 128, const bo { return {}; } - + long double step = std::is_floating_point_v ? (end - start) / static_cast(n - endpoint) : (end >= start ? static_cast(end - start) / (n - endpoint) : (static_cast(end) - start) / (n - endpoint)); - + RVec temp(n); temp[0] = std::is_floating_point_v ? static_cast(start) : std::floor(start); if constexpr (std::is_floating_point_v) @@ -3387,17 +3389,17 @@ inline RVec Logspace(T start, T end, unsigned long long n = 128, const bo return {}; } RVec temp(n); - + long double start_c = start; long double end_c = end; long double base_c = base; - + long double step = (end_c - start_c) / (n - endpoint); - + temp[0] = std::is_floating_point_v ? static_cast(std::pow(base_c, start_c)) : std::floor(std::pow(base_c, start_c)); - + if constexpr (std::is_floating_point_v) { for (unsigned long long i = 1; i < n; i++) @@ -3414,7 +3416,7 @@ inline RVec Logspace(T start, T end, unsigned long long n = 128, const bo temp[i] = std::floor(std::pow(base_c, exponent)); } } - + return temp; } @@ -3477,14 +3479,14 @@ template Arange(T start, T end, T step) { unsigned long long n = std::ceil(( end >= start ? (end - start) : static_cast(end)-start)/static_cast(step)); // Ensure floating-point division. - + if (!n || (n > std::numeric_limits::max())) // Check for invalid or absurd n. { return {}; } - + RVec temp(n); - + long double start_c = start; long double step_c = step; From 4aa95b24d1dd0cc64e7d7e83766ece28af9894f6 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 11:22:54 +0200 Subject: [PATCH 04/37] [ntuple] avoid throwing impossible exception --- tree/ntuple/src/RNTupleReader.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index e26aaba93323b..dfeb557bb2d86 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -40,9 +40,8 @@ void ROOT::RNTupleReader::RActiveEntryToken::DeactivateEntry(NTupleSize_t entryN { const auto descGuard = fPtrControlBlock->fPageSource->GetSharedDescriptorGuard(); const auto clusterId = Internal::CallFindClusterIdOn(descGuard.GetRef(), entryNumber); - - if (clusterId == kInvalidDescriptorId) - throw RException(R__FAIL(std::string("entry number ") + std::to_string(entryNumber) + " out of range")); + // We acquired the given entry number so we must be able to find it back + R__ASSERT(clusterId != kInvalidDescriptorId); auto itr = fPtrControlBlock->fActiveClusters.find(clusterId); assert(itr != fPtrControlBlock->fActiveClusters.end()); From 92fecb8cbd8074b2ee46de9243df6241af476538 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 11:38:06 +0200 Subject: [PATCH 05/37] [ntuple] don't throw in RNTupleModel::~RUpdater() --- tree/ntuple/src/RNTupleModel.cxx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleModel.cxx b/tree/ntuple/src/RNTupleModel.cxx index d326194e31af9..19c14c5886569 100644 --- a/tree/ntuple/src/RNTupleModel.cxx +++ b/tree/ntuple/src/RNTupleModel.cxx @@ -197,8 +197,15 @@ ROOT::RNTupleModel::RUpdater::~RUpdater() // If we made any changes, we should commit them because the model was already altered. // Otherwise, we _do not_ commit -- it may be that the referenced model is already expired if the // corresponding writer is already destructed. - if (!fOpenChangeset.IsEmpty()) + if (fOpenChangeset.IsEmpty()) + return; + + try { ROOT::RNTupleModel::RUpdater::CommitUpdate(); + } catch (std::runtime_error &e) { + Fatal("RNTupleModel::RUpdater::~RUpdater", "cannot commit model changes during updater destructor: %s", + e.what()); + } } void ROOT::RNTupleModel::RUpdater::BeginUpdate() From d9a8794d7bebef698a58435a8741fa197ab0e91a Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 12:51:00 +0200 Subject: [PATCH 06/37] [ntuple] safer use of RException --- tree/ntuple/src/RNTupleDescriptor.cxx | 2 +- tree/ntuple/src/RNTupleFillContext.cxx | 2 +- tree/ntuple/src/RNTupleMerger.cxx | 2 +- tree/ntuple/src/RNTupleParallelWriter.cxx | 2 +- tree/ntuple/src/RNTupleWriter.cxx | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index b1e8407b86b68..69ff62cb1fc57 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -143,7 +143,7 @@ ROOT::RFieldDescriptor::CreateField(const RNTupleDescriptor &ntplDesc, const ROO return field; } catch (const RException &ex) { if (options.GetReturnInvalidOnError()) - return std::make_unique(GetFieldName(), GetTypeName(), ex.GetError().GetReport(), + return std::make_unique(GetFieldName(), GetTypeName(), ex.what(), ROOT::RInvalidField::ECategory::kGeneric); else throw ex; diff --git a/tree/ntuple/src/RNTupleFillContext.cxx b/tree/ntuple/src/RNTupleFillContext.cxx index 03da350314b2b..7f72ef2d08833 100644 --- a/tree/ntuple/src/RNTupleFillContext.cxx +++ b/tree/ntuple/src/RNTupleFillContext.cxx @@ -44,7 +44,7 @@ ROOT::RNTupleFillContext::~RNTupleFillContext() try { FlushCluster(); } catch (const RException &err) { - R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure flushing cluster: " << err.GetError().GetReport(); + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure flushing cluster: " << err.what(); } if (!fStagedClusters.empty()) { diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 6ff1ad0ac8679..aea1e85c706ff 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -725,7 +725,7 @@ ExtendDestinationModel(std::span newFields, ROOT try { mergeData.fDestination.UpdateSchema(changeset, mergeData.fNumDstEntries); } catch (const ROOT::RException &ex) { - return R__FAIL(ex.GetError().GetReport()); + return R__FAIL(ex.what()); } commonFields.reserve(commonFields.size() + newFields.size()); diff --git a/tree/ntuple/src/RNTupleParallelWriter.cxx b/tree/ntuple/src/RNTupleParallelWriter.cxx index ce3ee96e3c557..d909f38265d0c 100644 --- a/tree/ntuple/src/RNTupleParallelWriter.cxx +++ b/tree/ntuple/src/RNTupleParallelWriter.cxx @@ -144,7 +144,7 @@ ROOT::RNTupleParallelWriter::~RNTupleParallelWriter() try { CommitDataset(); } catch (const RException &err) { - R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.GetError().GetReport(); + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.what(); } } diff --git a/tree/ntuple/src/RNTupleWriter.cxx b/tree/ntuple/src/RNTupleWriter.cxx index 4f0cd2d57f0dc..eec45995faf14 100644 --- a/tree/ntuple/src/RNTupleWriter.cxx +++ b/tree/ntuple/src/RNTupleWriter.cxx @@ -55,7 +55,7 @@ ROOT::RNTupleWriter::~RNTupleWriter() try { CommitDataset(); } catch (const RException &err) { - R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.GetError().GetReport(); + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.what(); } } From 692139d3fee1bdd8f5170d26c833a7c1e69b5aeb Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:14:16 +0200 Subject: [PATCH 07/37] [core] make RException nothrow copy constructible --- core/foundation/inc/ROOT/RError.hxx | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/core/foundation/inc/ROOT/RError.hxx b/core/foundation/inc/ROOT/RError.hxx index 1f9e491a515a6..7c6365f9f7327 100644 --- a/core/foundation/inc/ROOT/RError.hxx +++ b/core/foundation/inc/ROOT/RError.hxx @@ -76,11 +76,30 @@ public: */ // clang-format on class RException : public std::runtime_error { - RError fError; + std::optional fError; public: explicit RException(const RError &error) : std::runtime_error(error.GetReport()), fError(error) {} - const RError &GetError() const { return fError; } + RException(const RException &other) noexcept : std::runtime_error(other) + { + try { + fError = other.fError; + } catch (...) { + // OOM? Leave fError unset. + (void)fError; + } + } + + const RError &GetError() const + { + if (!fError) { + static const RError gOomError = RError("invalid fError in exception, possibly out of memory'", + {R__LOG_PRETTY_FUNCTION, __FILE__, __LINE__}); + + return gOomError; + } + return *fError; + } }; // clang-format off From 2d6ba604e9401f4e2405ea58982fb8373401fba2 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:16:13 +0200 Subject: [PATCH 08/37] [ntuple] fix throwing exception in RNTupleDescriptor --- tree/ntuple/src/RNTupleDescriptor.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index 69ff62cb1fc57..054e2d46ce94e 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -720,7 +720,7 @@ std::unique_ptr ROOT::RNTupleDescriptor::CreateModel(const R const auto cat = invalid.GetCategory(); bool mustThrow = cat != RInvalidField::ECategory::kUnknownStructure; if (mustThrow) - throw invalid.GetError(); + throw RException(R__FAIL(invalid.GetError())); // Not a hard error: skip the field and go on. continue; From eaf482604111ef5baac117860ee7a22a714dba0f Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:19:26 +0200 Subject: [PATCH 09/37] [ntuple] remove wrong forward declaration of RFieldVisitor --- tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx | 7 ------- tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx | 7 ------- 2 files changed, 14 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx index 9caf202bd0171..987f6799a81a8 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx @@ -28,13 +28,6 @@ #include namespace ROOT { -namespace Experimental { - -namespace Detail { -class RFieldVisitor; -} // namespace Detail - -} // namespace Experimental //////////////////////////////////////////////////////////////////////////////// /// Template specializations for concrete C++ fundamental types diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx index 132e285a113c5..f080e822121c1 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx @@ -31,13 +31,6 @@ #include namespace ROOT { -namespace Experimental { - -namespace Detail { -class RFieldVisitor; -} // namespace Detail - -} // namespace Experimental //////////////////////////////////////////////////////////////////////////////// /// Template specializations for C++ std::atomic From ee7229ff2190b5571026ca6ea13dbabf1bd31743 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:53:29 +0200 Subject: [PATCH 10/37] [core] fix definition of kMaxULong --- core/foundation/inc/RtypesCore.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/foundation/inc/RtypesCore.h b/core/foundation/inc/RtypesCore.h index 00a44bfced4d0..ee54dc95dab08 100644 --- a/core/foundation/inc/RtypesCore.h +++ b/core/foundation/inc/RtypesCore.h @@ -64,7 +64,7 @@ typedef int Seek_t; ///< File pointer (int). typedef long Long_t; ///< Signed long integer 8 bytes (long). Size depends on architecture \deprecated Consider replacing with `long` typedef unsigned long ULong_t; ///< Unsigned long integer 8 bytes (unsigned long). Size depends on architecture \deprecated Consider replacing with `unsigned long` #else -typedef int Seek_t; ///< File pointer (int). +typedef int Seek_t; ///< File pointer (int). typedef long Long_t; ///< Signed long integer 4 bytes (long). Size depends on architecture \deprecated Consider replacing with `long` typedef unsigned long ULong_t; ///< Unsigned long integer 4 bytes (unsigned long). Size depends on architecture \deprecated Consider replacing with `unsigned long` #endif @@ -119,7 +119,7 @@ constexpr UInt_t kMaxUInt = UInt_t(~0); ///< \deprecated Consider replaci constexpr Int_t kMaxInt = Int_t(kMaxUInt >> 1);///< \deprecated Consider replacing with `std::numeric_limits::max()` (or `std::int32_t`) constexpr Int_t kMinInt = -kMaxInt - 1; ///< \deprecated Consider replacing with `std::numeric_limits::lowest()` (or `std::int32_t`) -constexpr ULong_t kMaxULong = ULong_t(~0); ///< \deprecated Consider replacing with `std::numeric_limits::max()` +constexpr ULong_t kMaxULong = ULong_t(-1); ///< \deprecated Consider replacing with `std::numeric_limits::max()` constexpr Long_t kMaxLong = Long_t(kMaxULong >> 1);///< \deprecated Consider replacing with `std::numeric_limits::max()` constexpr Long_t kMinLong = -kMaxLong - 1; ///< \deprecated Consider replacing with `std::numeric_limits::lowest()` From 429acbbd6097cbb449f573880f7f853bf52343ff Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:53:58 +0200 Subject: [PATCH 11/37] [ntuple] merger: minor improvement in optional assignment --- tree/ntuple/src/RNTupleMerger.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index aea1e85c706ff..32131fabeda7f 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -217,7 +217,7 @@ try { "determined."; return -1; } - compression = (*firstColRange).GetCompressionSettings().value(); + compression = (*firstColRange).GetCompressionSettings(); R__LOG_INFO(NTupleMergeLog()) << "Using the first RNTuple's compression: " << *compression; } sources.push_back(std::move(source)); @@ -245,7 +245,7 @@ try { // Now merge RNTupleMerger merger{std::move(destination), std::move(model)}; RNTupleMergeOptions mergerOpts; - mergerOpts.fCompressionSettings = *compression; + mergerOpts.fCompressionSettings = compression; mergerOpts.fExtraVerbose = extraVerbose; if (auto mergingMode = ParseOptionMergingMode(mergeInfo->fOptions)) { mergerOpts.fMergingMode = *mergingMode; From d6f22eca80f371500b2428e76c103f7ddf35099b Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 14:15:43 +0200 Subject: [PATCH 12/37] [meta] remove unused TClassEdit::GetUniquePtrType() With commit c5ee8b53d163d6fe3b996df83f1cf5e80d974c61 (enable unique_ptr support for nested STL collections), the method TClassEdit::GetUniquePtrType() became obsolete. --- core/foundation/inc/TClassEdit.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/core/foundation/inc/TClassEdit.h b/core/foundation/inc/TClassEdit.h index ee69d729cf7fa..9f0586a539953 100644 --- a/core/foundation/inc/TClassEdit.h +++ b/core/foundation/inc/TClassEdit.h @@ -236,14 +236,6 @@ namespace TClassEdit { { return 0 == name.compare(0, 17, "std::__pair_base<") || 0 == name.compare(0, 12, "__pair_base<"); } - inline std::string GetUniquePtrType(std::string_view name) - { - // Find the first template parameter - std::vector v; - int i; - GetSplit(name.data(), v, i); - return v[1]; - } std::string GetNameForIO(const std::string& templateInstanceName, TClassEdit::EModType mode = TClassEdit::kNone, bool* hasChanged = nullptr); From 9f00eb1759decf9e49a2b93ece736d06f8e75028 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 14:18:56 +0200 Subject: [PATCH 13/37] [ntuple] minor readability improvement --- tree/ntuple/src/RPageStorage.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 825e183b379aa..74512d00e8475 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -1182,8 +1182,8 @@ void ROOT::Internal::RPagePersistentSink::CommitSealedPageV(std::spansecond.fSealedPage; - if (sealedPageIt->GetDataSize() != p->GetDataSize() || - memcmp(sealedPageIt->GetBuffer(), p->GetBuffer(), p->GetDataSize())) { + if ((sealedPageIt->GetDataSize() != p->GetDataSize()) || + (memcmp(sealedPageIt->GetBuffer(), p->GetBuffer(), p->GetDataSize()) != 0)) { mask.emplace_back(true); locatorIndexes.emplace_back(iLocator++); continue; From 07c1fcf19f015c7f7b468aa26244facd1c2f68c2 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:14:06 +0200 Subject: [PATCH 14/37] [ntuple] minor improvement in RPageSourceFile::CreateFromAnchor() --- tree/ntuple/src/RPageStorageFile.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 2d0c1b6681f9d..647f4db7418f3 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -395,9 +395,8 @@ ROOT::Internal::RPageSourceFile::CreateFromAnchor(const RNTuple &anchor, const R // For local TFiles, TDavixFile, TCurlFile, and TNetXNGFile, we want to open a new RRawFile to take advantage of the // faster reading. We check the exact class name to avoid classes inheriting in ROOT (for example TMemFile) or in // experiment frameworks. - std::string className = anchor.fFile->IsA()->GetName(); - auto url = anchor.fFile->GetEndpointUrl(); - auto protocol = std::string(url->GetProtocol()); + const std::string className = anchor.fFile->IsA()->GetName(); + const auto url = anchor.fFile->GetEndpointUrl(); if (className == "TFile") { rawFile = ROOT::Internal::RRawFile::Create(url->GetFile()); } else if (className == "TDavixFile" || className == "TCurlFile" || className == "TNetXNGFile") { From 04d1aa84425625d233edff0ddf3fe23e86676d72 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:15:05 +0200 Subject: [PATCH 15/37] [ntuple] remove unused variable in RFieldBase::Create() --- tree/ntuple/src/RFieldBase.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index 8cd20cc69442b..bbf6549f3f552 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -430,7 +430,6 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa } else if (resolvedType.substr(0, 24) == "std::unordered_multiset<") { std::string itemTypeName = resolvedType.substr(24, resolvedType.length() - 25); auto itemField = Create("_0", itemTypeName, options, desc, maybeGetChildId(0)).Unwrap(); - auto normalizedInnerTypeName = itemField->GetTypeName(); result = std::make_unique(fieldName, RSetField::ESetType::kUnorderedMultiSet, std::move(itemField)); } else if (resolvedType.substr(0, 9) == "std::map<") { auto innerTypes = TokenizeTypeList(resolvedType.substr(9, resolvedType.length() - 10)); From a173e5c0ce11123f94a351b6abddb1319e54d7af Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:21:21 +0200 Subject: [PATCH 16/37] [ntuple] fix use-after-move in RNTupleDescriptorBuilder::AddColumn() --- tree/ntuple/src/RNTupleDescriptor.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index 054e2d46ce94e..e3ddb5652b0b5 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -1300,9 +1300,9 @@ ROOT::RResult ROOT::Internal::RNTupleDescriptorBuilder::AddColumn(RColumnD if (!columnDesc.IsAliasColumn()) fDescriptor.fNPhysicalColumns++; - fDescriptor.fColumnDescriptors.emplace(logicalId, std::move(columnDesc)); if (fDescriptor.fHeaderExtension) fDescriptor.fHeaderExtension->MarkExtendedColumn(columnDesc); + fDescriptor.fColumnDescriptors.emplace(logicalId, std::move(columnDesc)); return RResult::Success(); } From 99fc90f54175e1889ac115d0f3e7e7d1a36299e3 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:39:02 +0200 Subject: [PATCH 17/37] [NFC][ntuple] suppress clang-tidy false positive --- tree/ntuple/src/RColumnElement.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RColumnElement.hxx b/tree/ntuple/src/RColumnElement.hxx index a1cba707103c6..e71763f3060a7 100644 --- a/tree/ntuple/src/RColumnElement.hxx +++ b/tree/ntuple/src/RColumnElement.hxx @@ -58,7 +58,7 @@ void UnpackBits(void *dst, const void *src, std::size_t count, std::size_t sizeo } // namespace ROOT::Internal::BitPacking -namespace { +namespace { // NOLINT(misc-anonymous-namespace-in-header): this header is only used in tests and in RColumnElement.cxx // In this namespace, common routines are defined for element packing and unpacking of ints and floats. // The following conversions and encodings exist: @@ -331,7 +331,7 @@ inline void CastZigzagSplitUnpack(void *destination, const void *source, std::si } // namespace // anonymous namespace because these definitions are not meant to be exported. -namespace { +namespace { // NOLINT(misc-anonymous-namespace-in-header): this header is only used in tests and in RColumnElement.cxx using ROOT::ENTupleColumnType; using ROOT::Internal::kTestFutureColumnType; From df5366b6a1bd1d7ca6b0be57222d5f2885a2572c Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:01:35 +0200 Subject: [PATCH 18/37] [ntuple] fix widening method visibility change --- tree/ntuple/inc/ROOT/RPageSinkBuf.hxx | 7 +++---- tree/ntuple/inc/ROOT/RPageStorage.hxx | 10 ++++++---- tree/ntuple/src/RNTupleParallelWriter.cxx | 12 ++++++------ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 8cda1d843c71e..53f081486c629 100644 --- a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx +++ b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx @@ -37,7 +37,6 @@ namespace Internal { */ // clang-format on class RPageSinkBuf : public RPageSink { -private: /// A buffered column. The column is not responsible for RPage memory management (i.e. ReservePage), /// which is handled by the enclosing RPageSinkBuf. class RColumnBuf { @@ -94,7 +93,6 @@ private: RPageStorage::SealedPageSequence_t fSealedPages; }; -private: /// I/O performance counters that get registered in fMetrics struct RCounters { ROOT::Experimental::Detail::RNTuplePlainCounter &fParallelZip; @@ -122,6 +120,9 @@ private: void ConnectFields(const std::vector &fields, ROOT::NTupleSize_t firstEntry); void FlushClusterImpl(std::function FlushClusterFn); + void InitImpl(ROOT::RNTupleModel &model) final; + RNTupleLink CommitDatasetImpl() final; + public: explicit RPageSinkBuf(std::unique_ptr inner); RPageSinkBuf(const RPageSinkBuf&) = delete; @@ -136,7 +137,6 @@ public: ROOT::NTupleSize_t GetNEntries() const final { return fInnerSink->GetNEntries(); } - void InitImpl(ROOT::RNTupleModel &model) final; void UpdateSchema(const RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) final; void UpdateExtraTypeInfo(const ROOT::RExtraTypeInfoDescriptor &extraTypeInfo) final; @@ -148,7 +148,6 @@ public: RStagedCluster StageCluster(ROOT::NTupleSize_t nNewEntries) final; void CommitStagedClusters(std::span clusters) final; void CommitClusterGroup() final; - RNTupleLink CommitDatasetImpl() final; void CommitAttributeSet(std::string_view attrSetName, const RNTupleLink &attrAnchorInfo) final; RPage ReservePage(ColumnHandle_t columnHandle, std::size_t nElements) final; diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index b3b82482303f6..2cf219e47cb8f 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -484,6 +484,9 @@ protected: virtual void InitImpl(unsigned char *serializedHeader, std::uint32_t length) = 0; + /// Updates the descriptor and calls InitImpl() that handles the backend-specific details (file, DAOS, etc.) + void InitImpl(RNTupleModel &model) final; + virtual RNTupleLocator CommitPageImpl(ColumnHandle_t columnHandle, const ROOT::Internal::RPage &page) = 0; virtual RNTupleLocator CommitSealedPageImpl(ROOT::DescriptorId_t physicalColumnId, const RPageStorage::RSealedPage &sealedPage) = 0; @@ -504,6 +507,9 @@ protected: virtual RNTupleLocator CommitClusterGroupImpl(unsigned char *serializedPageList, std::uint32_t length) = 0; virtual RNTupleLink CommitDatasetImpl(unsigned char *serializedFooter, std::uint32_t length) = 0; + /// \return The locator and length of the written anchor. + RNTupleLink CommitDatasetImpl() final; + /// Enables the default set of metrics provided by RPageSink. `prefix` will be used as the prefix for /// the counters registered in the internal RNTupleMetrics object. /// This set of counters can be extended by a subclass by calling `fMetrics.MakeCounter<...>()`. @@ -531,8 +537,6 @@ public: ROOT::NTupleSize_t GetNEntries() const final { return fPrevClusterNEntries; } - /// Updates the descriptor and calls InitImpl() that handles the backend-specific details (file, DAOS, etc.) - void InitImpl(RNTupleModel &model) final; void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) override; void UpdateExtraTypeInfo(const ROOT::RExtraTypeInfoDescriptor &extraTypeInfo) final; @@ -551,8 +555,6 @@ public: void CommitStagedClusters(std::span clusters) final; void CommitClusterGroup() final; void CommitAttributeSet(std::string_view attrSetName, const RNTupleLink &attrAnchorInfo) final; - /// \return The locator and length of the written anchor. - RNTupleLink CommitDatasetImpl() final; }; // class RPagePersistentSink // clang-format off diff --git a/tree/ntuple/src/RNTupleParallelWriter.cxx b/tree/ntuple/src/RNTupleParallelWriter.cxx index d909f38265d0c..453d6805b9287 100644 --- a/tree/ntuple/src/RNTupleParallelWriter.cxx +++ b/tree/ntuple/src/RNTupleParallelWriter.cxx @@ -60,6 +60,12 @@ class RPageSynchronizingSink : public RPageSink { RPageSink *fInnerSink; std::mutex *fMutex; + void InitImpl(ROOT::RNTupleModel &) final {} + ROOT::Internal::RNTupleLink CommitDatasetImpl() final + { + throw ROOT::RException(R__FAIL("should never commit dataset via RPageSynchronizingSink")); + } + public: explicit RPageSynchronizingSink(RPageSink &inner, std::mutex &mutex) : RPageSink(inner.GetNTupleName(), inner.GetWriteOptions()), fInnerSink(&inner), fMutex(&mutex) @@ -76,7 +82,6 @@ class RPageSynchronizingSink : public RPageSink { NTupleSize_t GetNEntries() const final { return fInnerSink->GetNEntries(); } ColumnHandle_t AddColumn(DescriptorId_t, RColumn &) final { return {}; } - void InitImpl(ROOT::RNTupleModel &) final {} void UpdateSchema(const RNTupleModelChangeset &, NTupleSize_t) final { throw ROOT::RException(R__FAIL("UpdateSchema not supported via RPageSynchronizingSink")); @@ -107,11 +112,6 @@ class RPageSynchronizingSink : public RPageSink { throw ROOT::RException(R__FAIL("should never commit cluster group via RPageSynchronizingSink")); } - ROOT::Internal::RNTupleLink CommitDatasetImpl() final - { - throw ROOT::RException(R__FAIL("should never commit dataset via RPageSynchronizingSink")); - } - RSinkGuard GetSinkGuard() final { return RSinkGuard(fMutex); } std::unique_ptr CloneAsHidden(std::string_view, const ROOT::RNTupleWriteOptions &) const final From f72e46136978b987f9e38ee5b81aefdb6c256af5 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:08:14 +0200 Subject: [PATCH 19/37] [ntuple] remove unusued usings --- tree/ntuple/src/RMiniFile.cxx | 1 - tree/ntuple/src/RNTupleDescriptor.cxx | 2 -- tree/ntuple/src/RPageSinkBuf.cxx | 1 - tree/ntuple/src/RPageStorage.cxx | 1 - tree/ntuple/src/RPageStorageFile.cxx | 2 -- 5 files changed, 7 deletions(-) diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index fa4ae6d3c014b..93355206818fe 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -55,7 +55,6 @@ #endif #endif /* R__LITTLE_ENDIAN */ -using ROOT::Internal::MakeUninitArray; using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; using ROOT::Internal::RNTupleSerializer; diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index e3ddb5652b0b5..ebfed9d89b747 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -32,8 +32,6 @@ #include #include -using ROOT::Internal::RNTupleSerializer; - bool ROOT::RFieldDescriptor::operator==(const RFieldDescriptor &other) const { return fFieldId == other.fFieldId && fFieldVersion == other.fFieldVersion && fTypeVersion == other.fTypeVersion && diff --git a/tree/ntuple/src/RPageSinkBuf.cxx b/tree/ntuple/src/RPageSinkBuf.cxx index a4b715e99dcbf..3b803eb585a26 100644 --- a/tree/ntuple/src/RPageSinkBuf.cxx +++ b/tree/ntuple/src/RPageSinkBuf.cxx @@ -27,7 +27,6 @@ using ROOT::Experimental::Detail::RNTupleMetrics; using ROOT::Experimental::Detail::RNTuplePlainCounter; using ROOT::Experimental::Detail::RNTuplePlainTimer; using ROOT::Experimental::Detail::RNTupleTickCounter; -using ROOT::Internal::MakeUninitArray; void ROOT::Internal::RPageSinkBuf::RColumnBuf::DropBufferedPages() { diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 74512d00e8475..46ded2885e976 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -41,7 +41,6 @@ #include #include -using ROOT::Internal::MakeUninitArray; using ROOT::Internal::RClusterDescriptorBuilder; using ROOT::Internal::RClusterGroupDescriptorBuilder; using ROOT::Internal::RColumn; diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 647f4db7418f3..e0e2b6a1081f8 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -45,9 +45,7 @@ using ROOT::Experimental::Detail::RNTupleAtomicCounter; using ROOT::Experimental::Detail::RNTupleAtomicTimer; using ROOT::Experimental::Detail::RNTupleCalcPerf; using ROOT::Experimental::Detail::RNTupleMetrics; -using ROOT::Internal::MakeUninitArray; using ROOT::Internal::RCluster; -using ROOT::Internal::RClusterPool; using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; using ROOT::Internal::RNTupleFileWriter; From 37d7e7bcc4f43861209f14d0ecf6770f81e8144a Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:14:36 +0200 Subject: [PATCH 20/37] [ntuple] move RAuxiliaryProcessorField to anonymous namespace --- tree/ntuple/src/RNTupleProcessor.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RNTupleProcessor.cxx b/tree/ntuple/src/RNTupleProcessor.cxx index 3a00539e0a82c..a6e3fb7c21910 100644 --- a/tree/ntuple/src/RNTupleProcessor.cxx +++ b/tree/ntuple/src/RNTupleProcessor.cxx @@ -398,13 +398,13 @@ void ROOT::Experimental::RNTupleChainProcessor::PrintStructureImpl(std::ostream //------------------------------------------------------------------------------ -namespace ROOT::Experimental::Internal { +namespace { class RAuxiliaryProcessorField final : public ROOT::RRecordField { private: using RFieldBase::GenerateColumns; void GenerateColumns() final { - throw RException(R__FAIL("RAuxiliaryProcessorField fields must only be used for reading")); + throw ROOT::RException(R__FAIL("RAuxiliaryProcessorField fields must only be used for reading")); } public: @@ -414,7 +414,7 @@ class RAuxiliaryProcessorField final : public ROOT::RRecordField { AttachItemFields(std::move(itemFields)); } }; -} // namespace ROOT::Experimental::Internal +} // anonymous namespace ROOT::Experimental::RNTupleJoinProcessor::RNTupleJoinProcessor(std::unique_ptr primaryProcessor, std::unique_ptr auxProcessor, From 410ab38b3f900daed8045ec4290014e4abd203c3 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:17:21 +0200 Subject: [PATCH 21/37] [ntuple] move RKeyBlob to anonymous namespace --- tree/ntuple/src/RMiniFile.cxx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 93355206818fe..5caa837846786 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -600,8 +600,7 @@ constexpr char const *kNTupleClassName = "ROOT::RNTuple"; } // anonymous namespace -namespace ROOT { -namespace Internal { +namespace ROOT::Internal { /// If a TFile container is written by a C stream (simple file), on dataset commit, the file header /// and the TFile record need to be updated struct RTFileControlBlock { @@ -610,7 +609,9 @@ struct RTFileControlBlock { std::uint64_t fSeekNTuple{0}; // Remember the offset for the keys list std::uint64_t fSeekFileRecord{0}; }; +} // namespace ROOT::Internal +namespace { /// The RKeyBlob writes an invisible key into a TFile. That is, a key that is not indexed in the list of keys, /// like a TBasket. /// NOTE: out of anonymous namespace because otherwise ClassDefInline fails to compile @@ -637,9 +638,7 @@ class RKeyBlob : public TKey { ClassDefInlineOverride(RKeyBlob, 0) }; - -} // namespace Internal -} // namespace ROOT +} // anonymous namespace // Computes how many chunks do we need to fit `nbytes` of payload, considering that the // first chunk also needs to house the offsets of the other chunks and no chunk can @@ -1159,7 +1158,7 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::ReserveBlobKey(T &caller, TFile std::size_t len, unsigned char keyBuffer[kBlobKeyLen]) { std::uint64_t offsetKey; - ROOT::Internal::RKeyBlob keyBlob(&file); + RKeyBlob keyBlob(&file); // Since it is unknown beforehand if offsetKey is beyond the 2GB limit or not, // RKeyBlob will always reserve space for a big key (version >= 1000) keyBlob.Reserve(nbytes, &offsetKey); From dc3fa75f0ee556d4723161830b698df89fb4b206 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:19:53 +0200 Subject: [PATCH 22/37] [ntuple] private linkage of stream operator in the merger --- tree/ntuple/src/RNTupleMerger.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 32131fabeda7f..3c7cadc71b76b 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -429,7 +429,7 @@ struct RSealedPageMergeData { std::vector> fBuffers; }; -std::ostream &operator<<(std::ostream &os, const std::optional &x) +static std::ostream &operator<<(std::ostream &os, const std::optional &x) { if (x) { os << '(' << x->fMin << ", " << x->fMax << ')'; From ca5668d6b3ca5c54fb3e26b87ee1236b57327af4 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:27:52 +0200 Subject: [PATCH 23/37] [ntuple] replace std::endl by \n --- tree/ntuple/src/RFieldVisitor.cxx | 4 ++-- tree/ntuple/src/RNTupleDescriptorFmt.cxx | 6 +++--- tree/ntuple/src/RNTupleMetrics.cxx | 4 ++-- tree/ntuple/src/RNTupleReader.cxx | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tree/ntuple/src/RFieldVisitor.cxx b/tree/ntuple/src/RFieldVisitor.cxx index 5ca6d26a7aadc..05e34fab35743 100644 --- a/tree/ntuple/src/RFieldVisitor.cxx +++ b/tree/ntuple/src/RFieldVisitor.cxx @@ -143,7 +143,7 @@ void ROOT::Internal::RPrintValueVisitor::PrintRecord(const ROOT::RFieldBase &fie auto elems = field.SplitValue(fValue); for (auto iValue = elems.begin(); iValue != elems.end();) { if (!fPrintOptions.fPrintSingleLine) - fOutput << std::endl; + fOutput << '\n'; RPrintOptions options; options.fPrintSingleLine = fPrintOptions.fPrintSingleLine; @@ -152,7 +152,7 @@ void ROOT::Internal::RPrintValueVisitor::PrintRecord(const ROOT::RFieldBase &fie if (++iValue == elems.end()) { if (!fPrintOptions.fPrintSingleLine) - fOutput << std::endl; + fOutput << '\n'; break; } else { fOutput << ","; diff --git a/tree/ntuple/src/RNTupleDescriptorFmt.cxx b/tree/ntuple/src/RNTupleDescriptorFmt.cxx index db18b0265d3e3..8063a7e39622b 100644 --- a/tree/ntuple/src/RNTupleDescriptorFmt.cxx +++ b/tree/ntuple/src/RNTupleDescriptorFmt.cxx @@ -159,7 +159,7 @@ void ROOT::RNTupleDescriptor::PrintInfo(std::ostream &output) const << float(headerSize + footerSize) / float(nBytesOnStorage) << "\n"; output << "------------------------------------------------------------\n"; output << "CLUSTER DETAILS\n"; - output << "------------------------------------------------------------" << std::endl; + output << "------------------------------------------------------------\n"; std::sort(clusters.begin(), clusters.end()); for (unsigned int i = 0; i < clusters.size(); ++i) { @@ -168,7 +168,7 @@ void ROOT::RNTupleDescriptor::PrintInfo(std::ostream &output) const output << " " << " # Pages: " << clusters[i].fNPages << "\n"; output << " " << " Size on storage: " << clusters[i].fNBytesOnStorage << " B\n"; output << " " << " Compression: " << std::fixed << std::setprecision(2) - << float(clusters[i].fNBytesInMemory) / float(float(clusters[i].fNBytesOnStorage)) << std::endl; + << float(clusters[i].fNBytesInMemory) / float(float(clusters[i].fNBytesOnStorage)) << '\n'; } output << "------------------------------------------------------------\n"; @@ -199,6 +199,6 @@ void ROOT::RNTupleDescriptor::PrintInfo(std::ostream &output) const output << " Size on storage: " << col.fNBytesOnStorage << " B\n"; output << " Compression: " << std::fixed << std::setprecision(2) << float(col.fElementSize * col.fNElements) / float(col.fNBytesOnStorage) << "\n"; - output << "............................................................" << std::endl; + output << "............................................................\n"; } } diff --git a/tree/ntuple/src/RNTupleMetrics.cxx b/tree/ntuple/src/RNTupleMetrics.cxx index c97146010f3a6..c02fbd3d88675 100644 --- a/tree/ntuple/src/RNTupleMetrics.cxx +++ b/tree/ntuple/src/RNTupleMetrics.cxx @@ -65,12 +65,12 @@ ROOT::Experimental::Detail::RNTupleMetrics::GetCounter(std::string_view name) co void ROOT::Experimental::Detail::RNTupleMetrics::Print(std::ostream &output, const std::string &prefix) const { if (!fIsEnabled) { - output << fName << " metrics disabled!" << std::endl; + output << fName << " metrics disabled!\n"; return; } for (const auto &c : fCounters) { - output << prefix << fName << kNamespaceSeperator << c->ToString() << std::endl; + output << prefix << fName << kNamespaceSeperator << c->ToString() << '\n'; } for (const auto c : fObservedMetrics) { c->Print(output, prefix + fName + "."); diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index dfeb557bb2d86..b2d0c4f8e5e62 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -309,18 +309,18 @@ void ROOT::RNTupleReader::Show(ROOT::NTupleSize_t index, std::ostream &output) reader->LoadEntry(index); output << "{"; for (auto iValue = entry.begin(); iValue != entry.end();) { - output << std::endl; + output << '\n'; ROOT::Internal::RPrintValueVisitor visitor(*iValue, output, 1 /* level */); iValue->GetField().AcceptVisitor(visitor); if (++iValue == entry.end()) { - output << std::endl; + output << '\n'; break; } else { output << ","; } } - output << "}" << std::endl; + output << "}\n"; } const ROOT::RNTupleDescriptor &ROOT::RNTupleReader::GetDescriptor() From 997ec63929662f9d112c2f4dd842dc9623cb06d0 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:30:28 +0200 Subject: [PATCH 24/37] [ntuple] minor performance improvement in RPageSourceFile::LoadClusters() --- tree/ntuple/src/RPageStorageFile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index e0e2b6a1081f8..fe5c263f28404 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -729,7 +729,7 @@ ROOT::Internal::RPageSourceFile::LoadClusters(std::span clusterK std::vector readRequests; clusters.reserve(clusterKeys.size()); - for (auto key : clusterKeys) { + for (const auto &key : clusterKeys) { clusters.emplace_back(PrepareSingleCluster(key, readRequests)); } From 5f79c9bbe1e8f9430c92d31afc283227a593fc0d Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:38:37 +0200 Subject: [PATCH 25/37] [core] pass RError::RLocation by const ref --- core/foundation/inc/ROOT/RError.hxx | 12 ++++++------ core/foundation/src/RError.cxx | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/foundation/inc/ROOT/RError.hxx b/core/foundation/inc/ROOT/RError.hxx index 7c6365f9f7327..90c29544c711d 100644 --- a/core/foundation/inc/ROOT/RError.hxx +++ b/core/foundation/inc/ROOT/RError.hxx @@ -58,9 +58,9 @@ private: public: /// Used by R__FAIL - RError(std::string_view message, RLocation &&sourceLocation); + RError(std::string_view message, const RLocation &sourceLocation); /// Used by R__FORWARD_RESULT - void AddFrame(RLocation &&sourceLocation); + void AddFrame(const RLocation &sourceLocation); /// Add more information to the diagnostics void AppendToMessage(std::string_view info) { fMessage += info; } /// Format a dignostics report, e.g. for an exception message @@ -147,9 +147,9 @@ public: static RError ForwardError(RResultBase &&result, RError::RLocation &&sourceLocation) { if (!result.fError) { - return RError("internal error: attempt to forward error of successful operation", std::move(sourceLocation)); + return RError("internal error: attempt to forward error of successful operation", sourceLocation); } - result.fError->AddFrame(std::move(sourceLocation)); + result.fError->AddFrame(sourceLocation); return *result.fError; } }; // class RResultBase @@ -249,7 +249,7 @@ public: RResult &Forward(RError::RLocation &&sourceLocation) { if (fError) - fError->AddFrame(std::move(sourceLocation)); + fError->AddFrame(sourceLocation); return *this; } @@ -299,7 +299,7 @@ public: RResult &Forward(RError::RLocation &&sourceLocation) { if (fError) - fError->AddFrame(std::move(sourceLocation)); + fError->AddFrame(sourceLocation); return *this; } diff --git a/core/foundation/src/RError.cxx b/core/foundation/src/RError.cxx index 858ddf721e9db..72503066ba1ca 100644 --- a/core/foundation/src/RError.cxx +++ b/core/foundation/src/RError.cxx @@ -28,7 +28,7 @@ std::string ROOT::RError::GetReport() const return report; } -ROOT::RError::RError(std::string_view message, RLocation &&sourceLocation) : fMessage(message) +ROOT::RError::RError(std::string_view message, const RLocation &sourceLocation) : fMessage(message) { // Avoid frequent reallocations as we move up the call stack @@ -36,7 +36,7 @@ ROOT::RError::RError(std::string_view message, RLocation &&sourceLocation) : fMe AddFrame(std::move(sourceLocation)); } -void ROOT::RError::AddFrame(RLocation &&sourceLocation) +void ROOT::RError::AddFrame(const RLocation &sourceLocation) { fStackTrace.emplace_back(sourceLocation); } From 1b4445d4e8ba33c52d83747c6e5607289d555242 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:41:36 +0200 Subject: [PATCH 26/37] [ntuple] use member move in RValue move ctor --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 2f05c3d6b1723..1e9773b7b4a22 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -775,7 +775,7 @@ public: fTypeInfo = nullptr; return *this; } - RValue(RValue &&other) : fField(other.fField), fObjPtr(other.fObjPtr) {} + RValue(RValue &&other) : fField(other.fField), fObjPtr(std::move(other.fObjPtr)) {} RValue &operator=(RValue &&other) { fField = other.fField; From eef418d9ff4c4a4d5b425761d83b5ef1c31b3959 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:43:56 +0200 Subject: [PATCH 27/37] [ntuple] minor perf improvement in type name normalization --- tree/ntuple/src/RFieldUtils.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RFieldUtils.cxx b/tree/ntuple/src/RFieldUtils.cxx index 6420d43cc8c47..d8b37d46baa60 100644 --- a/tree/ntuple/src/RFieldUtils.cxx +++ b/tree/ntuple/src/RFieldUtils.cxx @@ -393,7 +393,7 @@ void NormalizeTemplateArguments(std::string &templatedTypeName, int maxTemplateA // Given a type name normalized by ROOT Meta, return the type name normalized according to the RNTuple rules. std::string GetRenormalizedMetaTypeName(const std::string &metaNormalizedName) { - const auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName); + auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName); // RNTuple resolves Double32_t for the normalized type name but keeps Double32_t for the type alias // (also in template parameters) if (canonicalTypePrefix == "Double32_t") @@ -573,7 +573,7 @@ std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &o TClassEdit::TSplitType splitname(origName.c_str(), modType); std::string shortType; splitname.ShortType(shortType, modType); - const auto canonicalTypePrefix = GetCanonicalTypePrefix(shortType); + auto canonicalTypePrefix = GetCanonicalTypePrefix(shortType); if (canonicalTypePrefix.find('<') == std::string::npos) { // If there are no templates, the function is done. From 309c8149715c71a0d5914431c00bdc46d3b63918 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:53:38 +0200 Subject: [PATCH 28/37] [ntuple] make moving RPage(Ref) noexcept --- tree/ntuple/inc/ROOT/RPage.hxx | 4 ++-- tree/ntuple/inc/ROOT/RPagePool.hxx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPage.hxx b/tree/ntuple/inc/ROOT/RPage.hxx index 1c45836312ac0..2344a1981247e 100644 --- a/tree/ntuple/inc/ROOT/RPage.hxx +++ b/tree/ntuple/inc/ROOT/RPage.hxx @@ -81,7 +81,7 @@ public: {} RPage(const RPage &) = delete; RPage &operator=(const RPage &) = delete; - RPage(RPage &&other) + RPage(RPage &&other) noexcept { fBuffer = other.fBuffer; fPageAllocator = other.fPageAllocator; @@ -92,7 +92,7 @@ public: fClusterInfo = other.fClusterInfo; other.fPageAllocator = nullptr; } - RPage &operator=(RPage &&other) + RPage &operator=(RPage &&other) noexcept { if (this != &other) { std::swap(fBuffer, other.fBuffer); diff --git a/tree/ntuple/inc/ROOT/RPagePool.hxx b/tree/ntuple/inc/ROOT/RPagePool.hxx index 64b66aa2df4a4..7ee2aba14f31c 100644 --- a/tree/ntuple/inc/ROOT/RPagePool.hxx +++ b/tree/ntuple/inc/ROOT/RPagePool.hxx @@ -213,9 +213,9 @@ public: RPageRef(const RPageRef &other) = delete; RPageRef &operator=(const RPageRef &other) = delete; - RPageRef(RPageRef &&other) : RPageRef(other.fPage, other.fPagePool) { other.fPagePool = nullptr; } + RPageRef(RPageRef &&other) noexcept : RPageRef(other.fPage, other.fPagePool) { other.fPagePool = nullptr; } - RPageRef &operator=(RPageRef &&other) + RPageRef &operator=(RPageRef &&other) noexcept { if (this != &other) { std::swap(fPage, other.fPage); From f5d30c1885e034b18168b4d5fb5f33d441ef6529 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 09:09:48 +0200 Subject: [PATCH 29/37] [ntuple] avoid some unnecessary value copies --- .../inc/ROOT/RField/RFieldProxiedCollection.hxx | 4 ++-- tree/ntuple/inc/ROOT/RFieldBase.hxx | 6 +++--- tree/ntuple/inc/ROOT/RFieldVisitor.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleModel.hxx | 6 +++--- tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleReader.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleView.hxx | 4 ++-- tree/ntuple/inc/ROOT/RPageSinkBuf.hxx | 2 +- tree/ntuple/inc/ROOT/RPageStorage.hxx | 10 +++++----- tree/ntuple/src/RFieldBase.cxx | 4 ++-- tree/ntuple/src/RFieldMeta.cxx | 4 ++-- tree/ntuple/src/RFieldSequenceContainer.cxx | 2 +- tree/ntuple/src/RNTupleModel.cxx | 12 +++++++----- tree/ntuple/src/RNTupleProcessor.cxx | 4 ++-- tree/ntuple/src/RPageSinkBuf.cxx | 2 +- tree/ntuple/src/RPageStorage.cxx | 4 ++-- 16 files changed, 36 insertions(+), 34 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx b/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx index ab30a4d61f589..5d327b9aa7fb5 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx @@ -127,10 +127,10 @@ protected: RCollectionIterableOnce::RIteratorFuncs fIFuncsWrite; public: - explicit RProxiedCollectionDeleter(std::shared_ptr proxy) : fProxy(proxy) {} + explicit RProxiedCollectionDeleter(std::shared_ptr proxy) : fProxy(std::move(proxy)) {} RProxiedCollectionDeleter(std::shared_ptr proxy, std::unique_ptr itemDeleter, size_t itemSize) - : fProxy(proxy), fItemDeleter(std::move(itemDeleter)), fItemSize(itemSize) + : fProxy(std::move(proxy)), fItemDeleter(std::move(itemDeleter)), fItemSize(itemSize) { fIFuncsWrite = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), false /* readFromDisk */); } diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 1e9773b7b4a22..facd30575bf8f 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -497,7 +497,7 @@ protected: /// Set a user-defined function to be called after reading a value, giving a chance to inspect and/or modify the /// value object. /// Returns an index that can be used to remove the callback. - size_t AddReadCallback(ReadCallback_t func); + size_t AddReadCallback(const ReadCallback_t &func); void RemoveReadCallback(size_t idx); // Perform housekeeping tasks for global to cluster-local index translation @@ -763,7 +763,7 @@ private: std::shared_ptr fObjPtr; mutable std::atomic fTypeInfo = nullptr; - RValue(RFieldBase *field, std::shared_ptr objPtr) : fField(field), fObjPtr(objPtr) {} + RValue(RFieldBase *field, std::shared_ptr objPtr) : fField(field), fObjPtr(std::move(objPtr)) {} public: RValue(const RValue &other) : fField(other.fField), fObjPtr(other.fObjPtr) {} @@ -814,7 +814,7 @@ public: void Read(ROOT::NTupleSize_t globalIndex) { fField->Read(globalIndex, fObjPtr.get()); } void Read(RNTupleLocalIndex localIndex) { fField->Read(localIndex, fObjPtr.get()); } - void Bind(std::shared_ptr objPtr) { fObjPtr = objPtr; } + void Bind(std::shared_ptr objPtr) { fObjPtr = std::move(objPtr); } void BindRawPtr(void *rawPtr); /// Replace the current object pointer by a pointer to a new object constructed by the field void EmplaceNew() { fObjPtr = fField->CreateValue().GetPtr(); } diff --git a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx index 41af5a1b76e18..8e2fb0c53a71b 100644 --- a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx +++ b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx @@ -199,7 +199,7 @@ private: public: RPrintValueVisitor(ROOT::RFieldBase::RValue value, std::ostream &output, unsigned int level = 0, RPrintOptions options = RPrintOptions()) - : fValue(value), fOutput{output}, fLevel(level), fPrintOptions(options) + : fValue(std::move(value)), fOutput{output}, fLevel(level), fPrintOptions(options) { } diff --git a/tree/ntuple/inc/ROOT/RNTupleModel.hxx b/tree/ntuple/inc/ROOT/RNTupleModel.hxx index eb393b63bab54..8a5df8d244096 100644 --- a/tree/ntuple/inc/ROOT/RNTupleModel.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleModel.hxx @@ -314,7 +314,7 @@ public: /// ~~~ /// /// Creating projections for fields containing `std::variant` or fixed-size arrays is unsupported. - RResult AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping); + RResult AddProjectedField(std::unique_ptr field, const FieldMappingFunc_t &mapping); /// Transitions an RNTupleModel from the *building* state to the *frozen* state, disabling adding additional fields /// and enabling creating entries from it. Freezing an already-frozen model is a no-op. Throws an RException if the @@ -418,7 +418,7 @@ struct RNTupleModelChangeset { /// \see RNTupleModel::AddProjectedField() ROOT::RResult - AddProjectedField(std::unique_ptr field, RNTupleModel::FieldMappingFunc_t mapping); + AddProjectedField(std::unique_ptr field, const RNTupleModel::FieldMappingFunc_t &mapping); }; } // namespace Internal @@ -459,7 +459,7 @@ public: void AddField(std::unique_ptr field, std::string_view parentName = ""); /// \see RNTupleModel::AddProjectedField() - RResult AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping); + RResult AddProjectedField(std::unique_ptr field, const FieldMappingFunc_t &mapping); }; } // namespace ROOT diff --git a/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx b/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx index d72ac8f282247..42094516b8129 100644 --- a/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx @@ -107,7 +107,7 @@ private: fQualifiedFieldName(qualifiedFieldName), fValue(std::move(value)), fIsValid(isValid), - fProcessorProvenance(provenance) + fProcessorProvenance(std::move(provenance)) { } }; diff --git a/tree/ntuple/inc/ROOT/RNTupleReader.hxx b/tree/ntuple/inc/ROOT/RNTupleReader.hxx index 8f3e8e191d3d2..2f9c118f18c38 100644 --- a/tree/ntuple/inc/ROOT/RNTupleReader.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleReader.hxx @@ -166,7 +166,7 @@ public: void DeactivateEntry(NTupleSize_t entryNumber); explicit RActiveEntryToken(std::shared_ptr ptrControlBlock) - : fPtrControlBlock(ptrControlBlock) + : fPtrControlBlock(std::move(ptrControlBlock)) { } diff --git a/tree/ntuple/inc/ROOT/RNTupleView.hxx b/tree/ntuple/inc/ROOT/RNTupleView.hxx index 7dcf187e6f3c7..f963b1c3793c9 100644 --- a/tree/ntuple/inc/ROOT/RNTupleView.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleView.hxx @@ -115,7 +115,7 @@ protected: } RNTupleViewBase(std::unique_ptr field, ROOT::RNTupleGlobalRange range, std::shared_ptr objPtr) - : fField(std::move(field)), fFieldRange(range), fValue(fField->BindValue(objPtr)) + : fField(std::move(field)), fFieldRange(range), fValue(fField->BindValue(std::move(objPtr))) { } @@ -232,7 +232,7 @@ protected: } RNTupleView(std::unique_ptr field, ROOT::RNTupleGlobalRange range, std::shared_ptr objPtr) - : RNTupleViewBase(std::move(field), range, objPtr) + : RNTupleViewBase(std::move(field), range, std::move(objPtr)) { } diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 53f081486c629..2153e79fe3003 100644 --- a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx +++ b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx @@ -118,7 +118,7 @@ class RPageSinkBuf : public RPageSink { ROOT::DescriptorId_t fNColumns = 0; void ConnectFields(const std::vector &fields, ROOT::NTupleSize_t firstEntry); - void FlushClusterImpl(std::function FlushClusterFn); + void FlushClusterImpl(const std::function &FlushClusterFn); void InitImpl(ROOT::RNTupleModel &model) final; RNTupleLink CommitDatasetImpl() final; diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 2cf219e47cb8f..a40177087297a 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -135,8 +135,8 @@ public: SealedPageSequence_t::const_iterator fLast; RSealedPageGroup() = default; - RSealedPageGroup(ROOT::DescriptorId_t d, SealedPageSequence_t::const_iterator b, - SealedPageSequence_t::const_iterator e) + RSealedPageGroup(ROOT::DescriptorId_t d, const SealedPageSequence_t::const_iterator &b, + const SealedPageSequence_t::const_iterator &e) : fPhysicalColumnId(d), fFirst(b), fLast(e) { } @@ -399,7 +399,7 @@ public: virtual void CommitAttributeSet(std::string_view attrSetName, const RNTupleLink &attrAnchorInfo) = 0; /// The registered callback is executed at the beginning of CommitDataset(); - void RegisterOnCommitDatasetCallback(Callback_t callback) { fOnDatasetCommitCallbacks.emplace_back(callback); } + void RegisterOnCommitDatasetCallback(const Callback_t &cb) { fOnDatasetCommitCallbacks.emplace_back(cb); } /// Run the registered callbacks and finalize the current cluster and the entrire data set. RNTupleLink CommitDataset(); @@ -742,8 +742,8 @@ protected: /// commonly used as part of `LoadClusters()` in derived classes. void PrepareLoadCluster( const ROOT::Internal::RCluster::RKey &clusterKey, ROOT::Internal::ROnDiskPageMap &pageZeroMap, - std::function - perPageFunc); + const std::function + &perPageFunc); /// Enables the default set of metrics provided by RPageSource. `prefix` will be used as the prefix for /// the counters registered in the internal RNTupleMetrics object. diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index bbf6549f3f552..256ad69b91e76 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -757,7 +757,7 @@ ROOT::RFieldBase::RBulkValues ROOT::RFieldBase::CreateBulk() ROOT::RFieldBase::RValue ROOT::RFieldBase::BindValue(std::shared_ptr objPtr) { - return RValue(this, objPtr); + return RValue(this, std::move(objPtr)); } std::size_t ROOT::RFieldBase::ReadBulk(const RBulkSpec &bulkSpec) @@ -877,7 +877,7 @@ ROOT::RFieldBase::EnsureCompatibleColumnTypes(const ROOT::RNTupleDescriptor &des "(representation index: " + std::to_string(representationIndex) + ")")); } -size_t ROOT::RFieldBase::AddReadCallback(ReadCallback_t func) +size_t ROOT::RFieldBase::AddReadCallback(const ReadCallback_t &func) { fReadCallbacks.push_back(func); fIsSimple = false; diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index dee66a4774086..7aa98dd427427 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -397,7 +397,7 @@ ROOT::DescriptorId_t ROOT::RClassField::LookupMember(const ROOT::RNTupleDescript return idSourceMember; for (const auto &subFieldDesc : desc.GetFieldIterable(classFieldId)) { - const auto subFieldName = subFieldDesc.GetFieldName(); + const auto &subFieldName = subFieldDesc.GetFieldName(); if (subFieldName.length() > 2 && subFieldName[0] == ':' && subFieldName[1] == '_') { idSourceMember = LookupMember(desc, memberName, subFieldDesc.GetId()); if (idSourceMember != ROOT::kInvalidDescriptorId) @@ -1341,7 +1341,7 @@ class TBufferRecStreamer : public TBufferFile { public: TBufferRecStreamer(TBuffer::EMode mode, Int_t bufsize, RCallbackStreamerInfo callbackStreamerInfo) - : TBufferFile(mode, bufsize), fCallbackStreamerInfo(callbackStreamerInfo) + : TBufferFile(mode, bufsize), fCallbackStreamerInfo(std::move(callbackStreamerInfo)) { } void TagStreamerInfo(TVirtualStreamerInfo *info) final { fCallbackStreamerInfo(info); } diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index de0e859ff920a..8744c886d5f4d 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -14,7 +14,7 @@ namespace { -std::vector SplitVector(std::shared_ptr valuePtr, ROOT::RFieldBase &itemField) +std::vector SplitVector(const std::shared_ptr &valuePtr, ROOT::RFieldBase &itemField) { auto *vec = static_cast *>(valuePtr.get()); const auto itemSize = itemField.GetValueSize(); diff --git a/tree/ntuple/src/RNTupleModel.cxx b/tree/ntuple/src/RNTupleModel.cxx index 19c14c5886569..93d0ec8f7d0cf 100644 --- a/tree/ntuple/src/RNTupleModel.cxx +++ b/tree/ntuple/src/RNTupleModel.cxx @@ -277,8 +277,9 @@ void ROOT::RNTupleModel::RUpdater::AddField(std::unique_ptr fi fOpenChangeset.AddField(std::move(field), parentName); } -ROOT::RResult ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std::unique_ptr field, - RNTupleModel::FieldMappingFunc_t mapping) +ROOT::RResult +ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std::unique_ptr field, + const RNTupleModel::FieldMappingFunc_t &mapping) { auto fieldp = field.get(); auto result = fModel.AddProjectedField(std::move(field), mapping); @@ -288,9 +289,10 @@ ROOT::RResult ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std } ROOT::RResult -ROOT::RNTupleModel::RUpdater::AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping) +ROOT::RNTupleModel::RUpdater::AddProjectedField(std::unique_ptr field, + const FieldMappingFunc_t &mapping) { - return R__FORWARD_RESULT(fOpenChangeset.AddProjectedField(std::move(field), std::move(mapping))); + return R__FORWARD_RESULT(fOpenChangeset.AddProjectedField(std::move(field), mapping)); } void ROOT::RNTupleModel::EnsureValidFieldName(std::string_view fieldName) @@ -458,7 +460,7 @@ void ROOT::RNTupleModel::RegisterSubfield(std::string_view qualifiedFieldName) } ROOT::RResult -ROOT::RNTupleModel::AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping) +ROOT::RNTupleModel::AddProjectedField(std::unique_ptr field, const FieldMappingFunc_t &mapping) { EnsureNotFrozen(); if (!field) diff --git a/tree/ntuple/src/RNTupleProcessor.cxx b/tree/ntuple/src/RNTupleProcessor.cxx index a6e3fb7c21910..b8b251da50fdf 100644 --- a/tree/ntuple/src/RNTupleProcessor.cxx +++ b/tree/ntuple/src/RNTupleProcessor.cxx @@ -78,9 +78,9 @@ ROOT::Experimental::RNTupleProcessor::CreateJoin(RNTupleOpenSpec primaryNTuple, throw RException(R__FAIL("join fields must be unique")); } - std::unique_ptr primaryProcessor = Create(primaryNTuple, processorName); + std::unique_ptr primaryProcessor = Create(std::move(primaryNTuple), processorName); - std::unique_ptr auxProcessor = Create(auxNTuple); + std::unique_ptr auxProcessor = Create(std::move(auxNTuple)); return CreateJoin(std::move(primaryProcessor), std::move(auxProcessor), joinFields, processorName); } diff --git a/tree/ntuple/src/RPageSinkBuf.cxx b/tree/ntuple/src/RPageSinkBuf.cxx index 3b803eb585a26..dff99d93a74d6 100644 --- a/tree/ntuple/src/RPageSinkBuf.cxx +++ b/tree/ntuple/src/RPageSinkBuf.cxx @@ -251,7 +251,7 @@ void ROOT::Internal::RPageSinkBuf::CommitSealedPageV( // We implement both StageCluster() and CommitCluster() because we can call CommitCluster() on the inner sink more // efficiently in a single critical section. For parallel writing, it also guarantees that we produce a fully sequential // file. -void ROOT::Internal::RPageSinkBuf::FlushClusterImpl(std::function FlushClusterFn) +void ROOT::Internal::RPageSinkBuf::FlushClusterImpl(const std::function &FlushClusterFn) { WaitForAllTasks(); assert(fBufferedUncompressed == 0 && "all buffered pages should have been processed"); diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 46ded2885e976..1185cc6d0bd13 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -332,8 +332,8 @@ void ROOT::Internal::RPageSource::UnzipClusterImpl(RCluster *cluster) void ROOT::Internal::RPageSource::PrepareLoadCluster( const RCluster::RKey &clusterKey, ROnDiskPageMap &pageZeroMap, - std::function - perPageFunc) + const std::function + &perPageFunc) { auto descriptorGuard = GetSharedDescriptorGuard(); const auto &clusterDesc = descriptorGuard->GetClusterDescriptor(clusterKey.fClusterId); From adf6babe3812d660f5769d46bac9e52c4cb21886 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 09:20:57 +0200 Subject: [PATCH 30/37] [core] remove unnecessary declaration in RResult --- core/foundation/inc/ROOT/RError.hxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/foundation/inc/ROOT/RError.hxx b/core/foundation/inc/ROOT/RError.hxx index 90c29544c711d..d38712f664183 100644 --- a/core/foundation/inc/ROOT/RError.hxx +++ b/core/foundation/inc/ROOT/RError.hxx @@ -243,8 +243,6 @@ public: RResult &operator=(const RResult &other) = delete; RResult &operator=(RResult &&other) = default; - ~RResult() = default; - /// Used by R__FORWARD_RESULT in order to keep track of the stack trace in case of errors RResult &Forward(RError::RLocation &&sourceLocation) { From d9e5a9d8595b161361528a9a74a1eee7f4eeeffd Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 11:31:55 +0200 Subject: [PATCH 31/37] [ntuple] mark some move constructors noexcept --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 8 ++++---- tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx | 4 ++-- tree/ntuple/inc/ROOT/RNTupleReader.hxx | 4 ++-- tree/ntuple/inc/ROOT/RNTupleView.hxx | 4 ++-- tree/ntuple/inc/ROOT/RPageSinkBuf.hxx | 4 ++-- tree/ntuple/src/RFieldBase.cxx | 4 ++-- tree/ntuple/src/RNTupleReader.cxx | 5 +++-- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index facd30575bf8f..a989ea20788b8 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -775,8 +775,8 @@ public: fTypeInfo = nullptr; return *this; } - RValue(RValue &&other) : fField(other.fField), fObjPtr(std::move(other.fObjPtr)) {} - RValue &operator=(RValue &&other) + RValue(RValue &&other) noexcept : fField(other.fField), fObjPtr(std::move(other.fObjPtr)) {} + RValue &operator=(RValue &&other) noexcept { fField = other.fField; fObjPtr = other.fObjPtr; @@ -912,8 +912,8 @@ public: ~RBulkValues(); RBulkValues(const RBulkValues &) = delete; RBulkValues &operator=(const RBulkValues &) = delete; - RBulkValues(RBulkValues &&other); - RBulkValues &operator=(RBulkValues &&other); + RBulkValues(RBulkValues &&other) noexcept; + RBulkValues &operator=(RBulkValues &&other) noexcept; // Sets `fValues` and `fSize`/`fCapacity` to the given values. The capacity is specified in number of values. // Once a buffer is adopted, an attempt to read more values then available throws an exception. diff --git a/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx b/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx index bc5861fd990e1..76fed96f23ead 100644 --- a/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx @@ -75,10 +75,10 @@ public: RNTupleAttrPendingRange(const RNTupleAttrPendingRange &) = delete; RNTupleAttrPendingRange &operator=(const RNTupleAttrPendingRange &) = delete; - RNTupleAttrPendingRange(RNTupleAttrPendingRange &&other) { *this = std::move(other); } + RNTupleAttrPendingRange(RNTupleAttrPendingRange &&other) noexcept { *this = std::move(other); } // NOTE: explicitly implemented to make sure that 'other' gets invalidated upon move. - RNTupleAttrPendingRange &operator=(RNTupleAttrPendingRange &&other) + RNTupleAttrPendingRange &operator=(RNTupleAttrPendingRange &&other) noexcept { if (&other != this) { std::swap(fStart, other.fStart); diff --git a/tree/ntuple/inc/ROOT/RNTupleReader.hxx b/tree/ntuple/inc/ROOT/RNTupleReader.hxx index 2f9c118f18c38..c93d558c89b81 100644 --- a/tree/ntuple/inc/ROOT/RNTupleReader.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleReader.hxx @@ -173,9 +173,9 @@ public: public: ~RActiveEntryToken() { Reset(); } RActiveEntryToken(const RActiveEntryToken &other); - RActiveEntryToken(RActiveEntryToken &&other); + RActiveEntryToken(RActiveEntryToken &&other) noexcept; RActiveEntryToken &operator=(const RActiveEntryToken &other); - RActiveEntryToken &operator=(RActiveEntryToken &&other); + RActiveEntryToken &operator=(RActiveEntryToken &&other) noexcept; NTupleSize_t GetEntryNumber() const { return fEntryNumber; } /// Set or replace the entry number. If the entry number is replaced, the cluster corresponding to the new diff --git a/tree/ntuple/inc/ROOT/RNTupleView.hxx b/tree/ntuple/inc/ROOT/RNTupleView.hxx index f963b1c3793c9..8acad94ebbacd 100644 --- a/tree/ntuple/inc/ROOT/RNTupleView.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleView.hxx @@ -365,11 +365,11 @@ private: public: RNTupleCollectionView(const RNTupleCollectionView &other) = delete; RNTupleCollectionView &operator=(const RNTupleCollectionView &other) = delete; - RNTupleCollectionView(RNTupleCollectionView &&other) + RNTupleCollectionView(RNTupleCollectionView &&other) noexcept(false) : fSource(other.fSource), fField(std::move(other.fField)), fValue(fField.CreateValue()) { } - RNTupleCollectionView &operator=(RNTupleCollectionView &&other) + RNTupleCollectionView &operator=(RNTupleCollectionView &&other) noexcept(false) { if (this == &other) return *this; diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 2153e79fe3003..56f26e0b9a239 100644 --- a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx +++ b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx @@ -52,8 +52,8 @@ class RPageSinkBuf : public RPageSink { RColumnBuf() = default; RColumnBuf(const RColumnBuf&) = delete; RColumnBuf& operator=(const RColumnBuf&) = delete; - RColumnBuf(RColumnBuf&&) = default; - RColumnBuf& operator=(RColumnBuf&&) = default; + RColumnBuf(RColumnBuf&&) noexcept = default; + RColumnBuf& operator=(RColumnBuf&&) noexcept = default; ~RColumnBuf() { DropBufferedPages(); } /// Returns a reference to the newly buffered page. The reference remains diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index 256ad69b91e76..b2aa1cce993b3 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -121,7 +121,7 @@ void ROOT::RFieldBase::RValue::BindRawPtr(void *rawPtr) //------------------------------------------------------------------------------ -ROOT::RFieldBase::RBulkValues::RBulkValues(RBulkValues &&other) +ROOT::RFieldBase::RBulkValues::RBulkValues(RBulkValues &&other) noexcept : fField(other.fField), fValueSize(other.fValueSize), fCapacity(other.fCapacity), @@ -135,7 +135,7 @@ ROOT::RFieldBase::RBulkValues::RBulkValues(RBulkValues &&other) std::swap(fMaskAvail, other.fMaskAvail); } -ROOT::RFieldBase::RBulkValues &ROOT::RFieldBase::RBulkValues::operator=(RBulkValues &&other) +ROOT::RFieldBase::RBulkValues &ROOT::RFieldBase::RBulkValues::operator=(RBulkValues &&other) noexcept { std::swap(fField, other.fField); std::swap(fDeleter, other.fDeleter); diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index b2d0c4f8e5e62..3672603186efc 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -84,7 +84,7 @@ ROOT::RNTupleReader::RActiveEntryToken::RActiveEntryToken(const RActiveEntryToke SetEntryNumber(other.fEntryNumber); } -ROOT::RNTupleReader::RActiveEntryToken::RActiveEntryToken(RActiveEntryToken &&other) +ROOT::RNTupleReader::RActiveEntryToken::RActiveEntryToken(RActiveEntryToken &&other) noexcept { std::swap(fEntryNumber, other.fEntryNumber); std::swap(fPtrControlBlock, other.fPtrControlBlock); @@ -105,7 +105,8 @@ ROOT::RNTupleReader::RActiveEntryToken::operator=(const RActiveEntryToken &other return *this; } -ROOT::RNTupleReader::RActiveEntryToken &ROOT::RNTupleReader::RActiveEntryToken::operator=(RActiveEntryToken &&other) +ROOT::RNTupleReader::RActiveEntryToken & +ROOT::RNTupleReader::RActiveEntryToken::operator=(RActiveEntryToken &&other) noexcept { std::swap(fEntryNumber, other.fEntryNumber); std::swap(fPtrControlBlock, other.fPtrControlBlock); From be44b2788ccc4410eb7291f5047944a77f3cfb5e Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 11:35:08 +0200 Subject: [PATCH 32/37] [core] make moving TTaskGroup noexcept --- core/imt/inc/ROOT/TTaskGroup.hxx | 4 ++-- core/imt/src/TTaskGroup.cxx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/imt/inc/ROOT/TTaskGroup.hxx b/core/imt/inc/ROOT/TTaskGroup.hxx index 547819d8f9fc4..bea2a560a14f4 100644 --- a/core/imt/inc/ROOT/TTaskGroup.hxx +++ b/core/imt/inc/ROOT/TTaskGroup.hxx @@ -38,9 +38,9 @@ private: public: TTaskGroup(); - TTaskGroup(TTaskGroup &&other); + TTaskGroup(TTaskGroup &&other) noexcept; TTaskGroup(const TTaskGroup &) = delete; - TTaskGroup &operator=(TTaskGroup &&other); + TTaskGroup &operator=(TTaskGroup &&other) noexcept; ~TTaskGroup(); void Cancel(); diff --git a/core/imt/src/TTaskGroup.cxx b/core/imt/src/TTaskGroup.cxx index 1fdd12314dbbf..660194bf1d975 100644 --- a/core/imt/src/TTaskGroup.cxx +++ b/core/imt/src/TTaskGroup.cxx @@ -66,12 +66,12 @@ TTaskGroup::TTaskGroup() #endif } -TTaskGroup::TTaskGroup(TTaskGroup &&other) +TTaskGroup::TTaskGroup(TTaskGroup &&other) noexcept { *this = std::move(other); } -TTaskGroup &TTaskGroup::operator=(TTaskGroup &&other) +TTaskGroup &TTaskGroup::operator=(TTaskGroup &&other) noexcept { fTaskArenaW = other.fTaskArenaW; fTaskContainer = other.fTaskContainer; From d3cec4d4fd982e69adf191a9ce2f5637a3a9e04c Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:38:41 +0200 Subject: [PATCH 33/37] [RVec] improve noexcept specification --- math/vecops/inc/ROOT/RVec.hxx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 4e6bf5063bb5d..178f2faa3ae2a 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -559,6 +559,7 @@ namespace VecOps { template class R__CLING_PTRCHECK(off) RVecImpl : public Internal::VecOps::SmallVectorTemplateBase { using SuperClass = Internal::VecOps::SmallVectorTemplateBase; + static constexpr bool kIsNoExcept = std::is_nothrow_destructible_v && std::is_nothrow_move_constructible_v; public: using iterator = typename SuperClass::iterator; @@ -927,7 +928,7 @@ public: RVecImpl &operator=(const RVecImpl &RHS); - RVecImpl &operator=(RVecImpl &&RHS); + RVecImpl &operator=(RVecImpl &&RHS) noexcept(kIsNoExcept); }; template @@ -1044,7 +1045,7 @@ RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) } template -RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) +RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) noexcept(kIsNoExcept) { // Avoid self-assignment. if (this == &RHS) @@ -1192,7 +1193,7 @@ public: return *this; } - RVecN(RVecN &&RHS) : Detail::VecOps::RVecImpl(N) + RVecN(RVecN &&RHS) noexcept(false): Detail::VecOps::RVecImpl(N) { if (!RHS.empty()) Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); @@ -1206,7 +1207,7 @@ public: RVecN(const std::vector &RHS) : RVecN(RHS.begin(), RHS.end()) {} - RVecN &operator=(RVecN &&RHS) + RVecN &operator=(RVecN &&RHS) noexcept(std::is_nothrow_move_assignable_v>) { Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); return *this; @@ -1560,9 +1561,9 @@ public: return *this; } - RVec(RVec &&RHS) : SuperClass(std::move(RHS)) {} + RVec(RVec &&RHS) noexcept(std::is_nothrow_move_constructible_v) : SuperClass(std::move(RHS)) {} - RVec &operator=(RVec &&RHS) + RVec &operator=(RVec &&RHS) noexcept(std::is_nothrow_move_assignable_v) { SuperClass::operator=(std::move(RHS)); return *this; From 3b1983e2b9408e564f978de020c6999b3baed939 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:45:54 +0200 Subject: [PATCH 34/37] [ntuple] fix virtual destructor visibility --- tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx | 3 +-- tree/ntuple/inc/ROOT/RFieldVisitor.hxx | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx index 987f6799a81a8..044bce5e3dfb1 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx @@ -412,14 +412,13 @@ protected: fPrincipalColumn = fAvailableColumns[0].get(); } - ~RRealField() override = default; - public: using Base::SetColumnRepresentatives; RRealField(std::string_view name, std::string_view typeName) : RSimpleField(name, typeName) {} RRealField(RRealField &&other) = default; RRealField &operator=(RRealField &&other) = default; + ~RRealField() override = default; /// Sets this field to use a half precision representation, occupying half as much storage space (16 bits: /// 1 sign bit, 5 exponent bits, 10 mantissa bits) on disk. diff --git a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx index 8e2fb0c53a71b..01bca5bf7a49b 100644 --- a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx +++ b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx @@ -56,6 +56,8 @@ As an example of a concrete use case, see Internal::RPrintSchemaVisitor. // clang-format on class RFieldVisitor { public: + virtual ~RFieldVisitor() = default; + virtual void VisitField(const ROOT::RFieldBase &field) = 0; virtual void VisitFieldZero(const ROOT::RFieldZero &field) { VisitField(field); } virtual void VisitArrayField(const ROOT::RArrayField &field) { VisitField(field); } From 03594e6a8325401c2907e171d27c6823f62fb489 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:52:06 +0200 Subject: [PATCH 35/37] [ntuple] fix up RNTupleDescriptorBuilder::SetNTuple() signature --- tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx | 2 +- tree/ntuple/src/RNTupleDescriptor.cxx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx index 6a1c35ea264ac..f8c42e30f6e4f 100644 --- a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx @@ -1763,7 +1763,7 @@ public: std::uint16_t versionPatch); void SetVersionForWriting(); - void SetNTuple(const std::string_view name, const std::string_view description); + void SetNTuple(std::string_view name, std::string_view description); /// Sets the `flag`-th bit of the feature flag to 1. /// Note that `flag` itself is not a bitmask, just the bit index of the flag to enable. void SetFeature(unsigned int flag); diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index ebfed9d89b747..0e86459c8b663 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -1093,8 +1093,7 @@ void ROOT::Internal::RNTupleDescriptorBuilder::SetVersionForWriting() fDescriptor.fVersionPatch = RNTuple::kVersionPatch; } -void ROOT::Internal::RNTupleDescriptorBuilder::SetNTuple(const std::string_view name, - const std::string_view description) +void ROOT::Internal::RNTupleDescriptorBuilder::SetNTuple(std::string_view name, std::string_view description) { fDescriptor.fName = std::string(name); fDescriptor.fDescription = std::string(description); From 3f8c93d9c771cfb6268f61c50e1638f0b6ebe47f Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:57:12 +0200 Subject: [PATCH 36/37] [ntuple] remove duplicate include --- tree/ntuple/src/RPageStorage.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 1185cc6d0bd13..5183314b0159c 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -23,7 +23,6 @@ #include #include #include -#include #ifdef R__ENABLE_DAOS #include #endif From 8b910fd4cd9f646c163185c5f90257860f1cb5df Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:58:55 +0200 Subject: [PATCH 37/37] [core] minor improvement in TIterator --- core/cont/inc/TCollection.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/cont/inc/TCollection.h b/core/cont/inc/TCollection.h index 359154c52c08e..9dfcd096646fc 100644 --- a/core/cont/inc/TCollection.h +++ b/core/cont/inc/TCollection.h @@ -267,8 +267,7 @@ class TIter { } TIter &operator=(TIterator *iter) { - if (fIterator) - delete fIterator; + delete fIterator; fIterator = iter; return *this; }