Skip to content

Merge deeply nested objects without recursing per nesting level - #5547

Merged
nlohmann merged 3 commits into
claude/iterative-hashfrom
claude/iterative-merge-patch-update
Sep 24, 2026
Merged

nlohmann merged 3 commits into
claude/iterative-hashfrom
claude/iterative-merge-patch-update

Conversation

@nlohmann

@nlohmann nlohmann commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

What & why

merge_patch() and update(j, true) merge a nested object by calling themselves on it, once per nesting level. A value nested deeply enough terminates the process with SIGSEGV: 50,000 levels of objects on an 8 MiB stack. parse() accepts such values without complaint. Reported in #5393 (merge_patch) and #5545 (update).

Part of the same series as #5546, see the table there.

The change

Bound the descent the same way dump() does:

  • The recursion now carries the nesting level. update() keeps its public checks and hands the member loop to a private update_members(), which calls itself for nested objects. merge_patch() does the same through apply_merge_patch().
  • Once detail::recursion_depth_limit() (128, shared with the other operations) levels have been entered, update_members_iteratively() and merge_patch_iteratively() finish the merge on an explicit stack.
  • The iterative versions still merge a nested object completely before the next member, in the same order. So the results, including the parents JSON_DIAGNOSTICS reports paths from, are unchanged. That matters for ordered_json, whose inserts move sibling members.

Values nested less deeply than the bound run the same code as before, so the common case doesn't pay for a stack. A first, fully iterative version cost 10–14% on ordinary merges, which is why it bounds the descent instead.

Verification

  • Identical results: a differential against develop compares update(j, true), update(j) and merge_patch() on 4,000 random pairs each, plus explicit chains crossing the bound (120–131, 200, 300 levels). Results are identical for json and ordered_json, and under JSON_DIAGNOSTICS for json, where every node's diagnostic path is compared: 82,137 paths, the deepest about 300 levels. ordered_json under JSON_DIAGNOSTICS hits a pre-existing parent-pointer assertion in merge_patch() on develop too (minimal case below); it is unaffected by this change.
  • Deep values: 1,000,000 levels merge fine on a 1 MiB stack, with and without JSON_DIAGNOSTICS; develop crashes at 100,000.
  • Speed: unchanged: update(j), update(j, true) and merge_patch() on a small nested document measure the same as develop (clang -O3, three repetitions).
  • Tests:
    • Every depth up to 300 is checked against recursive reference implementations of both operations.
    • The diagnostic paths are checked past the bound, and objects nested 100,000 levels deep are merged.
    • A mutant whose iterative update never merges fails 515 assertions; one whose iterative merge_patch doesn't erase fails 339.
  • make amalgamate + make check-amalgamation; no new clang-tidy or -Weverything findings.

Pre-existing, for the record (same on develop): with JSON_DIAGNOSTICS, ordered_json::parse(R"({"a":{"c":{"d":{}}}})").merge_patch(ordered_json::parse(R"({"a":{"c":{"c":"s","d":null},"e":"s"}})")) fails the parent assertion in assert_invariant. To be investigated separately.

Public API impact

No breaking changes. The public signatures of update() and merge_patch() are unchanged. The recursive worker behind merge_patch() has its own name rather than being a private overload, so &basic_json::merge_patch stays unambiguous.


🤖 Generated with Claude Code

@nlohmann
nlohmann added this pull request to stack #5550 September 23, 2026 20:31
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Sep 23, 2026
@nlohmann
nlohmann force-pushed the claude/iterative-merge-patch-update branch from cc18941 to e905e1d Compare September 23, 2026 20:42
Comment thread include/nlohmann/json.hpp Outdated
Comment thread include/nlohmann/json.hpp Outdated
Comment thread include/nlohmann/json.hpp Outdated
@nlohmann
nlohmann force-pushed the claude/iterative-merge-patch-update branch from e905e1d to e5a2e8d Compare September 23, 2026 21:16
@nlohmann
nlohmann removed this pull request from stack #5550 September 23, 2026 21:17
@nlohmann
nlohmann force-pushed the claude/iterative-merge-patch-update branch from e5a2e8d to 3379ed5 Compare September 24, 2026 07:39
@nlohmann
nlohmann added this pull request to stack #5568 September 24, 2026 15:09
@nlohmann nlohmann added 🚀 ready to merge Ready to merge - just waiting for CI to complete. and removed review needed It would be great if someone could review the proposed changes. labels Sep 24, 2026
@nlohmann nlohmann added this to the Release 3.13.0 milestone Sep 24, 2026
merge_patch() and update(j, true) merged a nested object by calling
themselves on it, once per nesting level. A value nested deeply enough -
50,000 levels of objects on an 8 MiB stack - exhausted the call stack
and terminated the process, although parse() accepts such values without
complaint.

Bound the descent the same way dump() does. The recursion now carries
the nesting level, and once merge_depth_limit() (128) levels have been
entered, update_members_iteratively() and merge_patch_iteratively()
finish the merge on an explicit stack. They still merge a nested object
completely before the next member, and in the same order, so the results,
including the parents JSON_DIAGNOSTICS reports paths from, are unchanged.
Values nested less deeply than the bound run the same code as before, so
the common case does not pay for the stack: merging only on it cost
10-14% in a first version.

The public signatures are unchanged. The recursive worker behind
merge_patch() has its own name rather than being a private overload, so
that &basic_json::merge_patch stays unambiguous.

Tests check every depth up to 300 against recursive reference
implementations of both operations, check the diagnostic paths past the
bound, and merge objects nested 100,000 levels deep.

Fixes #5545 for update(j, true), and #5393 for merge_patch().

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
merge_depth_limit() is gone in favor of detail::recursion_depth_limit().
The two identical function-local frame structs become one member struct,
merge_frame, with a constructor, so both loops emplace_back() their
frames. merge_patch_iteratively() copies the frame it works on out of the
stack and changes it only through stack.back().

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
…arsing them

Parsed values carry byte positions under JSON_DIAGNOSTIC_POSITIONS, which
the expected messages do not include.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann force-pushed the claude/iterative-merge-patch-update branch from 3379ed5 to 6f971ae Compare September 24, 2026 15:11
@nlohmann
nlohmann merged commit 4daca40 into develop Sep 24, 2026
2 of 142 checks passed
nlohmann added a commit that referenced this pull request Sep 25, 2026
…tests (#5575)

The windows-11-arm runner image moved to windows-11-vs2026-arm64, which no
longer ships Visual Studio 2022, so the msvc-arm64 job failed at configure
time. Use the "Visual Studio 18 2026" generator like the msvc2026 job.

clang-tidy's modernize-raw-string-literal check flagged two string literals
in the nesting tests added by #5546 and #5547.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann added a commit that referenced this pull request Oct 9, 2026
* Flatten deeply nested values without recursing per nesting level

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>

* Construct flatten frames in place

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>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L 🚀 ready to merge Ready to merge - just waiting for CI to complete. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants