Skip to content

Commit 3291911

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 555c195 commit 3291911

File tree

5 files changed

+18
-7
lines changed

5 files changed

+18
-7
lines changed

bindings/pyroot/pythonizations/test/memory.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def test_ttree_clone_in_file_context(self):
4949

5050
ttree = ROOT.TTree("tree", "tree")
5151

52-
with ROOT.TFile(filename, "RECREATE") as infile:
52+
with ROOT.TFile(filename, "RECREATE") as _:
5353
ttree_clone = ttree.CloneTree()
5454

5555
os.remove(filename)
@@ -88,7 +88,6 @@ def test_objects_ownership_with_subdir(self):
8888
"TH1I": ("h", "h", 10, 0, 10),
8989
"TH1L": ("h", "h", 10, 0, 10),
9090
"TH1F": ("h", "h", 10, 0, 10),
91-
"TH1D": ("h", "h", 10, 0, 10),
9291
"TH1K": ("h", "h", 10, 0, 10),
9392
"TProfile": ("h", "h", 10, 0, 10),
9493
"TH2C": ("h", "h", 10, 0, 10, 10, 0, 10),
@@ -112,8 +111,6 @@ def test_objects_ownership_with_subdir(self):
112111
"TGraph2D": (100,),
113112
"TEntryList": ("name", "title"),
114113
"TEventList": ("name", "title"),
115-
"TTree": ("name", "title"),
116-
"TNtuple": ("name", "title", "x:y:z"),
117114
}
118115
for klass, args in objs.items():
119116
with self.subTest(klass=klass):

io/io/src/TFileMerger.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ Bool_t TFileMerger::MergeOne(TDirectory *target, TList *sourcelist, Int_t type,
530530
&& !cl->InheritsFrom( TDirectory::Class() )) {
531531
R__ASSERT(cl->IsTObject());
532532
TDirectory::TContext ctxt(current_sourcedir);
533-
obj = obj->Clone();
533+
obj = gDirectory->CloneObject(obj);
534534
ownobj = kTRUE;
535535
}
536536
} 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*>(gDirectory->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: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,17 @@ TTree::~TTree()
10601060
}
10611061
}
10621062

1063+
TObject *TTree::Clone(const char *) const
1064+
{
1065+
Error("Clone", "Not implemented, use TTree::CloneTree instead.");
1066+
return nullptr;
1067+
}
1068+
1069+
void TTree::Copy(TObject &) const
1070+
{
1071+
Error("Copy", "Not implemented, use TTree::CopyTree or TTree::CopyEntries instead.");
1072+
}
1073+
10631074
////////////////////////////////////////////////////////////////////////////////
10641075
/// Returns the transient buffer currently used by this TTree for reading/writing baskets.
10651076

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

32063217
// Note: For a chain, the returned clone will be
32073218
// a clone of the chain's first tree.
3208-
TTree* newtree = (TTree*) thistree->Clone();
3219+
TTree* newtree = (TTree*) thistree->TNamed::Clone();
32093220
if (!newtree) {
32103221
return nullptr;
32113222
}

0 commit comments

Comments
 (0)