Skip to content

Commit de9123c

Browse files
committed
[treereader] Correctly treat empty trees with range loop iteration
instead of allowing a spurious iteration with zero entries. Fixes #16249
1 parent 1a4d697 commit de9123c

2 files changed

Lines changed: 41 additions & 4 deletions

File tree

tree/treeplayer/inc/TTreeReader.h

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,28 @@ class TTreeReader: public TObject {
7878
fEntry(entry), fReader(&reader) {}
7979

8080
/// Compare two iterators for equality.
81-
bool operator==(const Iterator_t& lhs) const {
81+
bool operator==(const Iterator_t &lhs) const {
8282
// From C++14: value initialized (past-end) it compare equal.
83-
if (!IsValid() && !lhs.IsValid()) return true;
84-
return fEntry == lhs.fEntry && fReader == lhs.fReader;
83+
if (!IsValid() && !lhs.IsValid())
84+
return true;
85+
// The iterators refer to different readers
86+
if (fReader != lhs.fReader)
87+
return false;
88+
// #16249: range based loop and the tree has zero entries
89+
// as well as analogous cases.
90+
// Getting the number of events can have a cost, for example in
91+
// case of chains of remote files accessible with high latency.
92+
// However, it is reasonable to assume that if iterators are
93+
// being compared is because an iteration is taking place,
94+
// therefore such cost has to be paid anyway, it's just
95+
// anticipated.
96+
if (fReader->GetTree()->GetEntriesFast() == 0 && fEntry == 0 && !lhs.IsValid()) {
97+
return true;
98+
}
99+
if (lhs.fReader->GetTree()->GetEntriesFast() == 0 && lhs.fEntry == 0 && !IsValid()) {
100+
return true;
101+
}
102+
return fEntry == lhs.fEntry;
85103
}
86104

87105
/// Compare two iterators for inequality.
@@ -241,7 +259,7 @@ class TTreeReader: public TObject {
241259
return Iterator_t(*this, 0);
242260
}
243261
/// Return an iterator beyond the last TTree entry.
244-
Iterator_t end() const { return Iterator_t(); }
262+
Iterator_t end() { return Iterator_t(*this, -1); }
245263

246264
protected:
247265
using NamedProxies_t = std::unordered_map<std::string, std::unique_ptr<ROOT::Internal::TNamedBranchProxy>>;

tree/treeplayer/test/basic.cxx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,3 +509,22 @@ TEST(TTreeReaderBasic, DisappearingBranch)
509509
gSystem->Unlink("DisappearingBranch0.root");
510510
gSystem->Unlink("DisappearingBranch1.root");
511511
}
512+
513+
// Issue #16249
514+
TEST(TTreeReaderBasic, ZeroEntriesTree) {
515+
TTree t("t","t");
516+
TTreeReader tr(&t);
517+
auto b = tr.begin();
518+
auto e = tr.end();
519+
auto n_iterations = 0;
520+
for ([[maybe_unused]] auto &d : tr) {
521+
++n_iterations;
522+
}
523+
EXPECT_EQ(n_iterations, 0);
524+
EXPECT_TRUE(b == e);
525+
EXPECT_TRUE(b == b);
526+
EXPECT_TRUE(e == e);
527+
auto b_copy(b);
528+
EXPECT_TRUE(b++ == e);
529+
EXPECT_TRUE(++b_copy == e);
530+
}

0 commit comments

Comments
 (0)