From c5048fad38e15df49cea3737e915891a28ad834e Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 2 Nov 2023 22:20:14 +0100 Subject: [PATCH] Apply suggestions from clang-tidy to `dataframe` and `rntuple` Applies the clang-tidy suggestions from the following checks, which are the same as in the CMSSW `.clang-tidy`, only that the checks that replace copies with `const` references are removed: ``` ,boost-use-to-string, ,misc-definitions-in-headers, ,misc-string-compare, ,misc-uniqueptr-reset-release, ,modernize-deprecated-headers, ,modernize-make-shared, ,modernize-make-unique, ,modernize-use-bool-literals, ,modernize-use-equals-delete, ,modernize-use-nullptr, ,modernize-use-override, ,performance-inefficient-algorithm, ,performance-inefficient-vector-operation, ,performance-faster-string-find, ,performance-move-const-arg, ,readability-container-size-empty, ,readability-redundant-string-cstr, ,readability-static-definition-in-anonymous-namespace, ,readability-uniqueptr-delete-release ``` --- .../dataframe/inc/ROOT/RDF/InterfaceUtils.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RInterface.hxx | 4 +-- .../dataframe/inc/ROOT/RDF/RInterfaceBase.hxx | 4 +-- tree/dataframe/inc/ROOT/RDataFrame.hxx | 2 +- tree/dataframe/src/RArrowDS.cxx | 36 +++++++++---------- tree/dataframe/src/RDFHelpers.cxx | 4 +-- tree/dataframe/src/RDFHistoModels.cxx | 4 +-- tree/dataframe/src/RDFInterfaceUtils.cxx | 20 +++++------ tree/dataframe/src/RDFUtils.cxx | 4 +-- tree/dataframe/src/RDataFrame.cxx | 2 +- tree/dataframe/src/RLoopManager.cxx | 16 ++++----- tree/dataframe/src/RSqliteDS.cxx | 8 ++--- tree/dataframe/src/RVariationsDescription.cxx | 2 +- tree/dataframe/test/dataframe_datasetspec.cxx | 2 +- tree/dataframe/test/dataframe_simple.cxx | 3 +- tree/dataframe/test/dataframe_snapshot.cxx | 2 +- tree/dataframe/test/dataframe_vary.cxx | 4 +-- tree/ntuple/v7/inc/ROOT/RColumnElement.hxx | 2 +- tree/ntuple/v7/inc/ROOT/RField.hxx | 4 +-- tree/ntuple/v7/inc/ROOT/RNTuple.hxx | 3 +- tree/ntuple/v7/inc/ROOT/RNTupleZip.hxx | 4 +-- tree/ntuple/v7/src/RClusterPool.cxx | 2 +- tree/ntuple/v7/src/RField.cxx | 15 +++++--- tree/ntuple/v7/src/RMiniFile.cxx | 4 +-- tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx | 4 +-- tree/ntuple/v7/src/RPageSinkBuf.cxx | 3 +- tree/ntuple/v7/src/RPageSourceFriends.cxx | 1 + tree/ntuple/v7/src/RPageStorage.cxx | 5 +-- tree/ntuple/v7/test/ntuple_basics.cxx | 2 +- tree/ntuple/v7/test/ntuple_modelext.cxx | 2 +- .../v7/inc/ROOT/RNTupleImporter.hxx | 4 +-- tree/ntupleutil/v7/src/RNTupleImporter.cxx | 2 +- tree/ntupleutil/v7/src/RNTupleInspector.cxx | 4 +-- tree/ntupleutil/v7/test/ntuple_inspector.cxx | 4 +-- 34 files changed, 97 insertions(+), 87 deletions(-) diff --git a/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx b/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx index 57ece02efa1d4..7ab241761bc63 100644 --- a/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx +++ b/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx @@ -571,7 +571,7 @@ void JitVariationHelper(F &&f, const char **colsPtr, std::size_t colsSize, const // use unique_ptr instead of make_unique to reduce jit/compile-times std::unique_ptr newVariation{new RVariation, IsSingleColumn>( std::move(variedColNames), variationName, std::forward(f), std::move(tags), jittedVariation->GetTypeName(), - *colRegister, *lm, std::move(inputColNames))}; + *colRegister, *lm, inputColNames)}; jittedVariation->SetVariation(std::move(newVariation)); doDeletes(); diff --git a/tree/dataframe/inc/ROOT/RDF/RInterface.hxx b/tree/dataframe/inc/ROOT/RDF/RInterface.hxx index 525e305b25ebb..899ed99b9eef2 100644 --- a/tree/dataframe/inc/ROOT/RDF/RInterface.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RInterface.hxx @@ -2925,8 +2925,8 @@ private: const std::vector &variationTags, std::string_view variationName, bool isSingleColumn) { - R__ASSERT(variationTags.size() > 0 && "Must have at least one variation."); - R__ASSERT(colNames.size() > 0 && "Must have at least one varied column."); + R__ASSERT(!variationTags.empty() && "Must have at least one variation."); + R__ASSERT(!colNames.empty() && "Must have at least one varied column."); R__ASSERT(!variationName.empty() && "Must provide a variation name."); for (auto &colName : colNames) { diff --git a/tree/dataframe/inc/ROOT/RDF/RInterfaceBase.hxx b/tree/dataframe/inc/ROOT/RDF/RInterfaceBase.hxx index 83db637e12296..16dfc1f49a3ca 100644 --- a/tree/dataframe/inc/ROOT/RDF/RInterfaceBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RInterfaceBase.hxx @@ -71,8 +71,8 @@ protected: void SanityChecksForVary(const std::vector &colNames, const std::vector &variationTags, std::string_view variationName) { - R__ASSERT(variationTags.size() > 0 && "Must have at least one variation."); - R__ASSERT(colNames.size() > 0 && "Must have at least one varied column."); + R__ASSERT(!variationTags.empty() && "Must have at least one variation."); + R__ASSERT(!colNames.empty() && "Must have at least one varied column."); R__ASSERT(!variationName.empty() && "Must provide a variation name."); for (auto &colName : colNames) { diff --git a/tree/dataframe/inc/ROOT/RDataFrame.hxx b/tree/dataframe/inc/ROOT/RDataFrame.hxx index eb71345cb0a2e..b33755676ac4f 100644 --- a/tree/dataframe/inc/ROOT/RDataFrame.hxx +++ b/tree/dataframe/inc/ROOT/RDataFrame.hxx @@ -46,7 +46,7 @@ public: const ColumnNames_t &defaultColumns = {}); RDataFrame(std::string_view treename, std::initializer_list filenames, const ColumnNames_t &defaultColumns = {}): - RDataFrame(treename, std::vector(std::move(filenames)), defaultColumns) {} + RDataFrame(treename, std::vector(filenames), defaultColumns) {} RDataFrame(std::string_view treeName, ::TDirectory *dirPtr, const ColumnNames_t &defaultColumns = {}); RDataFrame(TTree &tree, const ColumnNames_t &defaultColumns = {}); RDataFrame(ULong64_t numEntries); diff --git a/tree/dataframe/src/RArrowDS.cxx b/tree/dataframe/src/RArrowDS.cxx index 14abb60070f4b..8708b73f9b960 100644 --- a/tree/dataframe/src/RArrowDS.cxx +++ b/tree/dataframe/src/RArrowDS.cxx @@ -115,58 +115,58 @@ class ArrayPtrVisitor : public ::arrow::ArrayVisitor { void SetEntry(ULong64_t entry) { fCurrentEntry = entry; } /// Check if we are asking the same entry as before. - virtual arrow::Status Visit(arrow::Int32Array const &array) final + arrow::Status Visit(arrow::Int32Array const &array) final { *fResult = (void *)(array.raw_values() + fCurrentEntry); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::Int64Array const &array) final + arrow::Status Visit(arrow::Int64Array const &array) final { *fResult = (void *)(array.raw_values() + fCurrentEntry); return arrow::Status::OK(); } /// Check if we are asking the same entry as before. - virtual arrow::Status Visit(arrow::UInt32Array const &array) final + arrow::Status Visit(arrow::UInt32Array const &array) final { *fResult = (void *)(array.raw_values() + fCurrentEntry); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::UInt64Array const &array) final + arrow::Status Visit(arrow::UInt64Array const &array) final { *fResult = (void *)(array.raw_values() + fCurrentEntry); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::FloatArray const &array) final + arrow::Status Visit(arrow::FloatArray const &array) final { *fResult = (void *)(array.raw_values() + fCurrentEntry); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::DoubleArray const &array) final + arrow::Status Visit(arrow::DoubleArray const &array) final { *fResult = (void *)(array.raw_values() + fCurrentEntry); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::BooleanArray const &array) final + arrow::Status Visit(arrow::BooleanArray const &array) final { fCachedBool = array.Value(fCurrentEntry); *fResult = reinterpret_cast(&fCachedBool); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::StringArray const &array) final + arrow::Status Visit(arrow::StringArray const &array) final { fCachedString = array.GetString(fCurrentEntry); *fResult = reinterpret_cast(&fCachedString); return arrow::Status::OK(); } - virtual arrow::Status Visit(arrow::ListArray const &array) final + arrow::Status Visit(arrow::ListArray const &array) final { switch (array.value_type()->id()) { case arrow::Type::FLOAT: { @@ -367,15 +367,15 @@ class RDFTypeNameGetter : public ::arrow::TypeVisitor { class VerifyValidColumnType : public ::arrow::TypeVisitor { private: public: - virtual arrow::Status Visit(const arrow::Int64Type &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::UInt64Type &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::Int32Type &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::UInt32Type &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::FloatType &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::DoubleType &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::StringType &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::BooleanType &) override { return arrow::Status::OK(); } - virtual arrow::Status Visit(const arrow::ListType &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::Int64Type &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::UInt64Type &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::Int32Type &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::UInt32Type &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::FloatType &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::DoubleType &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::StringType &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::BooleanType &) override { return arrow::Status::OK(); } + arrow::Status Visit(const arrow::ListType &) override { return arrow::Status::OK(); } using ::arrow::TypeVisitor::Visit; }; diff --git a/tree/dataframe/src/RDFHelpers.cxx b/tree/dataframe/src/RDFHelpers.cxx index 12413009a2e32..933e2d9906e69 100644 --- a/tree/dataframe/src/RDFHelpers.cxx +++ b/tree/dataframe/src/RDFHelpers.cxx @@ -24,7 +24,7 @@ #include #include #include -#include +#include // TODO, this function should be part of core libraries #include @@ -355,7 +355,7 @@ class ProgressBarAction final : public ROOT::Detail::RDF::RActionImplfHelper->registerNewSample(slot, id); diff --git a/tree/dataframe/src/RDFHistoModels.cxx b/tree/dataframe/src/RDFHistoModels.cxx index d282edeb78516..2fe3453df81cb 100644 --- a/tree/dataframe/src/RDFHistoModels.cxx +++ b/tree/dataframe/src/RDFHistoModels.cxx @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include "TAxis.h" @@ -268,7 +268,7 @@ std::shared_ptr<::THnD> THnDModel::GetHistogram() const { bool varbinning = false; for (const auto &bins : fBinEdges) { - if (bins.size()) { + if (!bins.empty()) { varbinning = true; break; } diff --git a/tree/dataframe/src/RDFInterfaceUtils.cxx b/tree/dataframe/src/RDFInterfaceUtils.cxx index 791eeb5a103d9..30a4b697f322e 100644 --- a/tree/dataframe/src/RDFInterfaceUtils.cxx +++ b/tree/dataframe/src/RDFInterfaceUtils.cxx @@ -92,7 +92,7 @@ struct ParsedExpression { }; /// Look at expression `expr` and return a pair of (column names used, aliases used) -static std::pair +std::pair FindUsedColsAndAliases(const std::string &expr, const ColumnNames_t &treeBranchNames, const ROOT::Internal::RDF::RColumnRegister &colRegister, const ColumnNames_t &dataSourceColNames) { @@ -155,7 +155,7 @@ FindUsedColsAndAliases(const std::string &expr, const ColumnNames_t &treeBranchN } /// Substitute each '.' in a string with '\.' -static std::string EscapeDots(const std::string &s) +std::string EscapeDots(const std::string &s) { TString out(s); TPRegexp dot("\\."); @@ -163,7 +163,7 @@ static std::string EscapeDots(const std::string &s) return std::string(std::move(out)); } -static TString ResolveAliases(const TString &expr, const ColumnNames_t &usedAliases, +TString ResolveAliases(const TString &expr, const ColumnNames_t &usedAliases, const ROOT::Internal::RDF::RColumnRegister &colRegister) { TString out(expr); @@ -177,7 +177,7 @@ static TString ResolveAliases(const TString &expr, const ColumnNames_t &usedAlia return out; } -static ParsedExpression ParseRDFExpression(std::string_view expr, const ColumnNames_t &treeBranchNames, +ParsedExpression ParseRDFExpression(std::string_view expr, const ColumnNames_t &treeBranchNames, const ROOT::Internal::RDF::RColumnRegister &colRegister, const ColumnNames_t &dataSourceColNames) { @@ -226,12 +226,12 @@ static ParsedExpression ParseRDFExpression(std::string_view expr, const ColumnNa /// jitted variable that corresponds to that expression. For example, for: /// auto f1(){ return 42; } /// key would be "(){ return 42; }" and value would be "f1". -static std::unordered_map &GetJittedExprs() { +std::unordered_map &GetJittedExprs() { static std::unordered_map jittedExpressions; return jittedExpressions; } -static std::string +std::string BuildFunctionString(const std::string &expr, const ColumnNames_t &vars, const ColumnNames_t &varTypes) { assert(vars.size() == varTypes.size()); @@ -301,7 +301,7 @@ BuildFunctionString(const std::string &expr, const ColumnNames_t &vars, const Co /// Declare a function to the interpreter in namespace R_rdf, return the name of the jitted function. /// If the function is already in GetJittedExprs, return the name for the function that has already been jitted. -static std::string DeclareFunction(const std::string &expr, const ColumnNames_t &vars, const ColumnNames_t &varTypes) +std::string DeclareFunction(const std::string &expr, const ColumnNames_t &vars, const ColumnNames_t &varTypes) { R__LOCKGUARD(gROOTMutex); @@ -321,7 +321,7 @@ static std::string DeclareFunction(const std::string &expr, const ColumnNames_t const auto toDeclare = "namespace R_rdf {\nauto " + funcBaseName + funcCode + "\nusing " + funcBaseName + "_ret_t = typename ROOT::TypeTraits::CallableTraits::ret_type;\n}"; - ROOT::Internal::RDF::InterpreterDeclare(toDeclare.c_str()); + ROOT::Internal::RDF::InterpreterDeclare(toDeclare); // InterpreterDeclare could throw. If it doesn't, mark the function as already jitted exprMap.insert({funcCode, funcFullName}); @@ -331,7 +331,7 @@ static std::string DeclareFunction(const std::string &expr, const ColumnNames_t /// Each jitted function comes with a func_ret_t type alias for its return type. /// Resolve that alias and return the true type as string. -static std::string RetTypeOfFunc(const std::string &funcName) +std::string RetTypeOfFunc(const std::string &funcName) { const auto dt = gROOT->GetType((funcName + "_ret_t").c_str()); R__ASSERT(dt != nullptr); @@ -913,7 +913,7 @@ ColumnNames_t GetValidatedColumnNames(RLoopManager &lm, const unsigned int nColu for (auto &unknownColumn : unknownColumns) errMsg += '"' + unknownColumn + "\", "; errMsg.resize(errMsg.size() - 2); // remove last ", " - throw std::runtime_error(std::move(errMsg)); + throw std::runtime_error(errMsg); } return selectedColumns; diff --git a/tree/dataframe/src/RDFUtils.cxx b/tree/dataframe/src/RDFUtils.cxx index 6b26ef5cbeb4f..d92e7f6e94fa1 100644 --- a/tree/dataframe/src/RDFUtils.cxx +++ b/tree/dataframe/src/RDFUtils.cxx @@ -53,7 +53,7 @@ const std::type_info &TypeName2TypeID(const std::string &name) if (auto c = TClass::GetClass(name.c_str())) { if (!c->GetTypeInfo()) { std::string msg("Cannot extract type_info of type "); - msg += name.c_str(); + msg += name; msg += "."; throw std::runtime_error(msg); } @@ -86,7 +86,7 @@ const std::type_info &TypeName2TypeID(const std::string &name) return typeid(bool); else { std::string msg("Cannot extract type_info of type "); - msg += name.c_str(); + msg += name; msg += "."; throw std::runtime_error(msg); } diff --git a/tree/dataframe/src/RDataFrame.cxx b/tree/dataframe/src/RDataFrame.cxx index e452eb0e9d669..709fea7f10c7e 100644 --- a/tree/dataframe/src/RDataFrame.cxx +++ b/tree/dataframe/src/RDataFrame.cxx @@ -1693,7 +1693,7 @@ namespace Experimental { ROOT::RDataFrame FromSpec(const std::string &jsonFile) { const nlohmann::ordered_json fullData = nlohmann::ordered_json::parse(std::ifstream(jsonFile)); - if (!fullData.contains("samples") || fullData["samples"].size() == 0) { + if (!fullData.contains("samples") || fullData["samples"].empty()) { throw std::runtime_error( R"(The input specification does not contain any samples. Please provide the samples in the specification like: { diff --git a/tree/dataframe/src/RLoopManager.cxx b/tree/dataframe/src/RLoopManager.cxx index 76563581b84be..88bb58341d038 100644 --- a/tree/dataframe/src/RLoopManager.cxx +++ b/tree/dataframe/src/RLoopManager.cxx @@ -59,13 +59,13 @@ namespace { /// We want RLoopManagers to be able to add their code to a global "code to execute via cling", /// so that, lazily, we can jit everything that's needed by all RDFs in one go, which is potentially /// much faster than jitting each RLoopManager's code separately. -static std::string &GetCodeToJit() +std::string &GetCodeToJit() { static std::string code; return code; } -static bool ContainsLeaf(const std::set &leaves, TLeaf *leaf) +bool ContainsLeaf(const std::set &leaves, TLeaf *leaf) { return (leaves.find(leaf) != leaves.end()); } @@ -73,7 +73,7 @@ static bool ContainsLeaf(const std::set &leaves, TLeaf *leaf) /////////////////////////////////////////////////////////////////////////////// /// This overload does not check whether the leaf/branch is already in bNamesReg. In case this is a friend leaf/branch, /// `allowDuplicates` controls whether we add both `friendname.bname` and `bname` or just the shorter version. -static void InsertBranchName(std::set &bNamesReg, ColumnNames_t &bNames, const std::string &branchName, +void InsertBranchName(std::set &bNamesReg, ColumnNames_t &bNames, const std::string &branchName, const std::string &friendName, bool allowDuplicates) { if (!friendName.empty()) { @@ -91,7 +91,7 @@ static void InsertBranchName(std::set &bNamesReg, ColumnNames_t &bN /////////////////////////////////////////////////////////////////////////////// /// This overload makes sure that the TLeaf has not been already inserted. -static void InsertBranchName(std::set &bNamesReg, ColumnNames_t &bNames, const std::string &branchName, +void InsertBranchName(std::set &bNamesReg, ColumnNames_t &bNames, const std::string &branchName, const std::string &friendName, std::set &foundLeaves, TLeaf *leaf, bool allowDuplicates) { @@ -105,7 +105,7 @@ static void InsertBranchName(std::set &bNamesReg, ColumnNames_t &bN foundLeaves.insert(leaf); } -static void ExploreBranch(TTree &t, std::set &bNamesReg, ColumnNames_t &bNames, TBranch *b, +void ExploreBranch(TTree &t, std::set &bNamesReg, ColumnNames_t &bNames, TBranch *b, std::string prefix, std::string &friendName, bool allowDuplicates) { for (auto sb : *b->GetListOfBranches()) { @@ -131,7 +131,7 @@ static void ExploreBranch(TTree &t, std::set &bNamesReg, ColumnName } } -static void GetBranchNamesImpl(TTree &t, std::set &bNamesReg, ColumnNames_t &bNames, +void GetBranchNamesImpl(TTree &t, std::set &bNamesReg, ColumnNames_t &bNames, std::set &analysedTrees, std::string &friendName, bool allowDuplicates) { std::set foundLeaves; @@ -214,7 +214,7 @@ static void GetBranchNamesImpl(TTree &t, std::set &bNamesReg, Colum } } -static void ThrowIfNSlotsChanged(unsigned int nSlots) +void ThrowIfNSlotsChanged(unsigned int nSlots) { const auto currentSlots = RDFInternal::GetNSlots(); if (currentSlots != nSlots) { @@ -299,7 +299,7 @@ DatasetLogInfo TreeDatasetLogInfo(const TTreeReader &r, unsigned int slot) return {std::move(what), static_cast(entryRange.first), end, slot}; } -static auto MakeDatasetColReadersKey(const std::string &colName, const std::type_info &ti) +auto MakeDatasetColReadersKey(const std::string &colName, const std::type_info &ti) { // We use a combination of column name and column type name as the key because in some cases we might end up // with concrete readers that use different types for the same column, e.g. std::vector and RVec here: diff --git a/tree/dataframe/src/RSqliteDS.cxx b/tree/dataframe/src/RSqliteDS.cxx index 25b564ed13ebe..2c84b63f4916e 100644 --- a/tree/dataframe/src/RSqliteDS.cxx +++ b/tree/dataframe/src/RSqliteDS.cxx @@ -153,7 +153,7 @@ int VfsRdOnlyDeviceCharacteristics(sqlite3_file * /*pFile*/) //////////////////////////////////////////////////////////////////////////// /// Set the function pointers of the custom VFS I/O operations in a /// forward-compatible way -static sqlite3_io_methods GetSqlite3IoMethods() +sqlite3_io_methods GetSqlite3IoMethods() { // The C style initialization is compatible with version 1 and later versions of the struct. // Version 1 was introduced with sqlite 3.6, version 2 with sqlite 3.7.8, version 3 with sqlite 3.7.17 @@ -286,7 +286,7 @@ int VfsRdOnlyCurrentTime(sqlite3_vfs *vfs, double *prNow) //////////////////////////////////////////////////////////////////////////// /// Set the function pointers of the VFS implementation in a /// forward-compatible way -static sqlite3_vfs GetSqlite3Vfs() +sqlite3_vfs GetSqlite3Vfs() { // The C style initialization is compatible with version 1 and later versions of the struct. // Version 1 was introduced with sqlite 3.5, version 2 with sqlite 3.7, version 3 with sqlite 3.7.6 @@ -309,9 +309,9 @@ static sqlite3_vfs GetSqlite3Vfs() //////////////////////////////////////////////////////////////////////////// /// A global struct of function pointers and details on the VfsRootFile class that together constitue a VFS module -static struct sqlite3_vfs kSqlite3Vfs = GetSqlite3Vfs(); +struct sqlite3_vfs kSqlite3Vfs = GetSqlite3Vfs(); -static bool RegisterSqliteVfs() +bool RegisterSqliteVfs() { int retval; retval = sqlite3_vfs_register(&kSqlite3Vfs, false); diff --git a/tree/dataframe/src/RVariationsDescription.cxx b/tree/dataframe/src/RVariationsDescription.cxx index cc7e53e90a5cc..23fac797e90f1 100644 --- a/tree/dataframe/src/RVariationsDescription.cxx +++ b/tree/dataframe/src/RVariationsDescription.cxx @@ -14,7 +14,7 @@ #include namespace { -static std::string GetStringRepr(const std::vector &variations) +std::string GetStringRepr(const std::vector &variations) { std::string s; diff --git a/tree/dataframe/test/dataframe_datasetspec.cxx b/tree/dataframe/test/dataframe_datasetspec.cxx index 0cd0be2a9215c..151758c6f94a8 100644 --- a/tree/dataframe/test/dataframe_datasetspec.cxx +++ b/tree/dataframe/test/dataframe_datasetspec.cxx @@ -93,7 +93,7 @@ class RDatasetSpecTest : public ::testing::TestWithParam { ROOT::EnableImplicitMT(NSLOTS); } - ~RDatasetSpecTest() + ~RDatasetSpecTest() override { if (GetParam()) ROOT::DisableImplicitMT(); diff --git a/tree/dataframe/test/dataframe_simple.cxx b/tree/dataframe/test/dataframe_simple.cxx index 3f71a766823c3..e2737a297b487 100644 --- a/tree/dataframe/test/dataframe_simple.cxx +++ b/tree/dataframe/test/dataframe_simple.cxx @@ -41,7 +41,7 @@ class RDFSimpleTests : public ::testing::TestWithParam { if (GetParam()) ROOT::EnableImplicitMT(NSLOTS); } - ~RDFSimpleTests() + ~RDFSimpleTests() override { if (GetParam()) ROOT::DisableImplicitMT(); @@ -637,6 +637,7 @@ class StdDevTestHelper { void GenerateNumbers(int n) { std::vector numbers; + numbers.reserve(n); for (int i = 0; i < n; ++i) numbers.push_back(fDistribution(fGenerator)); samples = numbers; diff --git a/tree/dataframe/test/dataframe_snapshot.cxx b/tree/dataframe/test/dataframe_snapshot.cxx index 35ea93040d4cb..2d358dff84dda 100644 --- a/tree/dataframe/test/dataframe_snapshot.cxx +++ b/tree/dataframe/test/dataframe_snapshot.cxx @@ -1049,7 +1049,7 @@ TEST(RDFSnapshotMore, ManyTasksPerThread) // test multi-thread Snapshotting from many tasks per worker thread const auto outputFile = "snapshot_manytasks_out.root"; - ROOT::RDataFrame tdf("t", (inputFilePrefix + "*.root").c_str()); + ROOT::RDataFrame tdf("t", inputFilePrefix + "*.root"); tdf.Snapshot("t", outputFile, {"x"}); // check output contents diff --git a/tree/dataframe/test/dataframe_vary.cxx b/tree/dataframe/test/dataframe_vary.cxx index 53fc803db8848..e31f89f60ffbe 100644 --- a/tree/dataframe/test/dataframe_vary.cxx +++ b/tree/dataframe/test/dataframe_vary.cxx @@ -26,7 +26,7 @@ class RDFVary : public ::testing::TestWithParam { if (GetParam()) ROOT::EnableImplicitMT(NSLOTS); } - ~RDFVary() + ~RDFVary() override { if (GetParam()) ROOT::DisableImplicitMT(); @@ -1629,7 +1629,7 @@ struct HelperWithCallback : ROOT::Detail::RDF::RActionImpl { void Initialize() {} void Finalize() { *fResult = fCallbackCallCount; } std::shared_ptr GetResultPtr() const { return fResult; } - ROOT::RDF::SampleCallback_t GetSampleCallback() // increments fCallbackCallCount + ROOT::RDF::SampleCallback_t GetSampleCallback() override // increments fCallbackCallCount { auto callback = [](unsigned int, const ROOT::RDF::RSampleInfo &) { ++fCallbackCallCount; }; return callback; diff --git a/tree/ntuple/v7/inc/ROOT/RColumnElement.hxx b/tree/ntuple/v7/inc/ROOT/RColumnElement.hxx index 81554f7102987..bf820b68b7e75 100644 --- a/tree/ntuple/v7/inc/ROOT/RColumnElement.hxx +++ b/tree/ntuple/v7/inc/ROOT/RColumnElement.hxx @@ -70,7 +70,7 @@ namespace { /// Used on big-endian architectures for packing/unpacking elements whose column type requires /// a little-endian on-disk representation. template -static void CopyBswap(void *destination, const void *source, std::size_t count) +void CopyBswap(void *destination, const void *source, std::size_t count) { auto dst = reinterpret_cast::value_type *>(destination); auto src = reinterpret_cast::value_type *>(source); diff --git a/tree/ntuple/v7/inc/ROOT/RField.hxx b/tree/ntuple/v7/inc/ROOT/RField.hxx index ef4405fe68cd7..9f08dd8cf96e9 100644 --- a/tree/ntuple/v7/inc/ROOT/RField.hxx +++ b/tree/ntuple/v7/inc/ROOT/RField.hxx @@ -1469,7 +1469,7 @@ protected: public: RCardinalityField(RCardinalityField &&other) = default; RCardinalityField &operator=(RCardinalityField &&other) = default; - ~RCardinalityField() = default; + ~RCardinalityField() override = default; void AcceptVisitor(Detail::RFieldVisitor &visitor) const final; @@ -1545,7 +1545,7 @@ public: explicit RField(std::string_view name) : RCardinalityField(name, TypeName()) {} RField(RField &&other) = default; RField &operator=(RField &&other) = default; - ~RField() = default; + ~RField() override = default; using Detail::RFieldBase::GenerateValue; size_t GetValueSize() const final { return sizeof(RNTupleCardinality); } diff --git a/tree/ntuple/v7/inc/ROOT/RNTuple.hxx b/tree/ntuple/v7/inc/ROOT/RNTuple.hxx index d95eed9bf75a6..d7c0fd1fc9dcd 100644 --- a/tree/ntuple/v7/inc/ROOT/RNTuple.hxx +++ b/tree/ntuple/v7/inc/ROOT/RNTuple.hxx @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -452,7 +453,7 @@ public: /// ~~~ std::unique_ptr CreateModelUpdater() { - return std::unique_ptr(new RNTupleModel::RUpdater(*this)); + return std::make_unique(*this); } }; diff --git a/tree/ntuple/v7/inc/ROOT/RNTupleZip.hxx b/tree/ntuple/v7/inc/ROOT/RNTupleZip.hxx index 4908513683058..174072b8a3635 100644 --- a/tree/ntuple/v7/inc/ROOT/RNTupleZip.hxx +++ b/tree/ntuple/v7/inc/ROOT/RNTupleZip.hxx @@ -52,7 +52,7 @@ public: } static constexpr size_t kMaxSingleBlock = kMAXZIPBUF; - RNTupleCompressor() : fZipBuffer(std::unique_ptr(new Buffer_t())) {} + RNTupleCompressor() : fZipBuffer(std::make_unique()) {} RNTupleCompressor(const RNTupleCompressor &other) = delete; RNTupleCompressor &operator =(const RNTupleCompressor &other) = delete; RNTupleCompressor(RNTupleCompressor &&other) = default; @@ -179,7 +179,7 @@ private: std::unique_ptr fUnzipBuffer; public: - RNTupleDecompressor() : fUnzipBuffer(std::unique_ptr(new Buffer_t())) {} + RNTupleDecompressor() : fUnzipBuffer(std::make_unique()) {} RNTupleDecompressor(const RNTupleDecompressor &other) = delete; RNTupleDecompressor &operator =(const RNTupleDecompressor &other) = delete; RNTupleDecompressor(RNTupleDecompressor &&other) = default; diff --git a/tree/ntuple/v7/src/RClusterPool.cxx b/tree/ntuple/v7/src/RClusterPool.cxx index 1491f9d1e9cfb..804217c6901b9 100644 --- a/tree/ntuple/v7/src/RClusterPool.cxx +++ b/tree/ntuple/v7/src/RClusterPool.cxx @@ -369,7 +369,7 @@ ROOT::Experimental::Detail::RClusterPool::GetCluster(DescriptorId_t clusterId, fReadQueue.emplace_back(std::move(readItem)); } - if (fReadQueue.size() > 0) + if (!fReadQueue.empty()) fCvHasReadWork.notify_one(); } } // work queue lock guard diff --git a/tree/ntuple/v7/src/RField.cxx b/tree/ntuple/v7/src/RField.cxx index 981c2c0071978..2f950a5f44546 100644 --- a/tree/ntuple/v7/src/RField.cxx +++ b/tree/ntuple/v7/src/RField.cxx @@ -46,13 +46,14 @@ #include // for memset #include #include +#include #include // hardware_destructive_interference_size #include #include namespace { -static const std::unordered_map typeTranslationMap{ +const std::unordered_map typeTranslationMap{ {"Bool_t", "bool"}, {"Float_t", "float"}, {"Double_t", "double"}, @@ -133,7 +134,7 @@ std::tuple> ParseArrayType(std::string_view typ // Only parse outer array definition, i.e. the right `]` should be at the end of the type name while (typeName.back() == ']') { auto posRBrace = typeName.size() - 1; - auto posLBrace = typeName.find_last_of("[", posRBrace); + auto posLBrace = typeName.find_last_of('[', posRBrace); if (posLBrace == std::string_view::npos) return {}; @@ -505,9 +506,9 @@ ROOT::Experimental::Detail::RFieldBase::Create(const std::string &fieldName, con ROOT::Experimental::RResult ROOT::Experimental::Detail::RFieldBase::EnsureValidFieldName(std::string_view fieldName) { - if (fieldName == "") { + if (fieldName.empty()) { return R__FAIL("name cannot be empty string \"\""); - } else if (fieldName.find(".") != std::string::npos) { + } else if (fieldName.find('.') != std::string::npos) { return R__FAIL("name '" + std::string(fieldName) + "' cannot contain dot characters '.'"); } return RResult::Success(); @@ -610,6 +611,7 @@ ROOT::Experimental::Detail::RFieldBase::EntryToColumnElementIndex(ROOT::Experime std::vector ROOT::Experimental::Detail::RFieldBase::GetSubFields() const { std::vector result; + result.reserve(fSubFields.size()); for (const auto &f : fSubFields) { result.emplace_back(f.get()); } @@ -1767,6 +1769,7 @@ std::unique_ptr ROOT::Experimental::RRecordField::CloneImpl(std::string_view newName) const { std::vector> cloneItems; + cloneItems.reserve(fSubFields.size()); for (auto &item : fSubFields) cloneItems.emplace_back(item->Clone(item->GetName())); return std::unique_ptr(new RRecordField(newName, std::move(cloneItems), fOffsets, GetType())); @@ -2196,6 +2199,7 @@ ROOT::Experimental::RRVecField::SplitValue(const RValue &value) const std::vector result; char *begin = reinterpret_cast(*beginPtr); // for pointer arithmetics + result.reserve(*sizePtr); for (std::int32_t i = 0; i < *sizePtr; ++i) { result.emplace_back(fSubFields[0]->BindValue(begin + i * fItemSize)); } @@ -2643,7 +2647,7 @@ std::unique_ptr ROOT::Experimental::RSetField::CloneImpl(std::string_view newName) const { auto newItemField = fSubFields[0]->Clone(fSubFields[0]->GetName()); - return std::unique_ptr(new RSetField(newName, GetType(), std::move(newItemField))); + return std::make_unique(newName, GetType(), std::move(newItemField)); } //------------------------------------------------------------------------------ @@ -2913,6 +2917,7 @@ std::unique_ptr ROOT::Experimental::RTupleField::CloneImpl(std::string_view newName) const { std::vector> items; + items.reserve(fSubFields.size()); for (const auto &item : fSubFields) items.push_back(item->Clone(item->GetName())); diff --git a/tree/ntuple/v7/src/RMiniFile.cxx b/tree/ntuple/v7/src/RMiniFile.cxx index 0d3becca0b514..ab3fef6ab3328 100644 --- a/tree/ntuple/v7/src/RMiniFile.cxx +++ b/tree/ntuple/v7/src/RMiniFile.cxx @@ -934,9 +934,9 @@ struct RBareFileHeader { #pragma pack(pop) /// The artifical class name shown for opaque RNTuple keys (see TBasket) -static constexpr char const *kBlobClassName = "RBlob"; +constexpr char const *kBlobClassName = "RBlob"; /// The class name of the RNTuple anchor -static constexpr char const *kNTupleClassName = "ROOT::Experimental::RNTuple"; +constexpr char const *kNTupleClassName = "ROOT::Experimental::RNTuple"; /// 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. diff --git a/tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx b/tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx index a086037d6b40f..4f64ef26ad9ec 100644 --- a/tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx +++ b/tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx @@ -62,7 +62,7 @@ struct ColumnInfo { } }; -static std::string GetFieldName(ROOT::Experimental::DescriptorId_t fieldId, +std::string GetFieldName(ROOT::Experimental::DescriptorId_t fieldId, const ROOT::Experimental::RNTupleDescriptor &ntupleDesc) { const auto &fieldDesc = ntupleDesc.GetFieldDescriptor(fieldId); @@ -71,7 +71,7 @@ static std::string GetFieldName(ROOT::Experimental::DescriptorId_t fieldId, return GetFieldName(fieldDesc.GetParentId(), ntupleDesc) + "." + fieldDesc.GetFieldName(); } -static std::string GetFieldDescription(ROOT::Experimental::DescriptorId_t fFieldId, +std::string GetFieldDescription(ROOT::Experimental::DescriptorId_t fFieldId, const ROOT::Experimental::RNTupleDescriptor &ntupleDesc) { const auto &fieldDesc = ntupleDesc.GetFieldDescriptor(fFieldId); diff --git a/tree/ntuple/v7/src/RPageSinkBuf.cxx b/tree/ntuple/v7/src/RPageSinkBuf.cxx index a39cfce1f1d6c..5d21a36cc9ad9 100644 --- a/tree/ntuple/v7/src/RPageSinkBuf.cxx +++ b/tree/ntuple/v7/src/RPageSinkBuf.cxx @@ -21,6 +21,7 @@ #include #include +#include void ROOT::Experimental::Detail::RPageSinkBuf::RColumnBuf::DropBufferedPages() { @@ -38,7 +39,7 @@ ROOT::Experimental::Detail::RPageSinkBuf::RPageSinkBuf(std::unique_ptr(new RCounters{ + fCounters = std::make_unique(RCounters{ *fMetrics.MakeCounter("ParallelZip", "", "compressing pages in parallel") }); diff --git a/tree/ntuple/v7/src/RPageSourceFriends.cxx b/tree/ntuple/v7/src/RPageSourceFriends.cxx index 0758dd0b1b6df..b1c23cd897b46 100644 --- a/tree/ntuple/v7/src/RPageSourceFriends.cxx +++ b/tree/ntuple/v7/src/RPageSourceFriends.cxx @@ -119,6 +119,7 @@ std::unique_ptr ROOT::Experimental::Detail::RPageSourceFriends::Clone() const { std::vector> cloneSources; + cloneSources.reserve(fSources.size()); for (const auto &f : fSources) cloneSources.emplace_back(f->Clone()); return std::make_unique(fNTupleName, cloneSources); diff --git a/tree/ntuple/v7/src/RPageStorage.cxx b/tree/ntuple/v7/src/RPageStorage.cxx index 56e61cb5fb4ab..e1debb9a40f81 100644 --- a/tree/ntuple/v7/src/RPageStorage.cxx +++ b/tree/ntuple/v7/src/RPageStorage.cxx @@ -23,6 +23,7 @@ #include #include #include +#include #include #ifdef R__ENABLE_DAOS # include @@ -202,7 +203,7 @@ void ROOT::Experimental::Detail::RPageSource::PrepareLoadCluster( void ROOT::Experimental::Detail::RPageSource::EnableDefaultMetrics(const std::string &prefix) { fMetrics = RNTupleMetrics(prefix); - fCounters = std::unique_ptr(new RCounters{ + fCounters = std::make_unique(RCounters{ *fMetrics.MakeCounter("nReadV", "", "number of vector read requests"), *fMetrics.MakeCounter("nRead", "", "number of byte ranges read"), *fMetrics.MakeCounter("szReadPayload", "B", "volume read from storage (required)"), @@ -572,7 +573,7 @@ ROOT::Experimental::Detail::RPageSink::SealPage( void ROOT::Experimental::Detail::RPageSink::EnableDefaultMetrics(const std::string &prefix) { fMetrics = RNTupleMetrics(prefix); - fCounters = std::unique_ptr(new RCounters{ + fCounters = std::make_unique(RCounters{ *fMetrics.MakeCounter("nPageCommitted", "", "number of pages committed to storage"), *fMetrics.MakeCounter("szWritePayload", "B", "volume written for committed pages"), *fMetrics.MakeCounter("szZip", "B", "volume before zipping"), diff --git a/tree/ntuple/v7/test/ntuple_basics.cxx b/tree/ntuple/v7/test/ntuple_basics.cxx index af4b36f4dae8f..6bab9b65e1bdd 100644 --- a/tree/ntuple/v7/test/ntuple_basics.cxx +++ b/tree/ntuple/v7/test/ntuple_basics.cxx @@ -654,7 +654,7 @@ struct RFieldCallbackInjector { }; } // namespace ROOT::Experimental::Internal namespace { -static unsigned gNCallReadCallback = 0; +unsigned gNCallReadCallback = 0; } TEST(RNTuple, ReadCallback) diff --git a/tree/ntuple/v7/test/ntuple_modelext.cxx b/tree/ntuple/v7/test/ntuple_modelext.cxx index f0f9c7e898356..431dcba1e5b32 100644 --- a/tree/ntuple/v7/test/ntuple_modelext.cxx +++ b/tree/ntuple/v7/test/ntuple_modelext.cxx @@ -28,7 +28,7 @@ struct RFieldBaseTest : public ROOT::Experimental::Detail::RFieldBase { } }; -static std::size_t GetFirstEntry(const RFieldBase *f) +std::size_t GetFirstEntry(const RFieldBase *f) { return static_cast(f)->GetFirstEntry(); } diff --git a/tree/ntupleutil/v7/inc/ROOT/RNTupleImporter.hxx b/tree/ntupleutil/v7/inc/ROOT/RNTupleImporter.hxx index fd13fa3be2769..3ae2a61331bca 100644 --- a/tree/ntupleutil/v7/inc/ROOT/RNTupleImporter.hxx +++ b/tree/ntupleutil/v7/inc/ROOT/RNTupleImporter.hxx @@ -190,7 +190,7 @@ private: /// Transform a NULL terminated C string branch into an std::string field struct RCStringTransformation : public RImportTransformation { RCStringTransformation(std::size_t b, std::size_t f) : RImportTransformation(b, f) {} - virtual ~RCStringTransformation() = default; + ~RCStringTransformation() override = default; RResult Transform(const RImportBranch &branch, RImportField &field) final; void ResetEntry() final {} }; @@ -201,7 +201,7 @@ private: struct RLeafArrayTransformation : public RImportTransformation { std::int64_t fNum = 0; RLeafArrayTransformation(std::size_t b, std::size_t f) : RImportTransformation(b, f) {} - virtual ~RLeafArrayTransformation() = default; + ~RLeafArrayTransformation() override = default; RResult Transform(const RImportBranch &branch, RImportField &field) final; void ResetEntry() final { fNum = 0; } }; diff --git a/tree/ntupleutil/v7/src/RNTupleImporter.cxx b/tree/ntupleutil/v7/src/RNTupleImporter.cxx index 645a039e710e7..bb8728e13ac56 100644 --- a/tree/ntupleutil/v7/src/RNTupleImporter.cxx +++ b/tree/ntupleutil/v7/src/RNTupleImporter.cxx @@ -46,7 +46,7 @@ class RDefaultProgressCallback : public ROOT::Experimental::RNTupleImporter::RPr std::uint64_t fNbytesNext = gUpdateFrequencyBytes; public: - virtual ~RDefaultProgressCallback() {} + ~RDefaultProgressCallback() override {} void Call(std::uint64_t nbytesWritten, std::uint64_t neventsWritten) final { // Report if more than 50MB (compressed) where written since the last status update diff --git a/tree/ntupleutil/v7/src/RNTupleInspector.cxx b/tree/ntupleutil/v7/src/RNTupleInspector.cxx index da1c8f15f65e8..01c12e2506b52 100644 --- a/tree/ntupleutil/v7/src/RNTupleInspector.cxx +++ b/tree/ntupleutil/v7/src/RNTupleInspector.cxx @@ -275,7 +275,7 @@ std::unique_ptr ROOT::Experimental::RNTupleInspector::GetColumnTypeInfoAsHist(ROOT::Experimental::ENTupleInspectorHist histKind, std::string_view histName, std::string_view histTitle) { - if (histName == "") { + if (histName.empty()) { switch (histKind) { case ENTupleInspectorHist::kCount: histName = "colTypeCountHist"; break; case ENTupleInspectorHist::kNElems: histName = "colTypeElemCountHist"; break; @@ -285,7 +285,7 @@ ROOT::Experimental::RNTupleInspector::GetColumnTypeInfoAsHist(ROOT::Experimental } } - if (histTitle == "") { + if (histTitle.empty()) { switch (histKind) { case ENTupleInspectorHist::kCount: histTitle = "Column count by type"; break; case ENTupleInspectorHist::kNElems: histTitle = "Number of elements by column type"; break; diff --git a/tree/ntupleutil/v7/test/ntuple_inspector.cxx b/tree/ntupleutil/v7/test/ntuple_inspector.cxx index b12383668b061..2d5440a9bc3d5 100644 --- a/tree/ntupleutil/v7/test/ntuple_inspector.cxx +++ b/tree/ntupleutil/v7/test/ntuple_inspector.cxx @@ -357,7 +357,7 @@ TEST(RNTupleInspector, PrintColumnTypeInfo) std::string colTypeStr; while (std::getline(csvOutput, line)) { ++nLines; - colTypeStr = line.substr(0, line.find(",")); + colTypeStr = line.substr(0, line.find(',')); if (colTypeStr != "SplitIndex64" && colTypeStr != "SplitInt64" && colTypeStr != "SplitReal32") FAIL() << "Unexpected column type: " << colTypeStr; @@ -376,7 +376,7 @@ TEST(RNTupleInspector, PrintColumnTypeInfo) nLines = 0; while (std::getline(tableOutput, line)) { ++nLines; - colTypeStr = line.substr(0, line.find("|")); + colTypeStr = line.substr(0, line.find('|')); colTypeStr.erase(remove_if(colTypeStr.begin(), colTypeStr.end(), isspace), colTypeStr.end()); if (colTypeStr != "SplitIndex64" && colTypeStr != "SplitInt64" && colTypeStr != "SplitReal32")