Skip to content

Commit

Permalink
[df] Implement rule of five for RNodeBase and derived classes
Browse files Browse the repository at this point in the history
  • Loading branch information
vepadulano committed Jan 9, 2025
1 parent b893352 commit 45d37b3
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 10 deletions.
7 changes: 6 additions & 1 deletion tree/dataframe/inc/ROOT/RDF/RFilter.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,14 @@ public:
fLoopManager->Register(this);
}

// Rule of five

RFilter(const RFilter &) = delete;
RFilter &operator=(const RFilter &) = delete;
~RFilter() {
RFilter(RFilter &&) = delete;
RFilter &operator=(RFilter &&) = delete;
~RFilter() final
{
// must Deregister objects from the RLoopManager here, before the fPrevNode data member is destroyed:
// otherwise if fPrevNode is the RLoopManager, it will be destroyed before the calls to Deregister happen.
fLoopManager->Deregister(this);
Expand Down
7 changes: 6 additions & 1 deletion tree/dataframe/inc/ROOT/RDF/RFilterBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,13 @@ public:
RFilterBase(RLoopManager *df, std::string_view name, const unsigned int nSlots,
const RDFInternal::RColumnRegister &colRegister, const ColumnNames_t &columns,
const std::vector<std::string> &prevVariations, const std::string &variation = "nominal");
RFilterBase &operator=(const RFilterBase &) = delete;

// Rule of five

RFilterBase(const RFilterBase &) = delete;
RFilterBase &operator=(const RFilterBase &) = delete;
RFilterBase(RFilterBase &&) = delete;
RFilterBase &operator=(RFilterBase &&) = delete;
~RFilterBase() override;

virtual void InitSlot(TTreeReader *r, unsigned int slot) = 0;
Expand Down
9 changes: 8 additions & 1 deletion tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,14 @@ class RJittedFilter final : public RFilterBase {

public:
RJittedFilter(RLoopManager *lm, std::string_view name, const std::vector<std::string> &variations);
~RJittedFilter();

// Rule of five

RJittedFilter(const RJittedFilter &) = delete;
RJittedFilter &operator=(const RJittedFilter &) = delete;
RJittedFilter(RJittedFilter &&) = delete;
RJittedFilter &operator=(RJittedFilter &&) = delete;
~RJittedFilter() final;

void SetFilter(std::unique_ptr<RFilterBase> f);

Expand Down
2 changes: 1 addition & 1 deletion tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public:
RLoopManager &operator=(const RLoopManager &) = delete;
RLoopManager(RLoopManager &&) = delete;
RLoopManager &operator=(RLoopManager &&) = delete;
~RLoopManager() = default;
~RLoopManager() override;

void JitDeclarations();
void Jit();
Expand Down
9 changes: 8 additions & 1 deletion tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ public:
: fLoopManager(lm), fVariations(variations)
{
}
virtual ~RNodeBase() {}

// Rule of five
RNodeBase(const RNodeBase &) = delete;
RNodeBase &operator=(const RNodeBase &) = delete;
RNodeBase(RNodeBase &&) = delete;
RNodeBase &operator=(RNodeBase &&) = delete;
virtual ~RNodeBase() = default;

virtual bool CheckFilters(unsigned int, Long64_t) = 0;
virtual void Report(ROOT::RDF::RCutFlowReport &) const = 0;
virtual void PartialReport(ROOT::RDF::RCutFlowReport &) const = 0;
Expand Down
6 changes: 5 additions & 1 deletion tree/dataframe/inc/ROOT/RDF/RRange.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@ public:
fLoopManager->Register(this);
}

// Rule of five

RRange(const RRange &) = delete;
RRange &operator=(const RRange &) = delete;
RRange(RRange &&) = delete;
RRange &operator=(RRange &&) = delete;
// must call Deregister here, before fPrevNode is destroyed,
// otherwise if fPrevNode is fLoopManager we get a use after delete
~RRange() { fLoopManager->Deregister(this); }
~RRange() final { fLoopManager->Deregister(this); }

/// Ranges act as filters when it comes to selecting entries that downstream nodes should process
bool CheckFilters(unsigned int slot, Long64_t entry) final
Expand Down
5 changes: 5 additions & 0 deletions tree/dataframe/inc/ROOT/RDF/RRangeBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ public:
RRangeBase(RLoopManager *implPtr, unsigned int start, unsigned int stop, unsigned int stride,
const unsigned int nSlots, const std::vector<std::string> &prevVariations);

// Rule of five

RRangeBase(const RRangeBase &) = delete;
RRangeBase &operator=(const RRangeBase &) = delete;
RRangeBase(RRangeBase &&) = delete;
RRangeBase &operator=(RRangeBase &&) = delete;
~RRangeBase() override;

void InitNode();
Expand Down
6 changes: 3 additions & 3 deletions tree/dataframe/src/RFilterBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ RFilterBase::RFilterBase(RLoopManager *implPtr, std::string_view name, const uns
}
}

// outlined to pin virtual table
RFilterBase::~RFilterBase() {}

bool RFilterBase::HasName() const
{
return !fName.empty();
Expand All @@ -61,3 +58,6 @@ void RFilterBase::InitNode()
if (!fName.empty()) // if this is a named filter we care about its report count
ResetReportCount();
}

// outlined to pin virtual table
ROOT::Detail::RDF::RFilterBase::~RFilterBase() = default;
3 changes: 3 additions & 0 deletions tree/dataframe/src/RLoopManager.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1305,3 +1305,6 @@ ROOT::Detail::RDF::CreateLMFromFile(std::string_view datasetName, const std::vec
"\" in file \"" + inFile->GetName() + "\".");
}
#endif

// outlined to pin virtual table
ROOT::Detail::RDF::RLoopManager::~RLoopManager() = default;
2 changes: 1 addition & 1 deletion tree/dataframe/src/RRangeBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ void RRangeBase::InitNode()
}

// outlined to pin virtual table
RRangeBase::~RRangeBase() { }
ROOT::Detail::RDF::RRangeBase::~RRangeBase() = default;

0 comments on commit 45d37b3

Please sign in to comment.