From 4f115126414cbe60380d142659d308a53fa66d79 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 15 Oct 2026 20:01:22 +0200 Subject: [PATCH] Speed up container destruction, iterator access and diff() (#5835) * Speed up container destruction, iterator access and diff() Signed-off-by: Niels Lohmann * Destroy the first levels of a value by recursion The pointer-reversing walk in destroy_container() avoids the call stack but moves a value around for every container it visits. Recurse for the first recursion_depth_limit() levels instead - no allocation, no thread-local, a bounded frame - and hand anything deeper to the walk. Destroy twitter 0.49 -> 0.36 ms, citm 0.80 -> 0.74 ms (3.12.0: 0.47 / 1.00). Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- .../nlohmann/detail/iterators/iter_impl.hpp | 197 ++---- include/nlohmann/json.hpp | 346 +++++++---- single_include/nlohmann/json.hpp | 583 +++++++++--------- tests/src/unit-constructor2.cpp | 54 ++ tests/src/unit-iterators1.cpp | 107 ++++ tests/src/unit-json_patch.cpp | 37 ++ 6 files changed, 792 insertions(+), 532 deletions(-) diff --git a/include/nlohmann/detail/iterators/iter_impl.hpp b/include/nlohmann/detail/iterators/iter_impl.hpp index 81eae570d..b7c1df24a 100644 --- a/include/nlohmann/detail/iterators/iter_impl.hpp +++ b/include/nlohmann/detail/iterators/iter_impl.hpp @@ -294,40 +294,25 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); - return m_it.object_iterator->second; - } - - case value_t::array: - { - JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); - return *m_it.array_iterator; - } - - case value_t::null: - throw_cannot_get_value(); - - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - if (JSON_HEDLEY_LIKELY(m_it.primitive_iterator.is_begin())) - { - return *m_object; - } - - throw_cannot_get_value(); - } + JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); + return *m_it.array_iterator; } + + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); + return m_it.object_iterator->second; + } + + // neither array nor object: a primitive value (null has no value to point to) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type != value_t::null && m_it.primitive_iterator.is_begin())) + { + return *m_object; + } + + throw_cannot_get_value(); } /*! @@ -338,38 +323,25 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); - return &(m_it.object_iterator->second); - } - - case value_t::array: - { - JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); - return &*m_it.array_iterator; - } - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - if (JSON_HEDLEY_LIKELY(m_it.primitive_iterator.is_begin())) - { - return m_object; - } - - throw_cannot_get_value(); - } + JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); + return &*m_it.array_iterator; } + + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); + return &(m_it.object_iterator->second); + } + + // neither array nor object: a primitive value + if (JSON_HEDLEY_LIKELY(m_it.primitive_iterator.is_begin())) + { + return m_object; + } + + throw_cannot_get_value(); } /*! @@ -391,35 +363,20 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - std::advance(m_it.object_iterator, 1); - break; - } - - case value_t::array: - { - std::advance(m_it.array_iterator, 1); - break; - } - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - ++m_it.primitive_iterator; - break; - } + ++m_it.array_iterator; + return *this; } + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + std::advance(m_it.object_iterator, 1); + return *this; + } + + // neither array nor object: a primitive value + ++m_it.primitive_iterator; return *this; } @@ -442,35 +399,20 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - std::advance(m_it.object_iterator, -1); - break; - } - - case value_t::array: - { - std::advance(m_it.array_iterator, -1); - break; - } - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - --m_it.primitive_iterator; - break; - } + --m_it.array_iterator; + return *this; } + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + std::advance(m_it.object_iterator, -1); + return *this; + } + + // neither array nor object: a primitive value + --m_it.primitive_iterator; return *this; } @@ -493,25 +435,18 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci return true; } - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - return (m_it.object_iterator == other.m_it.object_iterator); - - case value_t::array: - return (m_it.array_iterator == other.m_it.array_iterator); - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - return (m_it.primitive_iterator == other.m_it.primitive_iterator); + return (m_it.array_iterator == other.m_it.array_iterator); } + + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + return (m_it.object_iterator == other.m_it.object_iterator); + } + + // neither array nor object: a primitive value + return (m_it.primitive_iterator == other.m_it.primitive_iterator); } /*! diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index f51b0642b..5bc4d2014 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -734,6 +734,34 @@ private: } } + // true if no child of the non-empty array/object v has children of + // its own, i.e. v can be freed in one go (its destructor then + // destroys scalars and empty containers only, so it recurses at most + // one level deep); stops at the first structured child it meets + static bool all_children_leaves(const basic_json& v) noexcept + { + if (v.m_data.m_type == value_t::array) + { + const auto& a = *v.m_data.m_value.array; + for (auto it = a.rbegin(); it != a.rend(); ++it) + { + if (!has_no_children(*it)) + { + return false; + } + } + return true; + } + for (const auto& el : *v.m_data.m_value.object) + { + if (!has_no_children(el.second)) + { + return false; + } + } + return true; + } + // The walk in destroy_container() may take the children of a // container in any order, as long as it picks the same child again // while that container is not modified in between. Arrays and @@ -789,15 +817,16 @@ private: } } - // deallocates the (already empty) array/object held by v; this is - // the same allocator-based free the old recursive implementation - // used, just factored out so every level of the walk in destroy() - // can share it + // deallocates the array/object held by v, which is either empty or + // holds only scalars and empty containers (so its destructor + // recurses at most one level deep); this is the same allocator-based + // free the old recursive implementation used, just factored out so + // every level of the walk in destroy() can share it static void free_container(basic_json& v) noexcept { + JSON_ASSERT(has_no_children(v) || all_children_leaves(v)); if (v.m_data.m_type == value_t::array) { - JSON_ASSERT(v.m_data.m_value.array->empty()); AllocatorType alloc; std::allocator_traits::destroy(alloc, v.m_data.m_value.array); std::allocator_traits::deallocate(alloc, v.m_data.m_value.array, 1); @@ -805,7 +834,6 @@ private: else { JSON_ASSERT(v.m_data.m_type == value_t::object); - JSON_ASSERT(v.m_data.m_value.object->empty()); AllocatorType alloc; std::allocator_traits::destroy(alloc, v.m_data.m_value.object); std::allocator_traits::deallocate(alloc, v.m_data.m_value.object, 1); @@ -850,7 +878,58 @@ public: return; } - // Destroy the tree without recursing per nesting level and + // This json_value is not itself a basic_json, so the top-level + // container is first moved into a local stand-in ("cur"); a + // default-constructed basic_json has a null pointer in its + // m_value (see data::m_value's initializer), so swapping it + // with *this leaves this union's own pointer null, and it is + // never looked at or freed a second time. + basic_json cur; + cur.m_data.m_type = t; + using std::swap; + swap(cur.m_data.m_value, *this); + + destroy_bounded(cur, 0); + } + + // Destroy the array/object v, whose children are destroyed first. + // The first recursion_depth_limit() levels are destroyed by plain + // recursion, which is the fastest way and cannot throw; anything + // nested deeper is handed to destroy_walk(), which uses no call stack. + static void destroy_bounded(basic_json& v, std::size_t depth) noexcept + { + if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) + { + destroy_walk(v); + return; + } + + if (v.m_data.m_type == value_t::array) + { + for (auto& child : *v.m_data.m_value.array) + { + if (child.m_data.m_type == value_t::array || child.m_data.m_type == value_t::object) + { + destroy_bounded(child, depth + 1); + } + } + } + else + { + for (auto& child : *v.m_data.m_value.object) + { + if (child.second.m_data.m_type == value_t::array || child.second.m_data.m_type == value_t::object) + { + destroy_bounded(child.second, depth + 1); + } + } + } + + // only leaves and already destroyed (null) children remain + free_container(v); + } + + // Destroy the tree below top without recursing per nesting level and // without any heap allocation: a heap-allocated flattening // stack (the previous implementation) can itself throw // bad_alloc, which would escape this noexcept destructor and @@ -874,16 +953,24 @@ public: // pointer in its m_value (see data::m_value's initializer), // so swapping it with *this leaves this union's own pointer // null, and it is never looked at or freed a second time. + static void destroy_walk(basic_json& top) noexcept + { basic_json cur; - cur.m_data.m_type = t; - using std::swap; - swap(cur.m_data.m_value, *this); + take(cur, top); basic_json prev; // value_t::null: no parent + // true right after cur was entered: its children have not been + // looked at yet. If none of them has children of its own, cur is + // freed in one go instead of child by child; this is tried once + // per container, so the walk stays linear. + bool fresh = true; + while (true) { - if (has_no_children(cur)) + const bool fresh_now = fresh; + fresh = false; + if (has_no_children(cur) || (fresh_now && all_children_leaves(cur))) { if (prev.m_data.m_type == value_t::null) { @@ -922,6 +1009,7 @@ public: take(cur_child_ref, prev); take(prev, cur); take(cur, tmp); + fresh = true; } } @@ -7142,12 +7230,11 @@ public: std::size_t index = 0; /// objects: the next member of source to look at const_iterator member{}; // NOLINT(readability-redundant-member-init) - /// objects: the keys common to both, in source's order - std::vector common_keys{}; // NOLINT(readability-redundant-member-init) - /// objects: the next entry of common_keys - std::size_t next_common = 0; - /// objects: the "add" operations for keys only target has - basic_json added_ops{}; // NOLINT(readability-redundant-member-init) + /// objects: for each member of source (in order), its value in + /// target, or nullptr if target has no such key; `index` is the next + std::vector target_values{}; // NOLINT(readability-redundant-member-init) + /// objects: the members only target has, in target's order + std::vector added{}; // NOLINT(readability-redundant-member-init) }; // The operations of a diff are built by the functions below rather than @@ -7203,111 +7290,113 @@ public: } } + /// @brief appends the "add" operations for the members @a added to @a result + static void diff_added(basic_json& result, const std::vector& added, const string_t& path) + { + for (const auto& it : added) + { + diff_add(result, detail::concat(path, '/', detail::escape(it.key())), it.value()); + } + } + /*! @brief compare the keys of objects @a source and @a target If object_t does not keep its members in insertion order, or if the keys both objects have are in the same order in both, and the keys only - @a target has come after them, stores the keys common to both in - source's order in @a common_keys, stores the "add" operations for the keys - only @a target has in @a added_ops, and returns true: the caller then diffs - the objects member by member. Otherwise, appends operations that remove - every member of @a source and add every member of @a target to @a result, - and returns false. + @a target has come after them, stores in @a target_values, for each member + of source in order, its value in target (nullptr for a key target does not + have), stores the members only target has in @a added, and returns true: + the caller then diffs the objects member by member. Otherwise, appends + operations that remove every member of @a source and add every member of + @a target to @a result, and returns false. */ static bool diff_object_keys(basic_json& result, const basic_json& source, const basic_json& target, - const string_t& path, std::vector& common_keys, - basic_json& added_ops) + const string_t& path, std::vector& target_values, + std::vector& added) { - // 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 per-key diffs in the caller's fast path, to match - // source's original iteration order (as the original, - // pre-reordering-aware implementation did) instead of - // grouping all removes before all per-key diffs. - std::vector common_keys_source_order; + // first pass: one lookup per source key + target_values.reserve(source.size()); + std::size_t common = 0; for (auto it = source.cbegin(); it != source.cend(); ++it) { - if (target.find(it.key()) != target.end()) + const auto found = target.find(it.key()); + if (found != target.cend()) { - common_keys_source_order.push_back(it.key()); - } - } - - // 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, which only ever appends new keys - // 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. - // The patch ops for keys that were added (i.e., in target but not - // in source) are built here so the fast path 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 common_keys_target_order; - 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; - diff_add(added_ops, detail::concat(path, '/', detail::escape(it.key())), it.value()); + target_values.push_back(&found.value()); + ++common; } else { -#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) - { - 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 + target_values.push_back(nullptr); } } - // 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. + // second pass: find the keys only target has. An object type that + // keeps its members in insertion order, such as the one backing + // ordered_json, also needs the common keys to be in the same order + // in both objects, and the new keys to come after them, as patch() + // appends a new member at the end of such an object. Any other + // object type places its members itself, so a member-by-member diff + // always reproduces target there. #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 - || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)) + bool fast_path = true; + if (detail::is_ordered_map::value) { - // 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 diff is correct - // and minimal, as before - common_keys = std::move(common_keys_source_order); - return true; + // walk the common keys of source in lockstep with target: a key + // of target that is not the next one is either new or out of order + auto src = source.cbegin(); + std::size_t next = 0; + bool seen_new = false; + for (auto it = target.cbegin(); it != target.cend() && fast_path; ++it) + { + while (next < target_values.size() && target_values[next] == nullptr) + { + ++src; + ++next; + } + + if (next < target_values.size() && src.key() == it.key()) + { + // a common key after a new one: new keys are no suffix + fast_path = !seen_new; + ++src; + ++next; + } + else if (source.find(it.key()) == source.cend()) + { + seen_new = true; + added.push_back(it); + } + else + { + // a common key out of order + fast_path = false; + } + } + } + else if (target.size() != common) + { + for (auto it = target.cbegin(); it != target.cend(); ++it) + { + if (source.find(it.key()) == source.cend()) + { + added.push_back(it); + } + } } #ifdef JSON_HEDLEY_MSVC_VERSION #pragma warning( pop ) #endif + if (fast_path) + { + return true; + } + // 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 @@ -7357,6 +7446,14 @@ public: return; } + diff_unequal(result, source, target, path, depth); + } + + /// @brief @ref diff_recursively for values known to be unequal; callers + /// compare first so they build no path for equal ones + static void diff_unequal(basic_json& result, const basic_json& source, const basic_json& target, + const string_t& path, const std::size_t depth) + { if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) { diff_iteratively(result, source, target, path); @@ -7379,7 +7476,10 @@ public: while (i < source.size() && i < target.size()) { // recursive call to compare array values at index i - diff_recursively(result, source[i], target[i], detail::concat(path, '/', detail::to_string(i)), depth + 1); + if (!(source[i] == target[i])) + { + diff_unequal(result, source[i], target[i], detail::concat(path, '/', detail::to_string(i)), depth + 1); + } ++i; } @@ -7391,25 +7491,24 @@ public: case value_t::object: { - std::vector common_keys; - basic_json added_ops(value_t::array); - if (diff_object_keys(result, source, target, path, common_keys, added_ops)) + std::vector target_values; + std::vector added; + if (diff_object_keys(result, source, target, path, target_values, added)) { - // fast path: common_keys 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) are interleaved - // here too, in source's original order, to match the + // fast path: target_values holds, for every member of + // source in order, its value in target -- so no further + // lookup is needed. Deleted keys are interleaved here + // too, in source's original order, to match the // historical (pre-reordering-aware) output order. - auto common_it = common_keys.cbegin(); - for (auto it = source.cbegin(); it != source.cend(); ++it) + std::size_t i = 0; + for (auto it = source.cbegin(); it != source.cend(); ++it, ++i) { - if (common_it != common_keys.cend() && it.key() == *common_it) + if (target_values[i] != nullptr) { - diff_recursively(result, it.value(), target[it.key()], detail::concat(path, '/', detail::escape(it.key())), depth + 1); - ++common_it; + if (!(it.value() == *target_values[i])) + { + diff_unequal(result, it.value(), *target_values[i], detail::concat(path, '/', detail::escape(it.key())), depth + 1); + } } else { @@ -7418,10 +7517,8 @@ public: } } - // append the "add" ops for brand-new keys collected by - // diff_object_keys -- no second source.find() per target - // key needed - result.insert(result.end(), added_ops.begin(), added_ops.end()); + // append the "add" operations for brand-new keys + diff_added(result, added, path); } break; } @@ -7494,17 +7591,17 @@ public: case value_t::object: { - std::vector common_keys; - basic_json added_ops(value_t::array); - if (diff_object_keys(result, s, t, current_path, common_keys, added_ops)) + std::vector target_values; + std::vector added; + if (diff_object_keys(result, s, t, current_path, target_values, added)) { // fast path: the frame walks source in lockstep with // common_keys, as diff_recursively does, and appends // added_ops once all members are done stack.emplace_back(&s, &t, current_path.size()); stack.back().member = s.cbegin(); - stack.back().common_keys = std::move(common_keys); - stack.back().added_ops = std::move(added_ops); + stack.back().target_values = std::move(target_values); + stack.back().added = std::move(added); } return; } @@ -7566,11 +7663,10 @@ public: if (it != s->cend()) { ++stack.back().member; - const std::size_t next_common = stack.back().next_common; - if (next_common < stack.back().common_keys.size() && it.key() == stack.back().common_keys[next_common]) + const basic_json* const target_value_ptr = stack.back().target_values[stack.back().index++]; + if (target_value_ptr != nullptr) { - ++stack.back().next_common; - const basic_json& target_value = (*t)[it.key()]; + const basic_json& target_value = *target_value_ptr; detail::concat_into(current_path, '/', detail::escape(it.key())); enter(it.value(), target_value); if (stack.size() == depth) @@ -7588,7 +7684,7 @@ public: // append the "add" ops for brand-new keys collected when the // object was entered - result.insert(result.end(), stack.back().added_ops.begin(), stack.back().added_ops.end()); + diff_added(result, stack.back().added, current_path); } // this array or object is done: continue with the one it is in diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 2b0a0f705..79a3369e3 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -20147,40 +20147,25 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); - return m_it.object_iterator->second; - } - - case value_t::array: - { - JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); - return *m_it.array_iterator; - } - - case value_t::null: - throw_cannot_get_value(); - - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - if (JSON_HEDLEY_LIKELY(m_it.primitive_iterator.is_begin())) - { - return *m_object; - } - - throw_cannot_get_value(); - } + JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); + return *m_it.array_iterator; } + + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); + return m_it.object_iterator->second; + } + + // neither array nor object: a primitive value (null has no value to point to) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type != value_t::null && m_it.primitive_iterator.is_begin())) + { + return *m_object; + } + + throw_cannot_get_value(); } /*! @@ -20191,38 +20176,25 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); - return &(m_it.object_iterator->second); - } - - case value_t::array: - { - JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); - return &*m_it.array_iterator; - } - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - if (JSON_HEDLEY_LIKELY(m_it.primitive_iterator.is_begin())) - { - return m_object; - } - - throw_cannot_get_value(); - } + JSON_ASSERT(m_it.array_iterator != m_object->m_data.m_value.array->end()); + return &*m_it.array_iterator; } + + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + JSON_ASSERT(m_it.object_iterator != m_object->m_data.m_value.object->end()); + return &(m_it.object_iterator->second); + } + + // neither array nor object: a primitive value + if (JSON_HEDLEY_LIKELY(m_it.primitive_iterator.is_begin())) + { + return m_object; + } + + throw_cannot_get_value(); } /*! @@ -20244,35 +20216,20 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - std::advance(m_it.object_iterator, 1); - break; - } - - case value_t::array: - { - std::advance(m_it.array_iterator, 1); - break; - } - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - ++m_it.primitive_iterator; - break; - } + ++m_it.array_iterator; + return *this; } + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + std::advance(m_it.object_iterator, 1); + return *this; + } + + // neither array nor object: a primitive value + ++m_it.primitive_iterator; return *this; } @@ -20295,35 +20252,20 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci { JSON_ASSERT(m_object != nullptr); - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - { - std::advance(m_it.object_iterator, -1); - break; - } - - case value_t::array: - { - std::advance(m_it.array_iterator, -1); - break; - } - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - { - --m_it.primitive_iterator; - break; - } + --m_it.array_iterator; + return *this; } + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + std::advance(m_it.object_iterator, -1); + return *this; + } + + // neither array nor object: a primitive value + --m_it.primitive_iterator; return *this; } @@ -20346,25 +20288,18 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci return true; } - switch (m_object->m_data.m_type) + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::array)) { - case value_t::object: - return (m_it.object_iterator == other.m_it.object_iterator); - - case value_t::array: - return (m_it.array_iterator == other.m_it.array_iterator); - - case value_t::null: - case value_t::string: - case value_t::boolean: - case value_t::number_integer: - case value_t::number_unsigned: - case value_t::number_float: - case value_t::binary: - case value_t::discarded: - default: - return (m_it.primitive_iterator == other.m_it.primitive_iterator); + return (m_it.array_iterator == other.m_it.array_iterator); } + + if (JSON_HEDLEY_LIKELY(m_object->m_data.m_type == value_t::object)) + { + return (m_it.object_iterator == other.m_it.object_iterator); + } + + // neither array nor object: a primitive value + return (m_it.primitive_iterator == other.m_it.primitive_iterator); } /*! @@ -30564,6 +30499,34 @@ private: } } + // true if no child of the non-empty array/object v has children of + // its own, i.e. v can be freed in one go (its destructor then + // destroys scalars and empty containers only, so it recurses at most + // one level deep); stops at the first structured child it meets + static bool all_children_leaves(const basic_json& v) noexcept + { + if (v.m_data.m_type == value_t::array) + { + const auto& a = *v.m_data.m_value.array; + for (auto it = a.rbegin(); it != a.rend(); ++it) + { + if (!has_no_children(*it)) + { + return false; + } + } + return true; + } + for (const auto& el : *v.m_data.m_value.object) + { + if (!has_no_children(el.second)) + { + return false; + } + } + return true; + } + // The walk in destroy_container() may take the children of a // container in any order, as long as it picks the same child again // while that container is not modified in between. Arrays and @@ -30619,15 +30582,16 @@ private: } } - // deallocates the (already empty) array/object held by v; this is - // the same allocator-based free the old recursive implementation - // used, just factored out so every level of the walk in destroy() - // can share it + // deallocates the array/object held by v, which is either empty or + // holds only scalars and empty containers (so its destructor + // recurses at most one level deep); this is the same allocator-based + // free the old recursive implementation used, just factored out so + // every level of the walk in destroy() can share it static void free_container(basic_json& v) noexcept { + JSON_ASSERT(has_no_children(v) || all_children_leaves(v)); if (v.m_data.m_type == value_t::array) { - JSON_ASSERT(v.m_data.m_value.array->empty()); AllocatorType alloc; std::allocator_traits::destroy(alloc, v.m_data.m_value.array); std::allocator_traits::deallocate(alloc, v.m_data.m_value.array, 1); @@ -30635,7 +30599,6 @@ private: else { JSON_ASSERT(v.m_data.m_type == value_t::object); - JSON_ASSERT(v.m_data.m_value.object->empty()); AllocatorType alloc; std::allocator_traits::destroy(alloc, v.m_data.m_value.object); std::allocator_traits::deallocate(alloc, v.m_data.m_value.object, 1); @@ -30680,40 +30643,99 @@ public: return; } - // Destroy the tree without recursing per nesting level and - // without any heap allocation: a heap-allocated flattening - // stack (the previous implementation) can itself throw - // bad_alloc, which would escape this noexcept destructor and - // terminate the program (#5135). - // - // Instead, walk down a chain of children (always the one - // walk_child() picks), reversing links as we go: cur is the - // container currently being emptied, and prev is its parent - // (value_t::null when there is none). Each parent's - // walk_child() slot doubles as storage for that parent's own - // parent link while we are below it, so no extra memory is - // needed; the parent is not modified meanwhile, so - // walk_child() finds that same slot again on the way up. We - // only ever remove a child once it is a scalar or an empty - // array/object, which neither allocates nor recurses more - // than one level deep. - // - // This json_value is not itself a basic_json, so the - // top-level container is first moved into a local stand-in - // ("cur"); a default-constructed basic_json has a null - // pointer in its m_value (see data::m_value's initializer), - // so swapping it with *this leaves this union's own pointer - // null, and it is never looked at or freed a second time. + // This json_value is not itself a basic_json, so the top-level + // container is first moved into a local stand-in ("cur"); a + // default-constructed basic_json has a null pointer in its + // m_value (see data::m_value's initializer), so swapping it + // with *this leaves this union's own pointer null, and it is + // never looked at or freed a second time. basic_json cur; cur.m_data.m_type = t; using std::swap; swap(cur.m_data.m_value, *this); + destroy_bounded(cur, 0); + } + + // Destroy the array/object v, whose children are destroyed first. + // The first recursion_depth_limit() levels are destroyed by plain + // recursion, which is the fastest way and cannot throw; anything + // nested deeper is handed to destroy_walk(), which uses no call stack. + static void destroy_bounded(basic_json& v, std::size_t depth) noexcept + { + if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) + { + destroy_walk(v); + return; + } + + if (v.m_data.m_type == value_t::array) + { + for (auto& child : *v.m_data.m_value.array) + { + if (child.m_data.m_type == value_t::array || child.m_data.m_type == value_t::object) + { + destroy_bounded(child, depth + 1); + } + } + } + else + { + for (auto& child : *v.m_data.m_value.object) + { + if (child.second.m_data.m_type == value_t::array || child.second.m_data.m_type == value_t::object) + { + destroy_bounded(child.second, depth + 1); + } + } + } + + // only leaves and already destroyed (null) children remain + free_container(v); + } + + // Destroy the tree below top without recursing per nesting level and + // without any heap allocation: a heap-allocated flattening + // stack (the previous implementation) can itself throw + // bad_alloc, which would escape this noexcept destructor and + // terminate the program (#5135). + // + // Instead, walk down a chain of children (always the one + // walk_child() picks), reversing links as we go: cur is the + // container currently being emptied, and prev is its parent + // (value_t::null when there is none). Each parent's + // walk_child() slot doubles as storage for that parent's own + // parent link while we are below it, so no extra memory is + // needed; the parent is not modified meanwhile, so + // walk_child() finds that same slot again on the way up. We + // only ever remove a child once it is a scalar or an empty + // array/object, which neither allocates nor recurses more + // than one level deep. + // + // This json_value is not itself a basic_json, so the + // top-level container is first moved into a local stand-in + // ("cur"); a default-constructed basic_json has a null + // pointer in its m_value (see data::m_value's initializer), + // so swapping it with *this leaves this union's own pointer + // null, and it is never looked at or freed a second time. + static void destroy_walk(basic_json& top) noexcept + { + basic_json cur; + take(cur, top); + basic_json prev; // value_t::null: no parent + // true right after cur was entered: its children have not been + // looked at yet. If none of them has children of its own, cur is + // freed in one go instead of child by child; this is tried once + // per container, so the walk stays linear. + bool fresh = true; + while (true) { - if (has_no_children(cur)) + const bool fresh_now = fresh; + fresh = false; + if (has_no_children(cur) || (fresh_now && all_children_leaves(cur))) { if (prev.m_data.m_type == value_t::null) { @@ -30752,6 +30774,7 @@ public: take(cur_child_ref, prev); take(prev, cur); take(cur, tmp); + fresh = true; } } @@ -36972,12 +36995,11 @@ public: std::size_t index = 0; /// objects: the next member of source to look at const_iterator member{}; // NOLINT(readability-redundant-member-init) - /// objects: the keys common to both, in source's order - std::vector common_keys{}; // NOLINT(readability-redundant-member-init) - /// objects: the next entry of common_keys - std::size_t next_common = 0; - /// objects: the "add" operations for keys only target has - basic_json added_ops{}; // NOLINT(readability-redundant-member-init) + /// objects: for each member of source (in order), its value in + /// target, or nullptr if target has no such key; `index` is the next + std::vector target_values{}; // NOLINT(readability-redundant-member-init) + /// objects: the members only target has, in target's order + std::vector added{}; // NOLINT(readability-redundant-member-init) }; // The operations of a diff are built by the functions below rather than @@ -37033,111 +37055,113 @@ public: } } + /// @brief appends the "add" operations for the members @a added to @a result + static void diff_added(basic_json& result, const std::vector& added, const string_t& path) + { + for (const auto& it : added) + { + diff_add(result, detail::concat(path, '/', detail::escape(it.key())), it.value()); + } + } + /*! @brief compare the keys of objects @a source and @a target If object_t does not keep its members in insertion order, or if the keys both objects have are in the same order in both, and the keys only - @a target has come after them, stores the keys common to both in - source's order in @a common_keys, stores the "add" operations for the keys - only @a target has in @a added_ops, and returns true: the caller then diffs - the objects member by member. Otherwise, appends operations that remove - every member of @a source and add every member of @a target to @a result, - and returns false. + @a target has come after them, stores in @a target_values, for each member + of source in order, its value in target (nullptr for a key target does not + have), stores the members only target has in @a added, and returns true: + the caller then diffs the objects member by member. Otherwise, appends + operations that remove every member of @a source and add every member of + @a target to @a result, and returns false. */ static bool diff_object_keys(basic_json& result, const basic_json& source, const basic_json& target, - const string_t& path, std::vector& common_keys, - basic_json& added_ops) + const string_t& path, std::vector& target_values, + std::vector& added) { - // 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 per-key diffs in the caller's fast path, to match - // source's original iteration order (as the original, - // pre-reordering-aware implementation did) instead of - // grouping all removes before all per-key diffs. - std::vector common_keys_source_order; + // first pass: one lookup per source key + target_values.reserve(source.size()); + std::size_t common = 0; for (auto it = source.cbegin(); it != source.cend(); ++it) { - if (target.find(it.key()) != target.end()) + const auto found = target.find(it.key()); + if (found != target.cend()) { - common_keys_source_order.push_back(it.key()); - } - } - - // 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, which only ever appends new keys - // 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. - // The patch ops for keys that were added (i.e., in target but not - // in source) are built here so the fast path 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 common_keys_target_order; - 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; - diff_add(added_ops, detail::concat(path, '/', detail::escape(it.key())), it.value()); + target_values.push_back(&found.value()); + ++common; } else { -#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) - { - 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 + target_values.push_back(nullptr); } } - // 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. + // second pass: find the keys only target has. An object type that + // keeps its members in insertion order, such as the one backing + // ordered_json, also needs the common keys to be in the same order + // in both objects, and the new keys to come after them, as patch() + // appends a new member at the end of such an object. Any other + // object type places its members itself, so a member-by-member diff + // always reproduces target there. #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 - || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)) + bool fast_path = true; + if (detail::is_ordered_map::value) { - // 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 diff is correct - // and minimal, as before - common_keys = std::move(common_keys_source_order); - return true; + // walk the common keys of source in lockstep with target: a key + // of target that is not the next one is either new or out of order + auto src = source.cbegin(); + std::size_t next = 0; + bool seen_new = false; + for (auto it = target.cbegin(); it != target.cend() && fast_path; ++it) + { + while (next < target_values.size() && target_values[next] == nullptr) + { + ++src; + ++next; + } + + if (next < target_values.size() && src.key() == it.key()) + { + // a common key after a new one: new keys are no suffix + fast_path = !seen_new; + ++src; + ++next; + } + else if (source.find(it.key()) == source.cend()) + { + seen_new = true; + added.push_back(it); + } + else + { + // a common key out of order + fast_path = false; + } + } + } + else if (target.size() != common) + { + for (auto it = target.cbegin(); it != target.cend(); ++it) + { + if (source.find(it.key()) == source.cend()) + { + added.push_back(it); + } + } } #ifdef JSON_HEDLEY_MSVC_VERSION #pragma warning( pop ) #endif + if (fast_path) + { + return true; + } + // 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 @@ -37187,6 +37211,14 @@ public: return; } + diff_unequal(result, source, target, path, depth); + } + + /// @brief @ref diff_recursively for values known to be unequal; callers + /// compare first so they build no path for equal ones + static void diff_unequal(basic_json& result, const basic_json& source, const basic_json& target, + const string_t& path, const std::size_t depth) + { if (JSON_HEDLEY_UNLIKELY(depth >= detail::recursion_depth_limit())) { diff_iteratively(result, source, target, path); @@ -37209,7 +37241,10 @@ public: while (i < source.size() && i < target.size()) { // recursive call to compare array values at index i - diff_recursively(result, source[i], target[i], detail::concat(path, '/', detail::to_string(i)), depth + 1); + if (!(source[i] == target[i])) + { + diff_unequal(result, source[i], target[i], detail::concat(path, '/', detail::to_string(i)), depth + 1); + } ++i; } @@ -37221,25 +37256,24 @@ public: case value_t::object: { - std::vector common_keys; - basic_json added_ops(value_t::array); - if (diff_object_keys(result, source, target, path, common_keys, added_ops)) + std::vector target_values; + std::vector added; + if (diff_object_keys(result, source, target, path, target_values, added)) { - // fast path: common_keys 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) are interleaved - // here too, in source's original order, to match the + // fast path: target_values holds, for every member of + // source in order, its value in target -- so no further + // lookup is needed. Deleted keys are interleaved here + // too, in source's original order, to match the // historical (pre-reordering-aware) output order. - auto common_it = common_keys.cbegin(); - for (auto it = source.cbegin(); it != source.cend(); ++it) + std::size_t i = 0; + for (auto it = source.cbegin(); it != source.cend(); ++it, ++i) { - if (common_it != common_keys.cend() && it.key() == *common_it) + if (target_values[i] != nullptr) { - diff_recursively(result, it.value(), target[it.key()], detail::concat(path, '/', detail::escape(it.key())), depth + 1); - ++common_it; + if (!(it.value() == *target_values[i])) + { + diff_unequal(result, it.value(), *target_values[i], detail::concat(path, '/', detail::escape(it.key())), depth + 1); + } } else { @@ -37248,10 +37282,8 @@ public: } } - // append the "add" ops for brand-new keys collected by - // diff_object_keys -- no second source.find() per target - // key needed - result.insert(result.end(), added_ops.begin(), added_ops.end()); + // append the "add" operations for brand-new keys + diff_added(result, added, path); } break; } @@ -37324,17 +37356,17 @@ public: case value_t::object: { - std::vector common_keys; - basic_json added_ops(value_t::array); - if (diff_object_keys(result, s, t, current_path, common_keys, added_ops)) + std::vector target_values; + std::vector added; + if (diff_object_keys(result, s, t, current_path, target_values, added)) { // fast path: the frame walks source in lockstep with // common_keys, as diff_recursively does, and appends // added_ops once all members are done stack.emplace_back(&s, &t, current_path.size()); stack.back().member = s.cbegin(); - stack.back().common_keys = std::move(common_keys); - stack.back().added_ops = std::move(added_ops); + stack.back().target_values = std::move(target_values); + stack.back().added = std::move(added); } return; } @@ -37396,11 +37428,10 @@ public: if (it != s->cend()) { ++stack.back().member; - const std::size_t next_common = stack.back().next_common; - if (next_common < stack.back().common_keys.size() && it.key() == stack.back().common_keys[next_common]) + const basic_json* const target_value_ptr = stack.back().target_values[stack.back().index++]; + if (target_value_ptr != nullptr) { - ++stack.back().next_common; - const basic_json& target_value = (*t)[it.key()]; + const basic_json& target_value = *target_value_ptr; detail::concat_into(current_path, '/', detail::escape(it.key())); enter(it.value(), target_value); if (stack.size() == depth) @@ -37418,7 +37449,7 @@ public: // append the "add" ops for brand-new keys collected when the // object was entered - result.insert(result.end(), stack.back().added_ops.begin(), stack.back().added_ops.end()); + diff_added(result, stack.back().added, current_path); } // this array or object is done: continue with the one it is in diff --git a/tests/src/unit-constructor2.cpp b/tests/src/unit-constructor2.cpp index 1334d6245..4896bb1f0 100644 --- a/tests/src/unit-constructor2.cpp +++ b/tests/src/unit-constructor2.cpp @@ -323,3 +323,57 @@ TEST_CASE("copy constructor and destructor across value types") CHECK(w.size() == 9); } } + +TEST_CASE_TEMPLATE("destructor of nested containers", Json, nlohmann::json, nlohmann::ordered_json) +{ + SECTION("empty containers") + { + { + Json j = Json::array(); + } + { + Json j = Json::object(); + } + { + Json j = Json::array({Json::array(), Json::object(), Json::array({Json::array()})}); + } + } + + SECTION("leaves mixed with nested containers") + { + Json j = Json::object(); + j["leaf"] = 1; + j["str"] = "text"; + j["empty_a"] = Json::array(); + j["empty_o"] = Json::object(); + j["arr"] = Json::array({1, "a", Json::array({1, 2}), Json::object({{"k", Json::array({Json::object()})}}), nullptr}); + j["obj"] = Json::object({{"x", Json::object({{"y", Json::array({1, Json::object({{"z", 1}})})}})}, {"w", 2}}); + Json copy = j; + CHECK(copy == j); + j = Json::array({std::move(copy), j, 3, Json::array()}); + } + + SECTION("deep nesting") + { + for (int kind = 0; kind < 3; ++kind) + { + Json j = 1; + for (int i = 0; i < 5000; ++i) + { + switch (kind) + { + case 0: + j = Json::array({std::move(j)}); + break; + case 1: + j = Json::object({{"k", std::move(j)}}); + break; + default: + // siblings next to the nested child + j = (i % 2 == 0) ? Json::array({1, Json::array(), std::move(j)}) : Json::object({{"a", 1}, {"b", Json::object()}, {"c", std::move(j)}}); + break; + } + } + } + } +} diff --git a/tests/src/unit-iterators1.cpp b/tests/src/unit-iterators1.cpp index b7b0aa9eb..629c6abd7 100644 --- a/tests/src/unit-iterators1.cpp +++ b/tests/src/unit-iterators1.cpp @@ -10,6 +10,8 @@ #define JSON_TESTS_PRIVATE #include +#include +#include using nlohmann::json; TEST_CASE("iterators 1") @@ -1628,3 +1630,108 @@ TEST_CASE("iterators 1") } } } + +TEST_CASE_TEMPLATE("reverse iteration and member access", Json, nlohmann::json, nlohmann::ordered_json) +{ + SECTION("array: -- and rbegin/rend") + { + Json j = {1, 2, 3, 4}; + + auto it = j.end(); + std::vector seen; + while (it != j.begin()) + { + --it; + seen.push_back(it->template get()); + } + CHECK(seen == std::vector {4, 3, 2, 1}); + + auto it2 = j.end(); + auto old = it2--; + CHECK(old == j.end()); + CHECK(*it2 == 4); + + seen.clear(); + for (auto r = j.rbegin(); r != j.rend(); ++r) + { + seen.push_back(r->template get()); + } + CHECK(seen == std::vector {4, 3, 2, 1}); + + seen.clear(); + for (auto r = j.crbegin(); r != j.crend(); ++r) + { + seen.push_back(*r); + } + CHECK(seen == std::vector {4, 3, 2, 1}); + } + + SECTION("object: -- and rbegin/rend") + { + Json j = {{"a", 1}, {"b", 2}, {"c", 3}}; + // order of keys depends on the object type; use the forward order as reference + std::vector fwd; + for (auto it = j.begin(); it != j.end(); ++it) + { + fwd.push_back(it.key()); + } + std::vector rev(fwd.rbegin(), fwd.rend()); + + std::vector seen; + auto it = j.end(); + while (it != j.begin()) + { + --it; + seen.push_back(it.key()); + } + CHECK(seen == rev); + + auto it2 = j.end(); + auto old = it2--; + CHECK(old == j.end()); + CHECK(it2.key() == rev.front()); + + seen.clear(); + for (auto r = j.rbegin(); r != j.rend(); ++r) + { + seen.push_back(r.key()); + } + CHECK(seen == rev); + } + + SECTION("object: operator-> reads and writes the mapped value") + { + Json j = {{"a", 1}, {"s", "str"}}; + auto it = j.find("s"); + REQUIRE(it != j.end()); + CHECK(it->is_string()); + CHECK(it->size() == 1); + *it = Json::array({1, 2}); + CHECK(it->is_array()); + it->push_back(3); + CHECK(j["s"].size() == 3); + + const Json& cj = j; + auto cit = cj.find("a"); + REQUIRE(cit != cj.end()); + CHECK(cit->is_number_integer()); + CHECK(cit->template get() == 1); + } + + SECTION("primitives: -- and ->") + { + Json j = 42; + auto it = j.end(); + --it; + CHECK(it == j.begin()); + CHECK(it->template get() == 42); + } + + SECTION("comparing iterators of different containers throws") + { + Json a = {1, 2}; + Json b = {1, 2}; + CHECK_THROWS_WITH_AS(a.begin() == b.begin(), "[json.exception.invalid_iterator.212] cannot compare iterators of different containers", nlohmann::json::invalid_iterator); + CHECK_THROWS_WITH_AS(a.begin() < b.begin(), "[json.exception.invalid_iterator.212] cannot compare iterators of different containers", nlohmann::json::invalid_iterator); + } +} diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 9e83edbcf..67ffee6dd 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -2066,3 +2066,40 @@ TEST_CASE("JSON patch - every operation on ordered_json") } } } + +TEST_CASE_TEMPLATE("JSON patch - diff round trip on hand-made documents", Json, nlohmann::json, nlohmann::ordered_json) +{ + const auto roundtrip = [](const Json & a, const Json & b) + { + CAPTURE(a.dump()) + CAPTURE(b.dump()) + const Json p = Json::diff(a, b); + CHECK(a.patch(p) == b); + CHECK(Json::diff(b, b).empty()); + }; + + SECTION("reordered keys") + { + const Json a = Json::parse(R"({"x":1,"y":2,"z":{"p":1,"q":2}})"); + const Json b = Json::parse(R"({"z":{"q":2,"p":1},"y":2,"x":1})"); + roundtrip(a, b); + roundtrip(b, a); + } + + SECTION("nested add, remove and replace") + { + const Json a = Json::parse(R"({"a":[1,2,{"k":"v","gone":true}],"b":{"c":{"d":1}},"e":"s"})"); + const Json b = Json::parse(R"({"a":[1,3,{"k":"w","new":[]}],"b":{"c":{"d":[1,2]}},"f":null})"); + roundtrip(a, b); + roundtrip(b, a); + } + + SECTION("arrays of different length and type changes") + { + roundtrip(Json::parse("[1,2,3,4,5]"), Json::parse("[1,3]")); + roundtrip(Json::parse("[1]"), Json::parse("[1,[2],{\"a\":3}]")); + roundtrip(Json::parse("[]"), Json::parse("{}")); + roundtrip(Json::parse("{\"a\":[]}"), Json::parse("{\"a\":{}}")); + roundtrip(Json(1), Json::parse("[1]")); + } +}