Skip to content

Commit 333f2e5

Browse files
committed
[Tree] Delete virtual copy and copy assignment methods in TTree
The `TTree` class already deletes the copy constructor and the copy assignment, but users might still be tempted to use the virtual copy constructor or copy assignment provided by `TObject::Clone()` and `TObject::Copy()`. But these implementations also make no sense to the TTree. To avoid user confusion, this commit suggests to override these methods in TTree such that they thow an exception and tell you what to use instead. Prevents reports this one in the future: * https://its.cern.ch/jira/browse/ROOT-9111
1 parent 10d7bd3 commit 333f2e5

File tree

5 files changed

+19
-4
lines changed

5 files changed

+19
-4
lines changed

bindings/pyroot/pythonizations/test/memory.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ def test_objects_ownership_with_subdir(self):
112112
"TGraph2D": (100,),
113113
"TEntryList": ("name", "title"),
114114
"TEventList": ("name", "title"),
115-
"TTree": ("name", "title"),
116115
"TNtuple": ("name", "title", "x:y:z"),
117116
}
118117
for klass, args in objs.items():

io/io/src/TFileMerger.cxx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ to be merged, like the standalone hadd program.
5151

5252
#include <cstring>
5353
#include <map>
54+
#include <iostream>
5455

5556
ClassImp(TFileMerger);
5657

@@ -530,7 +531,7 @@ Bool_t TFileMerger::MergeOne(TDirectory *target, TList *sourcelist, Int_t type,
530531
&& !cl->InheritsFrom( TDirectory::Class() )) {
531532
R__ASSERT(cl->IsTObject());
532533
TDirectory::TContext ctxt(current_sourcedir);
533-
obj = obj->Clone();
534+
obj = gDirectory->CloneObject(obj);
534535
ownobj = kTRUE;
535536
}
536537
} else if (key) {

roofit/roofitcore/src/RooTreeDataStore.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ void RooTreeDataStore::loadValues(const TTree *t, const RooFormulaVar* select, c
369369
// We need a custom deleter, because if we don't deregister the Tree from the directory
370370
// of the original, it tears it down at destruction time!
371371
auto deleter = [](TTree* tree){tree->SetDirectory(nullptr); delete tree;};
372-
std::unique_ptr<TTree, decltype(deleter)> tClone(static_cast<TTree*>(t->Clone()), deleter);
372+
std::unique_ptr<TTree, decltype(deleter)> tClone{static_cast<TTree*>(t->GetDirectory()->CloneObject(t), deleter);
373373
tClone->SetDirectory(t->GetDirectory());
374374

375375
// Clone list of variables

tree/tree/inc/TTree.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,9 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker
352352
TTree(const TTree& tt) = delete;
353353
TTree& operator=(const TTree& tt) = delete;
354354

355+
TObject *Clone(const char *newname="") const override;
356+
void Copy(TObject &object) const override;
357+
355358
virtual Int_t AddBranchToCache(const char *bname, bool subbranches = false);
356359
virtual Int_t AddBranchToCache(TBranch *branch, bool subbranches = false);
357360
virtual Int_t DropBranchFromCache(const char *bname, bool subbranches = false);

tree/tree/src/TTree.cxx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ End_Macro
466466
#include <climits>
467467
#include <algorithm>
468468
#include <set>
469+
#include <stdexcept>
469470

470471
#ifdef R__USE_IMT
471472
#include "ROOT/TThreadExecutor.hxx"
@@ -1060,6 +1061,17 @@ TTree::~TTree()
10601061
}
10611062
}
10621063

1064+
TObject *TTree::Clone(const char *) const
1065+
{
1066+
throw std::runtime_error("TObject::Clone() is not supported by TTree! Use TTree::CloneTree().");
1067+
}
1068+
1069+
void TTree::Copy(TObject &) const
1070+
{
1071+
throw std::runtime_error(
1072+
"TObject::Copy() is not supported by TTree! Use TTree::CopyTree() or TTree::CopyEntries().");
1073+
}
1074+
10631075
////////////////////////////////////////////////////////////////////////////////
10641076
/// Returns the transient buffer currently used by this TTree for reading/writing baskets.
10651077

@@ -3205,7 +3217,7 @@ TTree* TTree::CloneTree(Long64_t nentries /* = -1 */, Option_t* option /* = "" *
32053217

32063218
// Note: For a chain, the returned clone will be
32073219
// a clone of the chain's first tree.
3208-
TTree* newtree = (TTree*) thistree->Clone();
3220+
TTree* newtree = (TTree*) thistree->TNamed::Clone();
32093221
if (!newtree) {
32103222
return nullptr;
32113223
}

0 commit comments

Comments
 (0)