Skip to content

Commit

Permalink
Apply suggestions from clang-tidy to dataframe and rntuple
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
guitargeek committed Nov 10, 2023
1 parent 8e460c3 commit c5048fa
Show file tree
Hide file tree
Showing 34 changed files with 97 additions and 87 deletions.
2 changes: 1 addition & 1 deletion tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ void JitVariationHelper(F &&f, const char **colsPtr, std::size_t colsSize, const
// use unique_ptr<RDefineBase> instead of make_unique<NewCol_t> to reduce jit/compile-times
std::unique_ptr<RVariationBase> newVariation{new RVariation<std::decay_t<F>, IsSingleColumn>(
std::move(variedColNames), variationName, std::forward<F>(f), std::move(tags), jittedVariation->GetTypeName(),
*colRegister, *lm, std::move(inputColNames))};
*colRegister, *lm, inputColNames)};
jittedVariation->SetVariation(std::move(newVariation));

doDeletes();
Expand Down
4 changes: 2 additions & 2 deletions tree/dataframe/inc/ROOT/RDF/RInterface.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -2925,8 +2925,8 @@ private:
const std::vector<std::string> &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) {
Expand Down
4 changes: 2 additions & 2 deletions tree/dataframe/inc/ROOT/RDF/RInterfaceBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ protected:
void SanityChecksForVary(const std::vector<std::string> &colNames, const std::vector<std::string> &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) {
Expand Down
2 changes: 1 addition & 1 deletion tree/dataframe/inc/ROOT/RDataFrame.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public:
const ColumnNames_t &defaultColumns = {});
RDataFrame(std::string_view treename, std::initializer_list<std::string> filenames,
const ColumnNames_t &defaultColumns = {}):
RDataFrame(treename, std::vector<std::string>(std::move(filenames)), defaultColumns) {}
RDataFrame(treename, std::vector<std::string>(filenames), defaultColumns) {}
RDataFrame(std::string_view treeName, ::TDirectory *dirPtr, const ColumnNames_t &defaultColumns = {});
RDataFrame(TTree &tree, const ColumnNames_t &defaultColumns = {});
RDataFrame(ULong64_t numEntries);
Expand Down
36 changes: 18 additions & 18 deletions tree/dataframe/src/RArrowDS.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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<void *>(&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<void *>(&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: {
Expand Down Expand Up @@ -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;
};
Expand Down
4 changes: 2 additions & 2 deletions tree/dataframe/src/RDFHelpers.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include <algorithm>
#include <iostream>
#include <set>
#include <stdio.h>
#include <cstdio>

// TODO, this function should be part of core libraries
#include <numeric>
Expand Down Expand Up @@ -355,7 +355,7 @@ class ProgressBarAction final : public ROOT::Detail::RDF::RActionImpl<ProgressBa
// dummy implementation of PartialUpdate
int &PartialUpdate(unsigned int) { return *fDummyResult; }

ROOT::RDF::SampleCallback_t GetSampleCallback()
ROOT::RDF::SampleCallback_t GetSampleCallback() final
{
return [this](unsigned int slot, const ROOT::RDF::RSampleInfo &id) {
this->fHelper->registerNewSample(slot, id);
Expand Down
4 changes: 2 additions & 2 deletions tree/dataframe/src/RDFHistoModels.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <ROOT/TSeq.hxx>
#include <TProfile.h>
#include <TProfile2D.h>
#include <stddef.h>
#include <cstddef>
#include <vector>

#include "TAxis.h"
Expand Down Expand Up @@ -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;
}
Expand Down
20 changes: 10 additions & 10 deletions tree/dataframe/src/RDFInterfaceUtils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ struct ParsedExpression {
};

/// Look at expression `expr` and return a pair of (column names used, aliases used)
static std::pair<ColumnNames_t, ColumnNames_t>
std::pair<ColumnNames_t, ColumnNames_t>
FindUsedColsAndAliases(const std::string &expr, const ColumnNames_t &treeBranchNames,
const ROOT::Internal::RDF::RColumnRegister &colRegister, const ColumnNames_t &dataSourceColNames)
{
Expand Down Expand Up @@ -155,15 +155,15 @@ 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("\\.");
dot.Substitute(out, "\\.", "g");
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);
Expand All @@ -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)
{
Expand Down Expand Up @@ -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<std::string, std::string> &GetJittedExprs() {
std::unordered_map<std::string, std::string> &GetJittedExprs() {
static std::unordered_map<std::string, std::string> 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());
Expand Down Expand Up @@ -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);

Expand All @@ -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<decltype(" + funcBaseName +
")>::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});
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions tree/dataframe/src/RDFUtils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion tree/dataframe/src/RDataFrame.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
{
Expand Down
16 changes: 8 additions & 8 deletions tree/dataframe/src/RLoopManager.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,21 @@ 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<TLeaf *> &leaves, TLeaf *leaf)
bool ContainsLeaf(const std::set<TLeaf *> &leaves, TLeaf *leaf)
{
return (leaves.find(leaf) != leaves.end());
}

///////////////////////////////////////////////////////////////////////////////
/// 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<std::string> &bNamesReg, ColumnNames_t &bNames, const std::string &branchName,
void InsertBranchName(std::set<std::string> &bNamesReg, ColumnNames_t &bNames, const std::string &branchName,
const std::string &friendName, bool allowDuplicates)
{
if (!friendName.empty()) {
Expand All @@ -91,7 +91,7 @@ static void InsertBranchName(std::set<std::string> &bNamesReg, ColumnNames_t &bN

///////////////////////////////////////////////////////////////////////////////
/// This overload makes sure that the TLeaf has not been already inserted.
static void InsertBranchName(std::set<std::string> &bNamesReg, ColumnNames_t &bNames, const std::string &branchName,
void InsertBranchName(std::set<std::string> &bNamesReg, ColumnNames_t &bNames, const std::string &branchName,
const std::string &friendName, std::set<TLeaf *> &foundLeaves, TLeaf *leaf,
bool allowDuplicates)
{
Expand All @@ -105,7 +105,7 @@ static void InsertBranchName(std::set<std::string> &bNamesReg, ColumnNames_t &bN
foundLeaves.insert(leaf);
}

static void ExploreBranch(TTree &t, std::set<std::string> &bNamesReg, ColumnNames_t &bNames, TBranch *b,
void ExploreBranch(TTree &t, std::set<std::string> &bNamesReg, ColumnNames_t &bNames, TBranch *b,
std::string prefix, std::string &friendName, bool allowDuplicates)
{
for (auto sb : *b->GetListOfBranches()) {
Expand All @@ -131,7 +131,7 @@ static void ExploreBranch(TTree &t, std::set<std::string> &bNamesReg, ColumnName
}
}

static void GetBranchNamesImpl(TTree &t, std::set<std::string> &bNamesReg, ColumnNames_t &bNames,
void GetBranchNamesImpl(TTree &t, std::set<std::string> &bNamesReg, ColumnNames_t &bNames,
std::set<TTree *> &analysedTrees, std::string &friendName, bool allowDuplicates)
{
std::set<TLeaf *> foundLeaves;
Expand Down Expand Up @@ -214,7 +214,7 @@ static void GetBranchNamesImpl(TTree &t, std::set<std::string> &bNamesReg, Colum
}
}

static void ThrowIfNSlotsChanged(unsigned int nSlots)
void ThrowIfNSlotsChanged(unsigned int nSlots)
{
const auto currentSlots = RDFInternal::GetNSlots();
if (currentSlots != nSlots) {
Expand Down Expand Up @@ -299,7 +299,7 @@ DatasetLogInfo TreeDatasetLogInfo(const TTreeReader &r, unsigned int slot)
return {std::move(what), static_cast<ULong64_t>(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:
Expand Down
8 changes: 4 additions & 4 deletions tree/dataframe/src/RSqliteDS.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tree/dataframe/src/RVariationsDescription.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <iostream>

namespace {
static std::string GetStringRepr(const std::vector<const ROOT::Internal::RDF::RVariationBase *> &variations)
std::string GetStringRepr(const std::vector<const ROOT::Internal::RDF::RVariationBase *> &variations)
{
std::string s;

Expand Down
Loading

0 comments on commit c5048fa

Please sign in to comment.