Skip to content

Commit 099cfa9

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 0f34653 commit 099cfa9

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
"TProfile": ("h", "h", 10, 0, 10),
9392
"TH2C": ("h", "h", 10, 0, 10, 10, 0, 10),
9493
"TH2S": ("h", "h", 10, 0, 10, 10, 0, 10),
@@ -111,8 +110,6 @@ def test_objects_ownership_with_subdir(self):
111110
"TGraph2D": (100,),
112111
"TEntryList": ("name", "title"),
113112
"TEventList": ("name", "title"),
114-
"TTree": ("name", "title"),
115-
"TNtuple": ("name", "title", "x:y:z"),
116113
}
117114
for klass, args in objs.items():
118115
with self.subTest(klass=klass):

io/io/src/TFileMerger.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ Bool_t TFileMerger::MergeOne(TDirectory *target, TList *sourcelist, Int_t type,
529529
&& !cl->InheritsFrom( TDirectory::Class() )) {
530530
R__ASSERT(cl->IsTObject());
531531
TDirectory::TContext ctxt(current_sourcedir);
532-
obj = obj->Clone();
532+
obj = gDirectory->CloneObject(obj);
533533
ownobj = kTRUE;
534534
}
535535
} 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
@@ -354,6 +354,9 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker
354354
TTree(const TTree& tt) = delete;
355355
TTree& operator=(const TTree& tt) = delete;
356356

357+
TObject *Clone(const char *newname="") const override;
358+
void Copy(TObject &object) const override;
359+
357360
virtual Int_t AddBranchToCache(const char *bname, bool subbranches = false);
358361
virtual Int_t AddBranchToCache(TBranch *branch, bool subbranches = false);
359362
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
@@ -1059,6 +1059,17 @@ TTree::~TTree()
10591059
}
10601060
}
10611061

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

@@ -3204,7 +3215,7 @@ TTree* TTree::CloneTree(Long64_t nentries /* = -1 */, Option_t* option /* = "" *
32043215

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

0 commit comments

Comments
 (0)