From 115182650851ed3d5a193b7fdc356bfd5552da88 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 20:07:10 +0200 Subject: [PATCH] Take diff()'s fast path unless the object type reorders members (#5691) * Take diff()'s fast path unless the object type reorders members For every object type except an insertion-ordered one like ordered_map, diff() no longer produced a member-by-member patch when target had a key that sorts before a key the two objects share: it fell through to the slow path, which removes every member of source and re-adds every member of target, instead of just adding the new key. #5465 added an order check to require the fast path to also reproduce target's member order, needed because ordered_map's patch()-driven "add" appends a new member at the end. The check compared the common keys' order between source and target and also required that every added key come after every common key in target's order ("new_keys_form_suffix"). The comment above it argued this check is always true for std::map, and that reasoning is correct for the order of the common keys themselves, but not for new_keys_form_suffix: a std::map iterates in sorted key order, so a new key that sorts before an existing common key is enumerated between common keys, making new_keys_form_suffix false even though std::map's own key order does not need reordering at all - it places every member itself, regardless of insertion history, so a member-by-member diff already reproduces target's iteration order. Only require the order check for an object type that keeps insertion order, using the same detail::is_ordered_map trait the library already uses to recognize such an object type in set_parent(). Every other object type - std::map in key order, a hash map in an order its operator== ignores - always takes the fast path. Added a regression test to unit-json_patch.cpp: the issue's example now yields a single "add" op for json, while ordered_json still takes the slow path to reproduce target's member order. Fixes #5639. Signed-off-by: Niels Lohmann * Only track the target key order in diff() for insertion-ordered objects common_keys_target_order and new_keys_form_suffix are only read when object_t keeps its members in insertion order; skip building them otherwise. Addresses review comment by @gregmarr. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- include/nlohmann/json.hpp | 35 +++++++++++++++------ single_include/nlohmann/json.hpp | 35 +++++++++++++++------ tests/src/unit-json_patch.cpp | 52 ++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 20 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3bc72805c..7f0f1d7c2 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -6156,12 +6156,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // the same time, determine whether every added key comes // after every common key in target's order (a precondition // for the fast path below, which only ever appends new keys - // at the very end): for an object_t whose iteration order is - // a pure function of the key set (e.g. the default std::map, - // which always iterates in sorted key order), the order - // check further below is always true and this whole - // mechanism is effectively a no-op; it only matters for a - // reorderable object_t such as the one backing `ordered_json`. + // at the very end). Both are only needed for an object_t that + // keeps its members in insertion order, such as the one + // backing `ordered_json`; for any other object_t, the fast + // path is always taken and they are not computed. // patch ops for keys that were added (i.e., in target but not // in source); built here so the fast path below can reuse // them without a second source.find() per target key. Only @@ -6185,15 +6183,32 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } else { - common_keys_target_order.push_back(it.key()); - if (seen_new_key) +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning(push ) +#pragma warning(disable : 4127) // ignore warning to replace if with if constexpr +#endif + if (detail::is_ordered_map::value) { - new_keys_form_suffix = false; + common_keys_target_order.push_back(it.key()); + if (seen_new_key) + { + new_keys_form_suffix = false; + } } +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning( pop ) +#endif } } - if (common_keys_source_order == common_keys_target_order && new_keys_form_suffix) + // Only an object type that keeps its members in insertion + // order, such as nlohmann::ordered_map, can need reordering: + // patch() appends a new member at the end of such an object. + // Any other object type places its members itself - std::map + // in key order, a hash map in an order its operator== ignores - + // so a member-by-member diff always reproduces target there. + if (!detail::is_ordered_map::value + || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)) { // fast path: order of common keys already matches (or the // object_t's iteration order does not depend on diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 8b442e3db..96a3e67ad 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -33040,12 +33040,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // the same time, determine whether every added key comes // after every common key in target's order (a precondition // for the fast path below, which only ever appends new keys - // at the very end): for an object_t whose iteration order is - // a pure function of the key set (e.g. the default std::map, - // which always iterates in sorted key order), the order - // check further below is always true and this whole - // mechanism is effectively a no-op; it only matters for a - // reorderable object_t such as the one backing `ordered_json`. + // at the very end). Both are only needed for an object_t that + // keeps its members in insertion order, such as the one + // backing `ordered_json`; for any other object_t, the fast + // path is always taken and they are not computed. // patch ops for keys that were added (i.e., in target but not // in source); built here so the fast path below can reuse // them without a second source.find() per target key. Only @@ -33069,15 +33067,32 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } else { - common_keys_target_order.push_back(it.key()); - if (seen_new_key) +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning(push ) +#pragma warning(disable : 4127) // ignore warning to replace if with if constexpr +#endif + if (detail::is_ordered_map::value) { - new_keys_form_suffix = false; + common_keys_target_order.push_back(it.key()); + if (seen_new_key) + { + new_keys_form_suffix = false; + } } +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning( pop ) +#endif } } - if (common_keys_source_order == common_keys_target_order && new_keys_form_suffix) + // Only an object type that keeps its members in insertion + // order, such as nlohmann::ordered_map, can need reordering: + // patch() appends a new member at the end of such an object. + // Any other object type places its members itself - std::map + // in key order, a hash map in an order its operator== ignores - + // so a member-by-member diff always reproduces target there. + if (!detail::is_ordered_map::value + || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)) { // fast path: order of common keys already matches (or the // object_t's iteration order does not depend on diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 216d00c41..ef239e9f4 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -1752,6 +1752,58 @@ TEST_CASE("JSON patch - diff emits array removals in descending index order") } } +TEST_CASE("JSON patch - diff() takes the fast path for non-reorderable object types (regression #5639)") +{ + // #5465 added an order check to diff()'s object handling so a + // member-by-member diff is only used when it would also reproduce + // target's member *order* -- needed for ordered_json, whose object_t + // keeps insertion order and whose patch() "add" op appends a new + // member at the end. For json's default object_t (std::map, which + // orders members by key regardless of insertion history), that check + // could still fail: a new key that sorts before an existing common key + // makes target's iteration interleave the new key between common keys, + // even though nothing else about the object changed. That sent the + // whole object through the slow (remove-every-member, + // re-add-every-member) path instead of the minimal one. + SECTION("json: added key sorts before an existing common key") + { + const json source = {{"a", 1}, {"c", {{"x", 1}, {"y", 2}}}}; + const json target = {{"a", 1}, {"b", 0}, {"c", {{"x", 1}, {"y", 2}}}}; + + const json patch = json::diff(source, target); + + // only the new key is added; "a" and "c" are left alone instead of + // being removed and re-added + const json expected = R"([{"op": "add", "path": "/b", "value": 0}])"_json; + CHECK(patch == expected); + CHECK(source.patch(patch) == target); + } + + SECTION("ordered_json: reordering behavior from #5465 is unchanged") + { + using nlohmann::ordered_json; + + // same key/value shape as the json case above, but for ordered_json + // the *target*'s member order must be reproduced, so the slow path + // is still required here. + ordered_json source; + source["a"] = 1; + source["c"] = ordered_json{{"x", 1}, {"y", 2}}; + + ordered_json target; + target["a"] = 1; + target["b"] = 0; + target["c"] = ordered_json{{"x", 1}, {"y", 2}}; + + const ordered_json patch = ordered_json::diff(source, target); + + // unlike the json case: every member is still removed and re-added + // so the result ends up in target's order (2 removes + 3 adds) + CHECK(patch.size() == 5); + CHECK(source.patch(patch) == target); + } +} + TEST_CASE("JSON patch - every operation on ordered_json") { using nlohmann::ordered_json;