diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index aa8b1a6c6..f698163fe 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -638,21 +638,49 @@ private: } } - static basic_json& last_child(basic_json& v) + // 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 + // objects with bidirectional iterators (std::map, ordered_map, ...) + // use their last child, which a vector-based container can remove + // in O(1). ObjectType only needs forward iterators, though (e.g. + // std::unordered_map), so other objects use their first child. + template + static typename ObjectType_::iterator walk_child_it(ObjectType_& o, std::bidirectional_iterator_tag /*unused*/) + { + return std::prev(o.end()); + } + + template + static typename ObjectType_::iterator walk_child_it(ObjectType_& o, std::forward_iterator_tag /*unused*/) + { + return o.begin(); + } + + template + static typename ObjectType_::iterator walk_child_it(ObjectType_& o) + { + JSON_ASSERT(!o.empty()); + return walk_child_it(o, typename std::iterator_traits::iterator_category()); + } + + // the child of a non-empty array/object v that the walk in + // destroy_container() continues with (see walk_child_it() above) + static basic_json& walk_child(basic_json& v) { if (v.m_data.m_type == value_t::array) { return v.m_data.m_value.array->back(); } JSON_ASSERT(v.m_data.m_type == value_t::object); - return v.m_data.m_value.object->rbegin()->second; + return walk_child_it(*v.m_data.m_value.object)->second; } - // removes the last child of a non-empty array/object v; this never + // removes walk_child(v) from a non-empty array/object v; this never // allocates, and since it is only ever called when that child is a // scalar or an already-empty array/object, destroying it never // recurses more than one level deep (see destroy() below) - static void pop_last_child(basic_json& v) + static void pop_walk_child(basic_json& v) { if (v.m_data.m_type == value_t::array) { @@ -661,9 +689,7 @@ private: else { JSON_ASSERT(v.m_data.m_type == value_t::object); - // erase() needs a forward iterator, so std::prev(end()) is - // used here rather than rbegin() (see last_child() above) - v.m_data.m_value.object->erase(std::prev(v.m_data.m_value.object->end())); + v.m_data.m_value.object->erase(walk_child_it(*v.m_data.m_value.object)); } } @@ -734,14 +760,17 @@ public: // bad_alloc, which would escape this noexcept destructor and // terminate the program (#5135). // - // Instead, walk down the "last child" chain, 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 last child slot doubles as storage for that - // parent's own parent link while we are below it, so no - // extra memory is needed. 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. + // 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 @@ -767,11 +796,11 @@ public: } // ascend: detach the grandparent link from prev's - // last slot, drop that (now null) slot, free cur + // walk_child() slot, drop that (now null) slot, free cur // (it is empty), then move up one level basic_json gp; - take(gp, last_child(prev)); - pop_last_child(prev); + take(gp, walk_child(prev)); + pop_walk_child(prev); free_container(cur); @@ -780,21 +809,21 @@ public: continue; } - basic_json& cur_last_ref = last_child(cur); + basic_json& cur_child_ref = walk_child(cur); - if (has_no_children(cur_last_ref)) + if (has_no_children(cur_child_ref)) { // scalar, or already-empty array/object - pop_last_child(cur); + pop_walk_child(cur); continue; } - // descend into the non-empty last child, reversing the + // descend into the non-empty child, reversing the // link: its slot takes over prev, and the child becomes // the new cur basic_json tmp; - take(tmp, cur_last_ref); - take(cur_last_ref, prev); + take(tmp, cur_child_ref); + take(cur_child_ref, prev); take(prev, cur); take(cur, tmp); } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 00e345dd3..b6e5ca7c5 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -27877,21 +27877,49 @@ private: } } - static basic_json& last_child(basic_json& v) + // 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 + // objects with bidirectional iterators (std::map, ordered_map, ...) + // use their last child, which a vector-based container can remove + // in O(1). ObjectType only needs forward iterators, though (e.g. + // std::unordered_map), so other objects use their first child. + template + static typename ObjectType_::iterator walk_child_it(ObjectType_& o, std::bidirectional_iterator_tag /*unused*/) + { + return std::prev(o.end()); + } + + template + static typename ObjectType_::iterator walk_child_it(ObjectType_& o, std::forward_iterator_tag /*unused*/) + { + return o.begin(); + } + + template + static typename ObjectType_::iterator walk_child_it(ObjectType_& o) + { + JSON_ASSERT(!o.empty()); + return walk_child_it(o, typename std::iterator_traits::iterator_category()); + } + + // the child of a non-empty array/object v that the walk in + // destroy_container() continues with (see walk_child_it() above) + static basic_json& walk_child(basic_json& v) { if (v.m_data.m_type == value_t::array) { return v.m_data.m_value.array->back(); } JSON_ASSERT(v.m_data.m_type == value_t::object); - return v.m_data.m_value.object->rbegin()->second; + return walk_child_it(*v.m_data.m_value.object)->second; } - // removes the last child of a non-empty array/object v; this never + // removes walk_child(v) from a non-empty array/object v; this never // allocates, and since it is only ever called when that child is a // scalar or an already-empty array/object, destroying it never // recurses more than one level deep (see destroy() below) - static void pop_last_child(basic_json& v) + static void pop_walk_child(basic_json& v) { if (v.m_data.m_type == value_t::array) { @@ -27900,9 +27928,7 @@ private: else { JSON_ASSERT(v.m_data.m_type == value_t::object); - // erase() needs a forward iterator, so std::prev(end()) is - // used here rather than rbegin() (see last_child() above) - v.m_data.m_value.object->erase(std::prev(v.m_data.m_value.object->end())); + v.m_data.m_value.object->erase(walk_child_it(*v.m_data.m_value.object)); } } @@ -27973,14 +27999,17 @@ public: // bad_alloc, which would escape this noexcept destructor and // terminate the program (#5135). // - // Instead, walk down the "last child" chain, 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 last child slot doubles as storage for that - // parent's own parent link while we are below it, so no - // extra memory is needed. 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. + // 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 @@ -28006,11 +28035,11 @@ public: } // ascend: detach the grandparent link from prev's - // last slot, drop that (now null) slot, free cur + // walk_child() slot, drop that (now null) slot, free cur // (it is empty), then move up one level basic_json gp; - take(gp, last_child(prev)); - pop_last_child(prev); + take(gp, walk_child(prev)); + pop_walk_child(prev); free_container(cur); @@ -28019,21 +28048,21 @@ public: continue; } - basic_json& cur_last_ref = last_child(cur); + basic_json& cur_child_ref = walk_child(cur); - if (has_no_children(cur_last_ref)) + if (has_no_children(cur_child_ref)) { // scalar, or already-empty array/object - pop_last_child(cur); + pop_walk_child(cur); continue; } - // descend into the non-empty last child, reversing the + // descend into the non-empty child, reversing the // link: its slot takes over prev, and the child becomes // the new cur basic_json tmp; - take(tmp, cur_last_ref); - take(cur_last_ref, prev); + take(tmp, cur_child_ref); + take(cur_child_ref, prev); take(prev, cur); take(cur, tmp); } diff --git a/tests/src/unit-custom-object-type.cpp b/tests/src/unit-custom-object-type.cpp index a579e0b2d..0eaaf2d3c 100644 --- a/tests/src/unit-custom-object-type.cpp +++ b/tests/src/unit-custom-object-type.cpp @@ -10,7 +10,9 @@ #include +#include #include +#include #include #include #include @@ -196,6 +198,198 @@ struct void_erase_map : std::map using void_erase_json = nlohmann::basic_json; +// wraps an iterator, but only offers the LegacyForwardIterator operations, +// like the iterators of std::unordered_map and other hash maps +template +class forward_only_iterator +{ + BaseIterator m_it{}; + + public: + using iterator_category = std::forward_iterator_tag; + using value_type = typename std::iterator_traits::value_type; + using difference_type = typename std::iterator_traits::difference_type; + using pointer = typename std::iterator_traits::pointer; + using reference = typename std::iterator_traits::reference; + + forward_only_iterator() = default; + explicit forward_only_iterator(BaseIterator it) : m_it(it) {} + + BaseIterator base() const + { + return m_it; + } + + reference operator*() const + { + return *m_it; + } + pointer operator->() const + { + return &*m_it; + } + forward_only_iterator& operator++() + { + ++m_it; + return *this; + } + forward_only_iterator operator++(int) + { + auto result = *this; + ++m_it; + return result; + } + + friend bool operator==(const forward_only_iterator& lhs, const forward_only_iterator& rhs) + { + return lhs.m_it == rhs.m_it; + } + friend bool operator!=(const forward_only_iterator& lhs, const forward_only_iterator& rhs) + { + return lhs.m_it != rhs.m_it; + } +}; + +// An ObjectType whose iterators are forward-only, as those of hash maps are; +// it has no rbegin() and its iterators no operator--. A hash map is not used +// directly for the same reason as in no_key_compare_map above. +template +class forward_only_map +{ + using map_t = std::map; + map_t data; + + public: + using key_type = typename map_t::key_type; + using mapped_type = typename map_t::mapped_type; + using value_type = typename map_t::value_type; + using size_type = typename map_t::size_type; + using allocator_type = typename map_t::allocator_type; + using iterator = forward_only_iterator; + using const_iterator = forward_only_iterator; + + forward_only_map() noexcept(std::is_nothrow_default_constructible::value) : data() {} + + template + forward_only_map(InputIt first, InputIt last) : data(first, last) {} + + iterator begin() noexcept + { + return iterator(data.begin()); + } + iterator end() noexcept + { + return iterator(data.end()); + } + const_iterator begin() const noexcept + { + return const_iterator(data.begin()); + } + const_iterator end() const noexcept + { + return const_iterator(data.end()); + } + const_iterator cbegin() const noexcept + { + return const_iterator(data.cbegin()); + } + const_iterator cend() const noexcept + { + return const_iterator(data.cend()); + } + + bool empty() const noexcept + { + return data.empty(); + } + size_type size() const noexcept + { + return data.size(); + } + size_type max_size() const noexcept + { + return data.max_size(); + } + void clear() noexcept + { + data.clear(); + } + + iterator find(const key_type& key) + { + return iterator(data.find(key)); + } + const_iterator find(const key_type& key) const + { + return const_iterator(data.find(key)); + } + size_type count(const key_type& key) const + { + return data.count(key); + } + + std::pair emplace(const key_type& key, const mapped_type& value) + { + const auto result = data.emplace(key, value); + return {iterator(result.first), result.second}; + } + + std::pair insert(const value_type& value) + { + const auto result = data.insert(value); + return {iterator(result.first), result.second}; + } + + template + void insert(InputIt first, InputIt last) + { + data.insert(first, last); + } + + mapped_type& operator[](const key_type& key) + { + return data[key]; + } + + mapped_type& at(const key_type& key) + { + return data.at(key); + } + const mapped_type& at(const key_type& key) const + { + return data.at(key); + } + + iterator erase(iterator pos) + { + return iterator(data.erase(pos.base())); + } + iterator erase(iterator first, iterator last) + { + return iterator(data.erase(first.base(), last.base())); + } + size_type erase(const key_type& key) + { + return data.erase(key); + } + + void swap(forward_only_map& other) noexcept(noexcept(data.swap(other.data))) + { + data.swap(other.data); + } + + friend bool operator==(const forward_only_map& lhs, const forward_only_map& rhs) + { + return lhs.data == rhs.data; + } + friend bool operator<(const forward_only_map& lhs, const forward_only_map& rhs) + { + return lhs.data < rhs.data; + } +}; + +using forward_only_json = nlohmann::basic_json; + } // namespace TEST_CASE("object type whose erase() returns void") @@ -322,3 +516,46 @@ TEST_CASE("object type without key_compare") } } + +TEST_CASE("object type with forward-only iterators") +{ + CHECK(std::is_same::iterator_category, + std::forward_iterator_tag>::value); + + SECTION("destroying nested objects and arrays") + { + forward_only_json j; + j["a"] = 1; + j["b"]["c"] = "x"; + j["b"]["d"] = forward_only_json::array(); + j["b"]["d"].push_back(forward_only_json::object()); + j["b"]["d"].push_back(true); + j["b"]["e"]["f"]["g"] = nullptr; + j["h"] = forward_only_json::object(); + j["i"]["j"] = 2; + + CHECK(j.size() == 4); + CHECK(j["b"].size() == 3); + CHECK(j["b"]["d"].size() == 2); + CHECK(j["b"]["e"]["f"]["g"].is_null()); + + CHECK(j.erase("b") == 1); + CHECK(j.size() == 3); + j = 42; + CHECK(j == 42); + } + + SECTION("destroying a deeply nested object") + { + constexpr std::size_t depth = 100000; + forward_only_json j; + forward_only_json* cur = &j; + for (std::size_t i = 0; i < depth; ++i) + { + (*cur)["s"] = i; + cur = &(*cur)["o"]; + } + CHECK(j["o"]["o"]["s"] == 2); + // destroyed at the end of scope without recursing per level + } +}