[cling] Fix infinite recursion when printing self-referencing containers#21072
Conversation
devajithvs
force-pushed
the
nlohmann_fix
branch
from
2720e8f to
a3b3b98
Compare
| // If the dereferenced iterator points to the container itself, we have | ||
| // infinite recursion. This occurs with scalar values in nlohmann::json |
There was a problem hiding this comment.
I am confused. In the example, the 'points to the container itself' happens only if the container is empty and thus this line should not be reached (see line 208).
There was a problem hiding this comment.
This is probably related to the odd implementation mentioned at https://github.com/root-project/root/pull/21072/changes#r2742975180
There was a problem hiding this comment.
This is a problem in nlohmann::json, scalar values are iterable and can dereference to themselves.
root [0] nlohmann::json j;
root [1] j[0] = 1;
root [2] auto it = j.begin();
root [3] bool(it == j.end())
(bool) false
root [4] auto& elem = *it;
root [5] auto elem_it = elem.begin();
root [6] bool(elem_it == elem.end()) // line 208 check
(bool) false
root [7] bool(&elem == &(*elem_it)) // iterator dereferences to itself and infinite recursion
(bool) trueThere was a problem hiding this comment.
I was trying to somewhat mimic what scalar values does in nlohmann::json, I have simplified the test case to avoid this confusion.
|
|
||
| // When empty (scalar), iterate over self | ||
| auto begin() const { return data.empty() ? this : &data[0]; } | ||
| auto end() const { return data.empty() ? this + 1 : &data[0] + data.size(); } |
There was a problem hiding this comment.
| auto end() const { return data.empty() ? this + 1 : &data[0] + data.size(); } | |
| auto end() const { return data.empty() ? this : &data[0] + data.size(); } |
Otherwise we don't seem to have the invariant
object->empty() == ( object->begin() == object->end() );
Test Results 22 files 22 suites 3d 16h 9m 35s ⏱️ Results for commit fe20f93. ♻️ This comment has been updated with latest results. |
dpiparo
left a comment
dpiparo
left a comment
There was a problem hiding this comment.
lgtm once this comment is addressed.
Some container types (e.g., nlohmann::json scalar values) have iterators that dereference to the container itself, causing infinite recursion in `printValue_impl`.
devajithvs
force-pushed
the
nlohmann_fix
branch
from
a3b3b98 to
fe20f93
Compare
| public: | ||
| RecursionTest() = default; | ||
| auto begin() const { return this; } // iterate over self | ||
| auto end() const { return this + 1; } // just to make sure begin() != end() |
There was a problem hiding this comment.
| auto end() const { return this + 1; } // just to make sure begin() != end() | |
| auto end() const { return this + 1; } // Hard code the size of the collection to be exactly one and it 'just' contains this object. |
|
Thanks!! Question: does this PR also fix by chance the infinite loop seen in #10140 (comment) ? |
That is very unlikely. |
5 participants
Some container types (e.g., nlohmann::json scalar values) have iterators that dereference to the container itself, causing infinite recursion in
printValue_impl.This Pull request:
Changes or fixes:
Checklist:
This PR fixes #21058