Repository navigation
Flatten deeply nested values without recursing per nesting level - #5792
Merged
Merged
Conversation
This was referenced Oct 9, 2026
nlohmann
added this pull request to stack #5794
October 9, 2026 07:50
gregmarr
approved these changes
Oct 9, 2026
| } | ||
| else | ||
| { | ||
| stack.push_back({&v, 0, object_const_iterator(), path.size()}); |
Owner
Author
There was a problem hiding this comment.
Done in e82b29b: the frame got a constructor (C++11 can't emplace_back an aggregate), and both call sites use emplace_back now.
Written by Claude Code.
| } | ||
| else | ||
| { | ||
| stack.push_back({&v, 0, v.m_data.m_value.object->begin(), path.size()}); |
Owner
Author
There was a problem hiding this comment.
Done in e82b29b: the frame got a constructor (C++11 can't emplace_back an aggregate), and both call sites use emplace_back now.
Written by Claude Code.
json_pointer::flatten() called itself once per nesting level, so flatten() on a value nested deeply enough exhausted the call stack. #5547 and #5548 fixed merge_patch() and diff() from #5393, but flatten() was left out. flatten() now walks the value with an explicit stack and keeps the path in one buffer that grows and shrinks with it. It has a single code path and no depth limit: the old version built a new path string per child, so the iterative one is no slower on shallow values and much faster on deep ones. The output, including the order of an ordered_json result, is unchanged. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Give the frame a constructor so both call sites can use emplace_back, as suggested in the review; index starts at 0 for every frame. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
force-pushed
the
fix-5393-flatten-recursion
branch
from
October 9, 2026 15:06
e82b29b to
f4090fd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the remaining part of #5393.
#5393 reported three functions that recurse once per nesting level:
diff(),merge_patch()andflatten(). #5547 and #5548 fixed the first two, and the issue was closed when #5548 merged.json_pointer::flatten()is still recursive ondevelop, sostill crashes with SIGSEGV (exit 139, macOS/arm64,
-O2).Change
json_pointer::flatten()now walks the value with an explicit stack of frames. Likediff_iteratively, it keeps the path in one buffer that grows and shrinks with the stack. The signature and the output stay the same, including the entry order of anordered_jsonresult.Unlike
diffandmerge_patch, this version doesn't recurse first and switch to the explicit stack atrecursion_depth_limit(). It has one code path. The old version built a new path string for every child, which costs time quadratic in the depth. Sharing one buffer avoids that, so the iterative version is no slower even on shallow values.Benchmark
Best-of-7 time per call to
flatten(),-O2, Apple clang 21, arm64. Old and new produced identicaldump()output in every case.jsonordered_jsonjsonordered_jsonjsonjsonTests
New
TEST_CASE("flatten of structured values")inunit-json_pointer.cpp:~,/), compared with a result written out by handordered_jsonresultLocally:
test-json_pointer_cpp11,test-json_pointer_cpp20andtest-json_patch_cpp11pass, and the build shows no warnings.The round-trip through
unflatten()is only tested at 500 levels. At 100,000 levelsunflatten()used about 9.6 GB of memory and was still running after 8 minutes, apparently because it copies pointer prefixes in an amount that grows quadratically with the depth. That's a separate problem and isn't touched here.Public API
No breaking changes:
flatten()keeps its signature and its output, and only the internals ofjson_pointer::flattenchange.🤖 Generated with Claude Code