diff --git a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx index 9caf202bd0171..044bce5e3dfb1 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 @@ -419,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/RField/RFieldSTLMisc.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx index 0a21ecb0c28fe..908f24706a8ad 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 diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 6e691a9f98d95..a60ee5c2c83bf 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -511,7 +511,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 @@ -778,7 +778,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) {} @@ -790,8 +790,8 @@ public: fTypeInfo = nullptr; return *this; } - RValue(RValue &&other) : fField(other.fField), fObjPtr(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; @@ -829,7 +829,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(); } @@ -927,8 +927,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/RFieldVisitor.hxx b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx index 41af5a1b76e18..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); } @@ -199,7 +201,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/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/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/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..c93d558c89b81 100644 --- a/tree/ntuple/inc/ROOT/RNTupleReader.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleReader.hxx @@ -166,16 +166,16 @@ public: void DeactivateEntry(NTupleSize_t entryNumber); explicit RActiveEntryToken(std::shared_ptr ptrControlBlock) - : fPtrControlBlock(ptrControlBlock) + : fPtrControlBlock(std::move(ptrControlBlock)) { } 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 7dcf187e6f3c7..8acad94ebbacd 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)) { } @@ -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/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/RPageNullSink.hxx b/tree/ntuple/inc/ROOT/RPageNullSink.hxx index e46951cb3fb80..b72c104171537 100644 --- a/tree/ntuple/inc/ROOT/RPageNullSink.hxx +++ b/tree/ntuple/inc/ROOT/RPageNullSink.hxx @@ -36,6 +36,14 @@ class RPageNullSink : public ROOT::Internal::RPageSink { ROOT::DescriptorId_t fNColumns = 0; std::uint64_t fNBytesCurrentCluster = 0; +protected: + void InitImpl(ROOT::RNTupleModel &model) final + { + auto &fieldZero = ROOT::Internal::GetFieldZeroOfModel(model); + ConnectFields(fieldZero.GetMutableSubfields(), 0); + } + ROOT::Internal::RNTupleLink CommitDatasetImpl() final { return {}; } + public: RPageNullSink(std::string_view ntupleName, const ROOT::RNTupleWriteOptions &options) : RPageSink(ntupleName, options) { @@ -66,11 +74,6 @@ public: } } } - void InitImpl(ROOT::RNTupleModel &model) final - { - auto &fieldZero = ROOT::Internal::GetFieldZeroOfModel(model); - ConnectFields(fieldZero.GetMutableSubfields(), 0); - } void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) final { ConnectFields(changeset.fAddedFields, firstEntry); @@ -104,7 +107,6 @@ public: } void CommitStagedClusters(std::span) final {} void CommitClusterGroup() final {} - ROOT::Internal::RNTupleLink CommitDatasetImpl() final { return {}; } void CommitAttributeSet(std::string_view, const ROOT::Internal::RNTupleLink &) final {} std::unique_ptr CloneAsHidden(std::string_view, const RNTupleWriteOptions &) const final 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); diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 8cda1d843c71e..da6d8db1a6a9f 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 { @@ -53,8 +52,8 @@ private: 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 @@ -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; @@ -120,7 +118,10 @@ private: 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; public: explicit RPageSinkBuf(std::unique_ptr inner); @@ -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 5a0723fd0248d..df661029f5d8f 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(); @@ -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 @@ -744,10 +746,10 @@ protected: /// Prepare a page range read for the column set in `clusterKey`. Specifically, pages referencing the /// `kTypePageZero` locator are filled in `pageZeroMap`; otherwise, `perPageFunc` is called for each page. This is /// commonly used as part of `LoadClusters()` in derived classes. - void PrepareLoadCluster( - const ROOT::Internal::RCluster::RKey &clusterKey, ROOT::Internal::ROnDiskPageMap &pageZeroMap, - std::function - perPageFunc); + void PrepareLoadCluster(const ROOT::Internal::RCluster::RKey &clusterKey, + ROOT::Internal::ROnDiskPageMap &pageZeroMap, + 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/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; diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index e185389f1190a..1a896167609de 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -129,7 +129,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), @@ -143,7 +143,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); @@ -438,7 +438,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)); @@ -774,7 +773,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) @@ -894,7 +893,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 cb62fe1e33291..5b97f86383703 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -402,7 +402,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) @@ -1250,14 +1250,14 @@ std::unique_ptr ROOT::RProxiedCollectionField::GetDe ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter( std::shared_ptr proxy) - : RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), fProxy(proxy) + : RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), fProxy(std::move(proxy)) { } ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter( std::shared_ptr proxy, std::unique_ptr itemDeleter, size_t itemSize) : RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), - fProxy(proxy), + fProxy(std::move(proxy)), fItemDeleter(std::move(itemDeleter)), fItemSize(itemSize) { @@ -1391,7 +1391,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 4e42fc1b6b32d..1d5a713532fe7 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -15,7 +15,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/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. 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/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 35f28cb666b71..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; @@ -1221,8 +1220,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 +1268,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 +1308,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 +1320,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/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index b1e8407b86b68..0e86459c8b663 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 && @@ -143,7 +141,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; @@ -720,7 +718,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; @@ -1095,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); @@ -1300,9 +1297,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(); } 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/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..3c7cadc71b76b 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; @@ -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 << ')'; @@ -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/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/RNTupleModel.cxx b/tree/ntuple/src/RNTupleModel.cxx index d326194e31af9..77e2bc6792d11 100644 --- a/tree/ntuple/src/RNTupleModel.cxx +++ b/tree/ntuple/src/RNTupleModel.cxx @@ -197,8 +197,14 @@ 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() @@ -270,8 +276,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); @@ -280,10 +287,10 @@ ROOT::RResult ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std return R__FORWARD_RESULT(result); } -ROOT::RResult -ROOT::RNTupleModel::RUpdater::AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping) +ROOT::RResult 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) @@ -451,7 +458,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/RNTupleParallelWriter.cxx b/tree/ntuple/src/RNTupleParallelWriter.cxx index ce3ee96e3c557..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 @@ -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/RNTupleProcessor.cxx b/tree/ntuple/src/RNTupleProcessor.cxx index 3a00539e0a82c..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); } @@ -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, diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index e26aaba93323b..3672603186efc 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()); @@ -85,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); @@ -106,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); @@ -310,18 +310,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() 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(); } } diff --git a/tree/ntuple/src/RPageSinkBuf.cxx b/tree/ntuple/src/RPageSinkBuf.cxx index 1fed3a0138ced..7720239f1cb83 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() { @@ -261,7 +260,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 ee3269b2be9a6..581a72f1af6c9 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; @@ -333,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); @@ -1227,8 +1226,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; diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index aad35a36cea88..0d8319538b34e 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -45,16 +45,13 @@ 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; using ROOT::Internal::RNTupleSerializer; using ROOT::Internal::ROnDiskPage; using ROOT::Internal::ROnDiskPageMap; -using ROOT::Internal::RPagePool; ROOT::Internal::RPageSinkFile::RPageSinkFile(std::string_view ntupleName, const ROOT::RNTupleWriteOptions &options) : RPagePersistentSink(ntupleName, options) @@ -74,7 +71,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, @@ -395,9 +392,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") { @@ -676,7 +672,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)); } diff --git a/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx b/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx index 912ea6e0cc5bd..ab68dd70faef4 100644 --- a/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx +++ b/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx @@ -254,7 +254,7 @@ public: static std::unique_ptr Create(TTree *sourceTree, std::string_view destFileName); ROOT::RNTupleWriteOptions GetWriteOptions() const { return fWriteOptions; } - void SetWriteOptions(ROOT::RNTupleWriteOptions options) { fWriteOptions = options; } + void SetWriteOptions(const ROOT::RNTupleWriteOptions &options) { fWriteOptions = options; } void SetNTupleName(const std::string &name) { fNTupleName = name; } void SetMaxEntries(std::uint64_t maxEntries) { fMaxEntries = maxEntries; }; @@ -267,7 +267,7 @@ public: /// Add custom method to adjust column representations. Will be called for every field of the frozen model /// before it is attached to the page sink - void SetFieldModifier(FieldModifier_t modifier) { fFieldModifier = modifier; } + void SetFieldModifier(const FieldModifier_t &modifier) { fFieldModifier = modifier; } /// Import works in two steps: /// 1. PrepareSchema() calls SetBranchAddress() on all the TTree branches and creates the corresponding RNTuple diff --git a/tree/ntupleutil/src/RNTupleInspector.cxx b/tree/ntupleutil/src/RNTupleInspector.cxx index 1a5b192f6ad53..2eb11d339b22e 100644 --- a/tree/ntupleutil/src/RNTupleInspector.cxx +++ b/tree/ntupleutil/src/RNTupleInspector.cxx @@ -73,7 +73,7 @@ void ROOT::Experimental::RNTupleInspector::CollectColumnInfo() nElems += columnRange.GetNElements(); if (!fCompressionSettings && columnRange.GetCompressionSettings()) { - fCompressionSettings = *columnRange.GetCompressionSettings(); + fCompressionSettings = columnRange.GetCompressionSettings(); } else if (fCompressionSettings && columnRange.GetCompressionSettings() && (*fCompressionSettings != *columnRange.GetCompressionSettings())) { // Note that currently all clusters and columns are compressed with the same settings and it is not yet @@ -265,21 +265,20 @@ void ROOT::Experimental::RNTupleInspector::PrintColumnTypeInfo(ENTupleInspectorP output << " column type | count | # elements | compressed bytes | uncompressed bytes | compression ratio | " "# pages \n" << "----------------|---------|-------------|------------------|--------------------|-------------------|-" - "------" - << std::endl; + "------\n"; for (const auto &[colType, typeInfo] : colTypeInfo) output << std::setw(15) << RColumnElementBase::GetColumnTypeName(colType) << " |" << std::setw(8) << typeInfo.count << " |" << std::setw(12) << typeInfo.nElems << " |" << std::setw(17) << typeInfo.compressedSize << " |" << std::setw(19) << typeInfo.uncompressedSize << " |" << std::fixed << std::setprecision(3) << std::setw(18) << typeInfo.GetCompressionFactor() << " |" << std::setw(6) - << typeInfo.nPages << " " << std::endl; + << typeInfo.nPages << " \n"; break; case ENTupleInspectorPrintFormat::kCSV: - output << "columnType,count,nElements,compressedSize,uncompressedSize,compressionFactor,nPages" << std::endl; + output << "columnType,count,nElements,compressedSize,uncompressedSize,compressionFactor,nPages\n"; for (const auto &[colType, typeInfo] : colTypeInfo) { output << RColumnElementBase::GetColumnTypeName(colType) << "," << typeInfo.count << "," << typeInfo.nElems << "," << typeInfo.compressedSize << "," << typeInfo.uncompressedSize << "," << std::fixed - << std::setprecision(3) << typeInfo.GetCompressionFactor() << "," << typeInfo.nPages << std::endl; + << std::setprecision(3) << typeInfo.GetCompressionFactor() << "," << typeInfo.nPages << '\n'; } break; default: R__ASSERT(false && "Invalid print format");