From 3395089b5fede4df1fbd0247ab80ff582855da77 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 23 Sep 2026 23:14:26 +0200 Subject: [PATCH] Use the shared recursion limit in update() and merge_patch() 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 --- include/nlohmann/json.hpp | 63 +++++++++++++++----------------- single_include/nlohmann/json.hpp | 63 +++++++++++++++----------------- tests/src/unit-diagnostics.cpp | 2 +- tests/src/unit-merge_patch.cpp | 2 +- tests/src/unit-modifiers.cpp | 2 +- 5 files changed, 61 insertions(+), 71 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 646730563..e40a2aa27 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3916,30 +3916,35 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec update_members(first, last, merge_objects, 0); } - JSON_PRIVATE_UNLESS_TESTED: - /// the number of nested objects @ref update and @ref merge_patch descend - /// into before handing over to their iterative versions - static constexpr std::size_t merge_depth_limit() noexcept - { - return 128; - } - private: + /// @brief an object @ref update_members_iteratively or @ref + /// merge_patch_iteratively is merging into, and the members still to merge + struct merge_frame + { + merge_frame(basic_json* target_, const_iterator position_, const_iterator last_) noexcept + : target(target_), position(std::move(position_)), last(std::move(last_)) + {} + + basic_json* target; + const_iterator position; + const_iterator last; + }; + /*! @brief the members loop of @ref update, for this object and range Merging a nested object calls this function again, once per nesting level, so a value nested deeply enough used to exhaust the call stack and terminate the process. The descent is bounded here: once @ref - merge_depth_limit levels have been entered, @ref update_members_iteratively - merges what is left without the call stack. + detail::recursion_depth_limit levels have been entered, @ref + update_members_iteratively merges what is left without the call stack. @param[in] depth nesting level of this object, counted from the object @ref update was called on */ void update_members(const const_iterator& first, const const_iterator& last, const bool merge_objects, const std::size_t depth) { - if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) { update_members_iteratively(first, last); return; @@ -3976,17 +3981,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec objects whose merge was interrupted by a nested one on an explicit stack instead of descending into them. A nested object is still merged completely before the next member, in the same order as the recursive - version. Only reached for values nested deeper than @ref merge_depth_limit. + version. Only reached for values nested deeper than @ref + detail::recursion_depth_limit. */ void update_members_iteratively(const_iterator first, const_iterator last) { - struct update_frame - { - basic_json* target; - const_iterator position; - const_iterator last; - }; - std::vector stack; + std::vector stack; basic_json* target = this; while (true) @@ -4016,7 +4016,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec { const basic_json& source = first.value(); ++first; - stack.push_back({target, first, last}); + stack.emplace_back(target, first, last); target = &it2->second; first = source.cbegin(); last = source.cend(); @@ -5937,14 +5937,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec Applying a nested object calls this function again, once per nesting level, so a patch nested deeply enough used to exhaust the call stack and terminate the process. The descent is bounded here: once @ref - merge_depth_limit levels have been entered, @ref merge_patch_iteratively - applies what is left without the call stack. + detail::recursion_depth_limit levels have been entered, @ref + merge_patch_iteratively applies what is left without the call stack. */ void apply_merge_patch(const basic_json& apply_patch, const std::size_t depth) { if (apply_patch.is_object()) { - if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) { merge_patch_iteratively(apply_patch); return; @@ -5979,16 +5979,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec explicit stack instead of descending into them. A nested object is still patched completely before the next member, in the same order as the recursive version. Only reached for patches nested deeper than @ref - merge_depth_limit. + detail::recursion_depth_limit. */ void merge_patch_iteratively(const basic_json& apply_patch) { - struct merge_frame - { - basic_json* target; - const_iterator position; - const_iterator last; - }; std::vector stack; // patch `target` with `patch`, or start patching it member by member @@ -6000,7 +5994,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec { target = basic_json::object(); } - stack.push_back({&target, patch.cbegin(), patch.cend()}); + stack.emplace_back(&target, patch.cbegin(), patch.cend()); } else { @@ -6011,7 +6005,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec apply(*this, apply_patch); while (!stack.empty()) { - merge_frame& frame = stack.back(); + // a copy, as applying a member below can reallocate the stack; + // the frame itself is only changed through stack.back() + const merge_frame frame = stack.back(); if (frame.position == frame.last) { stack.pop_back(); @@ -6019,14 +6015,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } const const_iterator member = frame.position; - ++frame.position; + ++stack.back().position; if (member.value().is_null()) { frame.target->erase(member.key()); } else { - // may push, which invalidates `frame` apply(frame.target->operator[](member.key()), member.value()); } } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 60cd4f0fc..3c5821d92 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -28374,30 +28374,35 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec update_members(first, last, merge_objects, 0); } - JSON_PRIVATE_UNLESS_TESTED: - /// the number of nested objects @ref update and @ref merge_patch descend - /// into before handing over to their iterative versions - static constexpr std::size_t merge_depth_limit() noexcept - { - return 128; - } - private: + /// @brief an object @ref update_members_iteratively or @ref + /// merge_patch_iteratively is merging into, and the members still to merge + struct merge_frame + { + merge_frame(basic_json* target_, const_iterator position_, const_iterator last_) noexcept + : target(target_), position(std::move(position_)), last(std::move(last_)) + {} + + basic_json* target; + const_iterator position; + const_iterator last; + }; + /*! @brief the members loop of @ref update, for this object and range Merging a nested object calls this function again, once per nesting level, so a value nested deeply enough used to exhaust the call stack and terminate the process. The descent is bounded here: once @ref - merge_depth_limit levels have been entered, @ref update_members_iteratively - merges what is left without the call stack. + detail::recursion_depth_limit levels have been entered, @ref + update_members_iteratively merges what is left without the call stack. @param[in] depth nesting level of this object, counted from the object @ref update was called on */ void update_members(const const_iterator& first, const const_iterator& last, const bool merge_objects, const std::size_t depth) { - if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) { update_members_iteratively(first, last); return; @@ -28434,17 +28439,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec objects whose merge was interrupted by a nested one on an explicit stack instead of descending into them. A nested object is still merged completely before the next member, in the same order as the recursive - version. Only reached for values nested deeper than @ref merge_depth_limit. + version. Only reached for values nested deeper than @ref + detail::recursion_depth_limit. */ void update_members_iteratively(const_iterator first, const_iterator last) { - struct update_frame - { - basic_json* target; - const_iterator position; - const_iterator last; - }; - std::vector stack; + std::vector stack; basic_json* target = this; while (true) @@ -28474,7 +28474,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec { const basic_json& source = first.value(); ++first; - stack.push_back({target, first, last}); + stack.emplace_back(target, first, last); target = &it2->second; first = source.cbegin(); last = source.cend(); @@ -30395,14 +30395,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec Applying a nested object calls this function again, once per nesting level, so a patch nested deeply enough used to exhaust the call stack and terminate the process. The descent is bounded here: once @ref - merge_depth_limit levels have been entered, @ref merge_patch_iteratively - applies what is left without the call stack. + detail::recursion_depth_limit levels have been entered, @ref + merge_patch_iteratively applies what is left without the call stack. */ void apply_merge_patch(const basic_json& apply_patch, const std::size_t depth) { if (apply_patch.is_object()) { - if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) { merge_patch_iteratively(apply_patch); return; @@ -30437,16 +30437,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec explicit stack instead of descending into them. A nested object is still patched completely before the next member, in the same order as the recursive version. Only reached for patches nested deeper than @ref - merge_depth_limit. + detail::recursion_depth_limit. */ void merge_patch_iteratively(const basic_json& apply_patch) { - struct merge_frame - { - basic_json* target; - const_iterator position; - const_iterator last; - }; std::vector stack; // patch `target` with `patch`, or start patching it member by member @@ -30458,7 +30452,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec { target = basic_json::object(); } - stack.push_back({&target, patch.cbegin(), patch.cend()}); + stack.emplace_back(&target, patch.cbegin(), patch.cend()); } else { @@ -30469,7 +30463,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec apply(*this, apply_patch); while (!stack.empty()) { - merge_frame& frame = stack.back(); + // a copy, as applying a member below can reallocate the stack; + // the frame itself is only changed through stack.back() + const merge_frame frame = stack.back(); if (frame.position == frame.last) { stack.pop_back(); @@ -30477,14 +30473,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } const const_iterator member = frame.position; - ++frame.position; + ++stack.back().position; if (member.value().is_null()) { frame.target->erase(member.key()); } else { - // may push, which invalidates `frame` apply(frame.target->operator[](member.key()), member.value()); } } diff --git a/tests/src/unit-diagnostics.cpp b/tests/src/unit-diagnostics.cpp index e54e3c1ae..df4dd5a66 100644 --- a/tests/src/unit-diagnostics.cpp +++ b/tests/src/unit-diagnostics.cpp @@ -365,7 +365,7 @@ TEST_CASE("Regression tests for extended diagnostics") TEST_CASE("Better diagnostics past the descent bound of update() and merge_patch()") { - // Both merge objects nested more than basic_json::merge_depth_limit() + // Both merge objects nested more than detail::recursion_depth_limit() // (128) levels deep without recursing; the values they add or replace // there must still know their parents. const std::size_t depth = 200; diff --git a/tests/src/unit-merge_patch.cpp b/tests/src/unit-merge_patch.cpp index 7155932be..8ac281ef5 100644 --- a/tests/src/unit-merge_patch.cpp +++ b/tests/src/unit-merge_patch.cpp @@ -302,7 +302,7 @@ TEST_CASE("JSON Merge Patch on deeply nested values") SECTION("patching past the descent bound gives the same result") { // every depth on either side of where the iterative version takes - // over (basic_json::merge_depth_limit(), 128) + // over (detail::recursion_depth_limit(), 128) for (std::size_t depth = 0; depth <= 300; ++depth) { CAPTURE(depth); diff --git a/tests/src/unit-modifiers.cpp b/tests/src/unit-modifiers.cpp index 6693b3189..55f9d467e 100644 --- a/tests/src/unit-modifiers.cpp +++ b/tests/src/unit-modifiers.cpp @@ -1041,7 +1041,7 @@ TEST_CASE("update() on deeply nested values") SECTION("merging past the descent bound gives the same result") { // every depth on either side of where the iterative version takes - // over (basic_json::merge_depth_limit(), 128) + // over (detail::recursion_depth_limit(), 128) for (std::size_t depth = 0; depth <= 300; ++depth) { CAPTURE(depth);