Compare commits

..
Author SHA1 Message Date
Niels Lohmann 450fc8dce7 Preserve diff()'s original op ordering and fix a slow-path deletion gap
Splitting removed-key detection and common-key recursion into separate
passes (for the earlier lookup-count fix) changed the emitted patch's
op order: all "remove" ops now came before all recursive per-key diffs,
instead of interleaved in source's iteration order as the original
implementation did. This broke docs/mkdocs/docs/examples/diff.output's
exact-match CI check (ci_test_examples) even though the patch was still
semantically correct.

Defer "remove" emission into the same walk that does the recursive
diffs, so common keys and deleted keys are interleaved in source order
again, matching historical output.

While restructuring that walk, the reordering ("slow path") branch was
only emitting "remove" for keys common to both objects, never for keys
present in source but genuinely absent from target -- a key deleted
alongside an actual reorder would silently survive the patch. Fixed by
removing every source key in the slow path (both deleted and common
keys need removing there; common keys are then re-added in target's
order). Verified with a targeted reorder+deletion case and a fresh
20,000-case round-trip fuzz run (0 failures).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 22:01:40 +02:00
Niels Lohmann 3a186c4570 Avoid redundant lookups in diff()'s object-order tracking
The previous fix for ordered_json member order re-derived common-key
order and suffix information with extra target.find()/source.find()
calls layered on top of the pre-existing removed/added-key passes,
instead of reusing those same passes. This roughly tripled the number
of map lookups per diff() call for every object, including plain
`json`, where the reordering path is never taken.

Piggyback the order tracking (and the "add" op construction for new
keys) onto the two passes the algorithm already needs to detect
removed/added keys, and walk the fast path's recursion in lockstep
with the precomputed common-key list instead of re-querying `target`.
This restores diff() to its pre-existing lookup count; benchmarked at
n=1000 keys, ordered_json::diff() was roughly 2x slower than baseline
before this change and is back within noise of baseline after it.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 22:01:40 +02:00
Niels Lohmann d82ab21724 Make diff() account for member order in ordered_json objects
diff() compared source/target objects purely by key set, ignoring
relative member order. For ordered_json (insertion-ordered, vector-
backed object_t), two objects that differ only in member order are
unequal via operator==, but diff() never emitted any patch operation
to fix the order, so source.patch(diff(source, target)) == target
could fail to hold.

Fix by detecting when common keys appear in a different relative
order in source vs. target (or when a new key would need to land
somewhere other than the end), and in that case removing and
re-adding the affected keys in target's order, which relies on
patch()'s "add" op appending new keys at the end of an ordered_map.
For plain json (std::map-backed, always key-sorted iteration) this
is a no-op and the original minimal per-key diff path is unchanged.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 22:01:40 +02:00
Niels Lohmann 3bb551f46f Fix swap(array_t&)/swap(object_t&) to update parent pointers under JSON_DIAGNOSTICS
Both overloads swapped the underlying container storage but never called
set_parents(), leaving elements moved into *this with stale m_parent
pointers (typically nullptr from the free-standing array_t/object_t).
This produced wrong JSON Pointer paths in diagnostic messages and could
trip assert_invariant() on subsequent copies. Mirrors the fix already
applied in swap(reference other).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 22:01:39 +02:00
6 changed files with 358 additions and 163 deletions
+2 -14
View File
@@ -71,7 +71,7 @@ class serializer
, thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep))) , thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep)))
, decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point))) , decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point)))
, indent_char(ichar) , indent_char(ichar)
, indent_string() , indent_string(512, indent_char)
, error_handler(error_handler_) , error_handler(error_handler_)
{} {}
@@ -126,10 +126,6 @@ class serializer
// variable to hold indentation for recursive calls // variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step; const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent)) if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{ {
indent_string.resize(indent_string.size() * 2, ' '); indent_string.resize(indent_string.size() * 2, ' ');
@@ -203,10 +199,6 @@ class serializer
// variable to hold indentation for recursive calls // variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step; const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent)) if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{ {
indent_string.resize(indent_string.size() * 2, ' '); indent_string.resize(indent_string.size() * 2, ' ');
@@ -268,10 +260,6 @@ class serializer
// variable to hold indentation for recursive calls // variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step; const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent)) if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{ {
indent_string.resize(indent_string.size() * 2, ' '); indent_string.resize(indent_string.size() * 2, ' ');
@@ -1022,7 +1010,7 @@ class serializer
/// the indentation character /// the indentation character
const char indent_char; const char indent_char;
/// the indentation string (lazily allocated on first use by a pretty-print branch) /// the indentation string
string_t indent_string; string_t indent_string;
/// error_handler how to react on decoding errors /// error_handler how to react on decoding errors
+119 -12
View File
@@ -3573,6 +3573,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{ {
using std::swap; using std::swap;
swap(*(m_data.m_value.array), other); swap(*(m_data.m_value.array), other);
set_parents();
} }
else else
{ {
@@ -3589,6 +3590,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{ {
using std::swap; using std::swap;
swap(*(m_data.m_value.object), other); swap(*(m_data.m_value.object), other);
set_parents();
} }
else else
{ {
@@ -5157,21 +5159,96 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
case value_t::object: case value_t::object:
{ {
// first pass: traverse this object's elements // first pass: record, for every source key, whether it is
// common to both objects (in source's iteration order) or
// was deleted (i.e., in source but not in target) -- this is
// a by-product of the target.find() call already needed to
// tell the two cases apart, so it adds no extra lookups. The
// "remove" ops themselves are emitted later, interleaved
// with the recursive per-key diffs in the fast path below,
// to match source's original iteration order (as the
// original, pre-reordering-aware implementation did) instead
// of grouping all removes before all recursive diffs.
std::vector<typename object_t::key_type> common_keys_source_order;
for (auto it = source.cbegin(); it != source.cend(); ++it) for (auto it = source.cbegin(); it != source.cend(); ++it)
{ {
// escape the key name to be used in a JSON patch
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
if (target.find(it.key()) != target.end()) if (target.find(it.key()) != target.end())
{ {
// recursive call to compare object values at key it common_keys_source_order.push_back(it.key());
auto temp_diff = diff(it.value(), target[it.key()], path_key); }
result.insert(result.end(), temp_diff.begin(), temp_diff.end()); }
// second pass: find keys that were added (i.e., in target but
// not in source), and record the keys common to both, in
// target's iteration order -- again a by-product of the
// source.find() call already needed to detect added keys. At
// 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`.
// 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
// used by the fast path -- the slow (reordering) path
// rebuilds "add" ops for every key itself.
std::vector<typename object_t::key_type> common_keys_target_order;
basic_json added_ops(value_t::array);
bool new_keys_form_suffix = true;
bool seen_new_key = false;
for (auto it = target.cbegin(); it != target.cend(); ++it)
{
if (source.find(it.key()) == source.end())
{
seen_new_key = true;
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
added_ops.push_back(
{
{"op", "add"}, {"path", path_key},
{"value", it.value()}
});
} }
else else
{ {
// found a key that is not in o -> remove it common_keys_target_order.push_back(it.key());
if (seen_new_key)
{
new_keys_form_suffix = false;
}
}
}
if (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
// insertion history), so a plain per-key recursive diff
// is correct and minimal, as before. common_keys_source_order
// is, by construction, the subsequence of source's keys
// that are common to both objects, in source's iteration
// order -- so it can be walked in lockstep with `source`
// using a cheap key comparison instead of another lookup.
// Deleted keys (those source keys not in common_keys_source_order)
// are interleaved here too, in source's original order, to
// match the historical (pre-reordering-aware) output order.
auto common_it = common_keys_source_order.cbegin();
for (auto it = source.cbegin(); it != source.cend(); ++it)
{
if (common_it != common_keys_source_order.cend() && it.key() == *common_it)
{
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
auto temp_diff = diff(it.value(), target[it.key()], path_key);
result.insert(result.end(), temp_diff.begin(), temp_diff.end());
++common_it;
}
else
{
// found a key that is not in target -> remove it
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
result.push_back(object( result.push_back(object(
{ {
{"op", "remove"}, {"path", path_key} {"op", "remove"}, {"path", path_key}
@@ -5179,12 +5256,42 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
} }
} }
// second pass: traverse other object's elements // append the "add" ops for brand-new keys collected above
// during the pass over target -- no second source.find()
// per target key needed
result.insert(result.end(), added_ops.begin(), added_ops.end());
}
else
{
// slow path: the common keys are in a different relative
// order in source and target (only possible for a
// reorderable object_t like ordered_map). Building a
// minimal reordering patch is a nontrivial (LCS-like)
// problem; instead, remove every source key -- both
// deleted keys (which must be removed regardless) and
// common keys (removed so they can be re-added in
// target's order) -- and re-add every key that should
// remain, with its final target value, in target's
// order. basic_json::patch()'s "add" operation on an
// object uses operator[], which appends at the end for a
// vector-backed insertion-ordered map when the key does
// not already exist -- so removing a key and then adding
// it moves it to the end, fixing its position.
for (auto it = source.cbegin(); it != source.cend(); ++it)
{
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
result.push_back(object(
{
{"op", "remove"}, {"path", path_key}
}));
}
// add every key that is either common (just removed
// above) or brand new, in target's iteration order, so
// that the final order after applying the patch matches
// target exactly
for (auto it = target.cbegin(); it != target.cend(); ++it) for (auto it = target.cbegin(); it != target.cend(); ++it)
{ {
if (source.find(it.key()) == source.end())
{
// found a key that is not in this -> add it
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key())); const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
result.push_back( result.push_back(
{ {
+121 -26
View File
@@ -20150,7 +20150,7 @@ class serializer
, thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep))) , thousands_sep(loc->thousands_sep == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->thousands_sep)))
, decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point))) , decimal_point(loc->decimal_point == nullptr ? '\0' : std::char_traits<char>::to_char_type(* (loc->decimal_point)))
, indent_char(ichar) , indent_char(ichar)
, indent_string() , indent_string(512, indent_char)
, error_handler(error_handler_) , error_handler(error_handler_)
{} {}
@@ -20205,10 +20205,6 @@ class serializer
// variable to hold indentation for recursive calls // variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step; const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent)) if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{ {
indent_string.resize(indent_string.size() * 2, ' '); indent_string.resize(indent_string.size() * 2, ' ');
@@ -20282,10 +20278,6 @@ class serializer
// variable to hold indentation for recursive calls // variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step; const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent)) if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{ {
indent_string.resize(indent_string.size() * 2, ' '); indent_string.resize(indent_string.size() * 2, ' ');
@@ -20347,10 +20339,6 @@ class serializer
// variable to hold indentation for recursive calls // variable to hold indentation for recursive calls
const auto new_indent = current_indent + indent_step; const auto new_indent = current_indent + indent_step;
if (JSON_HEDLEY_UNLIKELY(indent_string.empty()))
{
indent_string.resize(512, indent_char);
}
if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent)) if (JSON_HEDLEY_UNLIKELY(indent_string.size() < new_indent))
{ {
indent_string.resize(indent_string.size() * 2, ' '); indent_string.resize(indent_string.size() * 2, ' ');
@@ -21101,7 +21089,7 @@ class serializer
/// the indentation character /// the indentation character
const char indent_char; const char indent_char;
/// the indentation string (lazily allocated on first use by a pretty-print branch) /// the indentation string
string_t indent_string; string_t indent_string;
/// error_handler how to react on decoding errors /// error_handler how to react on decoding errors
@@ -25013,6 +25001,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{ {
using std::swap; using std::swap;
swap(*(m_data.m_value.array), other); swap(*(m_data.m_value.array), other);
set_parents();
} }
else else
{ {
@@ -25029,6 +25018,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{ {
using std::swap; using std::swap;
swap(*(m_data.m_value.object), other); swap(*(m_data.m_value.object), other);
set_parents();
} }
else else
{ {
@@ -26597,21 +26587,96 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
case value_t::object: case value_t::object:
{ {
// first pass: traverse this object's elements // first pass: record, for every source key, whether it is
// common to both objects (in source's iteration order) or
// was deleted (i.e., in source but not in target) -- this is
// a by-product of the target.find() call already needed to
// tell the two cases apart, so it adds no extra lookups. The
// "remove" ops themselves are emitted later, interleaved
// with the recursive per-key diffs in the fast path below,
// to match source's original iteration order (as the
// original, pre-reordering-aware implementation did) instead
// of grouping all removes before all recursive diffs.
std::vector<typename object_t::key_type> common_keys_source_order;
for (auto it = source.cbegin(); it != source.cend(); ++it) for (auto it = source.cbegin(); it != source.cend(); ++it)
{ {
// escape the key name to be used in a JSON patch
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
if (target.find(it.key()) != target.end()) if (target.find(it.key()) != target.end())
{ {
// recursive call to compare object values at key it common_keys_source_order.push_back(it.key());
auto temp_diff = diff(it.value(), target[it.key()], path_key); }
result.insert(result.end(), temp_diff.begin(), temp_diff.end()); }
// second pass: find keys that were added (i.e., in target but
// not in source), and record the keys common to both, in
// target's iteration order -- again a by-product of the
// source.find() call already needed to detect added keys. At
// 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`.
// 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
// used by the fast path -- the slow (reordering) path
// rebuilds "add" ops for every key itself.
std::vector<typename object_t::key_type> common_keys_target_order;
basic_json added_ops(value_t::array);
bool new_keys_form_suffix = true;
bool seen_new_key = false;
for (auto it = target.cbegin(); it != target.cend(); ++it)
{
if (source.find(it.key()) == source.end())
{
seen_new_key = true;
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
added_ops.push_back(
{
{"op", "add"}, {"path", path_key},
{"value", it.value()}
});
} }
else else
{ {
// found a key that is not in o -> remove it common_keys_target_order.push_back(it.key());
if (seen_new_key)
{
new_keys_form_suffix = false;
}
}
}
if (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
// insertion history), so a plain per-key recursive diff
// is correct and minimal, as before. common_keys_source_order
// is, by construction, the subsequence of source's keys
// that are common to both objects, in source's iteration
// order -- so it can be walked in lockstep with `source`
// using a cheap key comparison instead of another lookup.
// Deleted keys (those source keys not in common_keys_source_order)
// are interleaved here too, in source's original order, to
// match the historical (pre-reordering-aware) output order.
auto common_it = common_keys_source_order.cbegin();
for (auto it = source.cbegin(); it != source.cend(); ++it)
{
if (common_it != common_keys_source_order.cend() && it.key() == *common_it)
{
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
auto temp_diff = diff(it.value(), target[it.key()], path_key);
result.insert(result.end(), temp_diff.begin(), temp_diff.end());
++common_it;
}
else
{
// found a key that is not in target -> remove it
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
result.push_back(object( result.push_back(object(
{ {
{"op", "remove"}, {"path", path_key} {"op", "remove"}, {"path", path_key}
@@ -26619,12 +26684,42 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
} }
} }
// second pass: traverse other object's elements // append the "add" ops for brand-new keys collected above
// during the pass over target -- no second source.find()
// per target key needed
result.insert(result.end(), added_ops.begin(), added_ops.end());
}
else
{
// slow path: the common keys are in a different relative
// order in source and target (only possible for a
// reorderable object_t like ordered_map). Building a
// minimal reordering patch is a nontrivial (LCS-like)
// problem; instead, remove every source key -- both
// deleted keys (which must be removed regardless) and
// common keys (removed so they can be re-added in
// target's order) -- and re-add every key that should
// remain, with its final target value, in target's
// order. basic_json::patch()'s "add" operation on an
// object uses operator[], which appends at the end for a
// vector-backed insertion-ordered map when the key does
// not already exist -- so removing a key and then adding
// it moves it to the end, fixing its position.
for (auto it = source.cbegin(); it != source.cend(); ++it)
{
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
result.push_back(object(
{
{"op", "remove"}, {"path", path_key}
}));
}
// add every key that is either common (just removed
// above) or brand new, in target's iteration order, so
// that the final order after applying the patch matches
// target exactly
for (auto it = target.cbegin(); it != target.cend(); ++it) for (auto it = target.cbegin(); it != target.cend(); ++it)
{ {
if (source.find(it.key()) == source.end())
{
// found a key that is not in this -> add it
const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key())); const auto path_key = detail::concat<string_t>(path, '/', detail::escape(it.key()));
result.push_back( result.push_back(
{ {
+31
View File
@@ -273,5 +273,36 @@ TEST_CASE("Regression tests for extended diagnostics")
CHECK(j1["numbers"]["two"] == 2); CHECK(j1["numbers"]["two"] == 2);
CHECK(j1["string"] == "t"); CHECK(j1["string"] == "t");
} }
SECTION("Regression test - swap(array_t&)/swap(object_t&) must update JSON_DIAGNOSTICS parent pointers")
{
// swap(array_t&)
{
json j = json::array();
json::array_t arr = {json::array({1})};
j.swap(arr);
// parent pointers of the moved-in elements must point into j, not
// into the now-defunct free-standing array_t
CHECK_THROWS_WITH_AS(j[0][0].get<std::string>(), "[json.exception.type_error.302] (/0/0) type must be string, but is number", json::type_error);
// must not trigger assert_invariant() in a debug/assert-enabled build
json const k = j;
CHECK(k == j);
}
// swap(object_t&)
{
json o = json::object();
json::object_t obj = {{"a", json::array({1})}};
o.swap(obj);
CHECK_THROWS_WITH_AS(o["a"][0].get<std::string>(), "[json.exception.type_error.302] (/a/0) type must be string, but is number", json::type_error);
// must not trigger assert_invariant() in a debug/assert-enabled build
json const p = o;
CHECK(p == o);
}
}
} }
+81
View File
@@ -81,3 +81,84 @@ TEST_CASE("regression test for issue #3732 - iteration_proxy_value<iter_impl<ord
}; };
static_cast<void>(fn); static_cast<void>(fn);
} }
TEST_CASE("regression test - diff() must account for ordered_json member order")
{
SECTION("pure reorder, no value changes")
{
ordered_json a = {{"a", 1}, {"b", 2}};
ordered_json b = {{"b", 2}, {"a", 1}};
CHECK(a != b); // order-sensitive equality
CHECK(a.patch(ordered_json::diff(a, b)) == b);
}
SECTION("new key must land at the front")
{
ordered_json c = {{"b", 2}};
ordered_json e = {{"a", 1}, {"b", 2}};
CHECK(c.patch(ordered_json::diff(c, e)) == e);
}
SECTION("reorder plus a value change on one of the reordered keys")
{
ordered_json a = {{"a", 1}, {"b", 2}};
ordered_json b = {{"b", 20}, {"a", 1}};
CHECK(a != b);
CHECK(a.patch(ordered_json::diff(a, b)) == b);
}
SECTION("reorder plus a deleted key")
{
ordered_json a = {{"a", 1}, {"b", 2}, {"c", 3}};
ordered_json b = {{"b", 2}, {"a", 1}};
CHECK(a != b);
CHECK(a.patch(ordered_json::diff(a, b)) == b);
}
SECTION("reorder plus a nested value that itself needs a recursive diff")
{
ordered_json a = {{"a", {{"x", 1}, {"y", 2}}}, {"b", 2}};
ordered_json b = {{"b", 2}, {"a", {{"x", 1}, {"y", 99}}}};
CHECK(a != b);
CHECK(a.patch(ordered_json::diff(a, b)) == b);
}
SECTION("three or more keys shuffled into a different order")
{
ordered_json a = {{"a", 1}, {"b", 2}, {"c", 3}, {"d", 4}};
ordered_json b = {{"d", 4}, {"b", 2}, {"a", 1}, {"c", 3}};
CHECK(a != b);
CHECK(a.patch(ordered_json::diff(a, b)) == b);
}
SECTION("matching order still produces a minimal patch (fast path unaffected)")
{
ordered_json a = {{"a", 1}, {"b", 2}, {"c", 3}};
ordered_json b = {{"a", 1}, {"b", 20}, {"c", 3}};
auto p = ordered_json::diff(a, b);
// only the changed value should be touched, not a wholesale remove+add
CHECK(p.size() == 1);
CHECK(p[0]["op"] == "replace");
CHECK(p[0]["path"] == "/b");
CHECK(a.patch(p) == b);
}
SECTION("plain json (std::map-backed) is unaffected by same-key-different-insertion-order")
{
json a;
a["b"] = 2;
a["a"] = 1;
json b;
b["a"] = 1;
b["b"] = 2;
// std::map iteration is always sorted by key, so a == b regardless of
// insertion order, and diff() must still produce the same minimal
// (empty) result as before this fix
CHECK(a == b);
auto p = json::diff(a, b);
CHECK(p.empty());
CHECK(a.patch(p) == b);
}
}
-107
View File
@@ -14,40 +14,6 @@ using nlohmann::json;
#include <array> #include <array>
#include <sstream> #include <sstream>
#include <iomanip> #include <iomanip>
#include <cstdlib>
#include <new>
namespace
{
// heap allocation counter used by the regression test for issue #5413
// (https://github.com/nlohmann/json/issues/5413); disabled (and thus a
// no-op besides the counting) unless explicitly toggled on
bool count_heap_allocations = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
std::size_t heap_allocations = 0; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
} // namespace
void* operator new (std::size_t size) // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
if (count_heap_allocations)
{
++heap_allocations;
}
if (void* ptr = std::malloc(size)) // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
{
return ptr;
}
throw std::bad_alloc(); // NOLINT(hicpp-exception-baseclass)
}
void operator delete (void* ptr) noexcept // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
std::free(ptr); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
}
void operator delete (void* ptr, std::size_t /*size*/) noexcept // NOLINT(cppcoreguidelines-owning-memory,misc-new-delete-overloads)
{
std::free(ptr); // NOLINT(cppcoreguidelines-no-malloc,cppcoreguidelines-owning-memory)
}
TEST_CASE("serialization") TEST_CASE("serialization")
{ {
@@ -416,76 +382,3 @@ TEST_CASE("dump for basic_json with long double number_float_t")
check_same(100.0L, 100.0); check_same(100.0L, 100.0);
} }
} }
TEST_CASE("regression test for issue #5413 - lazily allocated indent_string")
{
// the serializer used to unconditionally allocate a 512-byte
// indent_string in its constructor, even though it is only ever read
// inside the pretty_print branches of dump(). This wasted a heap
// allocation (and its matching deallocation) on every single compact
// (i.e. non-pretty, the default) dump() call. indent_string is now
// allocated lazily, the first time a pretty-print branch actually
// needs it -- so a compact dump() must perform strictly fewer heap
// allocations than a pretty dump() of the same value.
const json j = {{"level", "info"}, {"msg", "hello world"}, {"id", 12345}};
// warm up anything unrelated to indentation (e.g., one-time locale
// lookups) that might otherwise allocate on first use regardless of
// pretty-printing, so it does not skew the counts measured below
const auto warmup = j.dump();
const auto warmup_pretty = j.dump(4);
CHECK(!warmup.empty());
CHECK(!warmup_pretty.empty());
SECTION("compact dump() has a stable, minimal allocation count")
{
count_heap_allocations = true;
heap_allocations = 0;
const auto compact1 = j.dump();
const auto allocs_compact1 = heap_allocations;
heap_allocations = 0;
const auto compact2 = j.dump(-1);
const auto allocs_compact2 = heap_allocations;
count_heap_allocations = false;
CHECK(compact1 == compact2);
// dump() and dump(-1) both take the compact code path and must
// never touch indent_string, so they allocate identically often
CHECK(allocs_compact1 == allocs_compact2);
}
SECTION("first pretty dump() allocates more than a compact dump()")
{
// use a tiny value whose compact ({"a":1}, 7 bytes) and pretty
// ({"a": 1} with 1-space indent, 11 bytes) serializations both stay
// well inside every common std::string small-string-optimization
// buffer (>= 15 bytes on libstdc++/MSVC STL, >= 22 on libc++), so
// building the result string itself causes no heap allocation
// either way -- isolating indent_string as the only thing that can
// possibly account for a difference in allocation count
const json tiny = {{"a", 1}};
count_heap_allocations = true;
heap_allocations = 0;
const auto compact = tiny.dump();
const auto allocs_compact = heap_allocations;
heap_allocations = 0;
const auto pretty = tiny.dump(1);
const auto allocs_pretty = heap_allocations;
count_heap_allocations = false;
CHECK(compact == "{\"a\":1}");
CHECK(pretty == "{\n \"a\": 1\n}");
// a fresh serializer is created per dump() call; the pretty branch
// lazily allocates indent_string on its first use, so it must
// allocate at least once more than the compact branch, which never
// touches indent_string at all
CHECK(allocs_pretty > allocs_compact);
}
}