From d40e98ecef776b909af4d91ffa266a966290e73f Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 12 Jul 2021 13:38:28 +0200 Subject: [PATCH 1/5] :bug: fix assertion failure #2838 --- include/nlohmann/json.hpp | 61 ++++++++++++++++++++++++++------ single_include/nlohmann/json.hpp | 61 ++++++++++++++++++++++++++------ test/src/unit-diagnostics.cpp | 57 +++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 22 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index c55bdf54b..0622d1a9c 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -5371,8 +5371,18 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (move semantics) + const auto capacity = m_value.array->capacity(); m_value.array->push_back(std::move(val)); - set_parent(m_value.array->back()); + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of last element is sufficient + set_parent(m_value.array->back()); + } + else + { + // capacity has changed: update all elements' parents + set_parents(); + } // if val is moved from, basic_json move constructor marks it null so we do not call the destructor } @@ -5407,7 +5417,18 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array + const auto capacity = m_value.array->capacity(); m_value.array->push_back(val); + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of last element is sufficient + set_parent(m_value.array->back()); + } + else + { + // capacity has changed: update all elements' parents + set_parents(); + } set_parent(m_value.array->back()); } @@ -5562,12 +5583,18 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (perfect forwarding) -#ifdef JSON_HAS_CPP_17 - return set_parent(m_value.array->emplace_back(std::forward(args)...)); -#else + const auto capacity = m_value.array->capacity(); m_value.array->emplace_back(std::forward(args)...); - return set_parent(m_value.array->back()); -#endif + + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of last element is sufficient + return set_parent(m_value.array->back()); + } + + // capacity has changed: update all elements' parents + set_parents(); + return m_value.array->back(); } /*! @@ -5630,12 +5657,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @note: This uses std::distance to support GCC 4.8, /// see https://github.com/nlohmann/json/pull/1257 template - iterator insert_iterator(const_iterator pos, Args&& ... args) + iterator insert_iterator(const_iterator pos, typename iterator::difference_type cnt, Args&& ... args) { iterator result(this); JSON_ASSERT(m_value.array != nullptr); auto insert_pos = std::distance(m_value.array->begin(), pos.m_it.array_iterator); + const auto capacity = m_value.array->capacity(); m_value.array->insert(pos.m_it.array_iterator, std::forward(args)...); result.m_it.array_iterator = m_value.array->begin() + insert_pos; @@ -5643,6 +5671,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // result.m_it.array_iterator = m_value.array->insert(pos.m_it.array_iterator, cnt, val); // but the return value of insert is missing in GCC 4.8, so it is written this way instead. + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of inserted elements is sufficient + set_parents(result, cnt); + } + else + { + // capacity has changed: update all elements' parents + set_parents(); + } + return result; } @@ -5680,7 +5719,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, val), static_cast(1)); + return insert_iterator(pos, static_cast(1), val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -5731,7 +5770,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, cnt, val), static_cast(cnt)); + return insert_iterator(pos, static_cast(cnt), cnt, val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -5793,7 +5832,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, first.m_it.array_iterator, last.m_it.array_iterator), std::distance(first, last)); + return insert_iterator(pos, std::distance(first, last), first.m_it.array_iterator, last.m_it.array_iterator); } /*! @@ -5835,7 +5874,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, ilist.begin(), ilist.end()), static_cast(ilist.size())); + return insert_iterator(pos, static_cast(ilist.size()), ilist.begin(), ilist.end()); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index cbe69ef47..999d136aa 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -22406,8 +22406,18 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (move semantics) + const auto capacity = m_value.array->capacity(); m_value.array->push_back(std::move(val)); - set_parent(m_value.array->back()); + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of last element is sufficient + set_parent(m_value.array->back()); + } + else + { + // capacity has changed: update all elements' parents + set_parents(); + } // if val is moved from, basic_json move constructor marks it null so we do not call the destructor } @@ -22442,7 +22452,18 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array + const auto capacity = m_value.array->capacity(); m_value.array->push_back(val); + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of last element is sufficient + set_parent(m_value.array->back()); + } + else + { + // capacity has changed: update all elements' parents + set_parents(); + } set_parent(m_value.array->back()); } @@ -22597,12 +22618,18 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (perfect forwarding) -#ifdef JSON_HAS_CPP_17 - return set_parent(m_value.array->emplace_back(std::forward(args)...)); -#else + const auto capacity = m_value.array->capacity(); m_value.array->emplace_back(std::forward(args)...); - return set_parent(m_value.array->back()); -#endif + + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of last element is sufficient + return set_parent(m_value.array->back()); + } + + // capacity has changed: update all elements' parents + set_parents(); + return m_value.array->back(); } /*! @@ -22665,12 +22692,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @note: This uses std::distance to support GCC 4.8, /// see https://github.com/nlohmann/json/pull/1257 template - iterator insert_iterator(const_iterator pos, Args&& ... args) + iterator insert_iterator(const_iterator pos, typename iterator::difference_type cnt, Args&& ... args) { iterator result(this); JSON_ASSERT(m_value.array != nullptr); auto insert_pos = std::distance(m_value.array->begin(), pos.m_it.array_iterator); + const auto capacity = m_value.array->capacity(); m_value.array->insert(pos.m_it.array_iterator, std::forward(args)...); result.m_it.array_iterator = m_value.array->begin() + insert_pos; @@ -22678,6 +22706,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // result.m_it.array_iterator = m_value.array->insert(pos.m_it.array_iterator, cnt, val); // but the return value of insert is missing in GCC 4.8, so it is written this way instead. + if (capacity == m_value.array->capacity()) + { + // capacity has not changed: updating parent of inserted elements is sufficient + set_parents(result, cnt); + } + else + { + // capacity has changed: update all elements' parents + set_parents(); + } + return result; } @@ -22715,7 +22754,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, val), static_cast(1)); + return insert_iterator(pos, static_cast(1), val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -22766,7 +22805,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, cnt, val), static_cast(cnt)); + return insert_iterator(pos, static_cast(cnt), cnt, val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -22828,7 +22867,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, first.m_it.array_iterator, last.m_it.array_iterator), std::distance(first, last)); + return insert_iterator(pos, std::distance(first, last), first.m_it.array_iterator, last.m_it.array_iterator); } /*! @@ -22870,7 +22909,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return set_parents(insert_iterator(pos, ilist.begin(), ilist.end()), static_cast(ilist.size())); + return insert_iterator(pos, static_cast(ilist.size()), ilist.begin(), ilist.end()); } /*! diff --git a/test/src/unit-diagnostics.cpp b/test/src/unit-diagnostics.cpp index 21ced33b1..c0b5a6476 100644 --- a/test/src/unit-diagnostics.cpp +++ b/test/src/unit-diagnostics.cpp @@ -109,4 +109,61 @@ TEST_CASE("Better diagnostics") j["/foo"] = {1, 2, 3}; CHECK_THROWS_WITH_AS(j.unflatten(), "[json.exception.type_error.315] (/~1foo) values in object must be primitive", json::type_error); } + + SECTION("Regression test for https://github.com/nlohmann/json/issues/2838") + { + // void push_back(basic_json&& val) + { + json j_arr = json::array(); + j_arr.push_back(json::object()); + j_arr.push_back(json::object()); + json j_obj = json::object(); + j_obj["key"] = j_arr; + } + + // void push_back(const basic_json& val) + { + json j_arr = json::array(); + auto object = json::object(); + j_arr.push_back(object); + j_arr.push_back(object); + json j_obj = json::object(); + j_obj["key"] = j_arr; + } + + // reference emplace_back(Args&& ... args) + { + json j_arr = json::array(); + j_arr.emplace_back(json::object()); + j_arr.emplace_back(json::object()); + json j_obj = json::object(); + j_obj["key"] = j_arr; + } + + // iterator insert(const_iterator pos, const basic_json& val) + { + json j_arr = json::array(); + j_arr.insert(j_arr.begin(), json::object()); + j_arr.insert(j_arr.begin(), json::object()); + json j_obj = json::object(); + j_obj["key"] = j_arr; + } + + // iterator insert(const_iterator pos, size_type cnt, const basic_json& val) + { + json j_arr = json::array(); + j_arr.insert(j_arr.begin(), 2, json::object()); + json j_obj = json::object(); + j_obj["key"] = j_arr; + } + + // iterator insert(const_iterator pos, const_iterator first, const_iterator last) + { + json j_arr = json::array(); + json j_objects = {json::object(), json::object()}; + j_arr.insert(j_arr.begin(), j_objects.begin(), j_objects.end()); + json j_obj = json::object(); + j_obj["key"] = j_arr; + } + } } From b0730f29cf2376a27591197eb793410370fdd32b Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 12 Jul 2021 15:24:06 +0200 Subject: [PATCH 2/5] :bug: fix logics --- include/nlohmann/json.hpp | 23 ++++++----------------- single_include/nlohmann/json.hpp | 23 ++++++----------------- test/src/unit-diagnostics.cpp | 8 ++++++++ 3 files changed, 20 insertions(+), 34 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 0622d1a9c..710c5a5cf 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -5657,13 +5657,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @note: This uses std::distance to support GCC 4.8, /// see https://github.com/nlohmann/json/pull/1257 template - iterator insert_iterator(const_iterator pos, typename iterator::difference_type cnt, Args&& ... args) + iterator insert_iterator(const_iterator pos, Args&& ... args) { iterator result(this); JSON_ASSERT(m_value.array != nullptr); auto insert_pos = std::distance(m_value.array->begin(), pos.m_it.array_iterator); - const auto capacity = m_value.array->capacity(); m_value.array->insert(pos.m_it.array_iterator, std::forward(args)...); result.m_it.array_iterator = m_value.array->begin() + insert_pos; @@ -5671,17 +5670,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // result.m_it.array_iterator = m_value.array->insert(pos.m_it.array_iterator, cnt, val); // but the return value of insert is missing in GCC 4.8, so it is written this way instead. - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of inserted elements is sufficient - set_parents(result, cnt); - } - else - { - // capacity has changed: update all elements' parents - set_parents(); - } - + set_parents(); return result; } @@ -5719,7 +5708,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, static_cast(1), val); + return insert_iterator(pos, val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -5770,7 +5759,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, static_cast(cnt), cnt, val); + return insert_iterator(pos, cnt, val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -5832,7 +5821,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, std::distance(first, last), first.m_it.array_iterator, last.m_it.array_iterator); + return insert_iterator(pos, first.m_it.array_iterator, last.m_it.array_iterator); } /*! @@ -5874,7 +5863,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, static_cast(ilist.size()), ilist.begin(), ilist.end()); + return insert_iterator(pos, ilist.begin(), ilist.end()); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 999d136aa..3700505c9 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -22692,13 +22692,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @note: This uses std::distance to support GCC 4.8, /// see https://github.com/nlohmann/json/pull/1257 template - iterator insert_iterator(const_iterator pos, typename iterator::difference_type cnt, Args&& ... args) + iterator insert_iterator(const_iterator pos, Args&& ... args) { iterator result(this); JSON_ASSERT(m_value.array != nullptr); auto insert_pos = std::distance(m_value.array->begin(), pos.m_it.array_iterator); - const auto capacity = m_value.array->capacity(); m_value.array->insert(pos.m_it.array_iterator, std::forward(args)...); result.m_it.array_iterator = m_value.array->begin() + insert_pos; @@ -22706,17 +22705,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // result.m_it.array_iterator = m_value.array->insert(pos.m_it.array_iterator, cnt, val); // but the return value of insert is missing in GCC 4.8, so it is written this way instead. - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of inserted elements is sufficient - set_parents(result, cnt); - } - else - { - // capacity has changed: update all elements' parents - set_parents(); - } - + set_parents(); return result; } @@ -22754,7 +22743,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, static_cast(1), val); + return insert_iterator(pos, val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -22805,7 +22794,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, static_cast(cnt), cnt, val); + return insert_iterator(pos, cnt, val); } JSON_THROW(type_error::create(309, "cannot use insert() with " + std::string(type_name()), *this)); @@ -22867,7 +22856,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, std::distance(first, last), first.m_it.array_iterator, last.m_it.array_iterator); + return insert_iterator(pos, first.m_it.array_iterator, last.m_it.array_iterator); } /*! @@ -22909,7 +22898,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // insert to array and return iterator - return insert_iterator(pos, static_cast(ilist.size()), ilist.begin(), ilist.end()); + return insert_iterator(pos, ilist.begin(), ilist.end()); } /*! diff --git a/test/src/unit-diagnostics.cpp b/test/src/unit-diagnostics.cpp index c0b5a6476..ebbe64f38 100644 --- a/test/src/unit-diagnostics.cpp +++ b/test/src/unit-diagnostics.cpp @@ -117,6 +117,8 @@ TEST_CASE("Better diagnostics") json j_arr = json::array(); j_arr.push_back(json::object()); j_arr.push_back(json::object()); + j_arr.push_back(json::object()); + j_arr.push_back(json::object()); json j_obj = json::object(); j_obj["key"] = j_arr; } @@ -127,6 +129,8 @@ TEST_CASE("Better diagnostics") auto object = json::object(); j_arr.push_back(object); j_arr.push_back(object); + j_arr.push_back(object); + j_arr.push_back(object); json j_obj = json::object(); j_obj["key"] = j_arr; } @@ -136,6 +140,8 @@ TEST_CASE("Better diagnostics") json j_arr = json::array(); j_arr.emplace_back(json::object()); j_arr.emplace_back(json::object()); + j_arr.emplace_back(json::object()); + j_arr.emplace_back(json::object()); json j_obj = json::object(); j_obj["key"] = j_arr; } @@ -145,6 +151,8 @@ TEST_CASE("Better diagnostics") json j_arr = json::array(); j_arr.insert(j_arr.begin(), json::object()); j_arr.insert(j_arr.begin(), json::object()); + j_arr.insert(j_arr.begin(), json::object()); + j_arr.insert(j_arr.begin(), json::object()); json j_obj = json::object(); j_obj["key"] = j_arr; } From bc7e8faa4f1f463ef97b2c1491b1f8886bec154b Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 12 Jul 2021 19:21:07 +0200 Subject: [PATCH 3/5] :fire: remove duplicated line --- include/nlohmann/json.hpp | 1 - single_include/nlohmann/json.hpp | 1 - 2 files changed, 2 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 710c5a5cf..ae77747b5 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -5429,7 +5429,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // capacity has changed: update all elements' parents set_parents(); } - set_parent(m_value.array->back()); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 3700505c9..2500eb9cb 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -22464,7 +22464,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // capacity has changed: update all elements' parents set_parents(); } - set_parent(m_value.array->back()); } /*! From 3bb9467073f366bfb2b46c9c6a0c1fe11219da60 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Tue, 13 Jul 2021 15:27:27 +0200 Subject: [PATCH 4/5] :recycle: move capacity check to set_parent function --- include/nlohmann/json.hpp | 53 ++++++++++++-------------------- single_include/nlohmann/json.hpp | 53 ++++++++++++-------------------- 2 files changed, 38 insertions(+), 68 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index ae77747b5..d18f082b4 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -1304,9 +1304,21 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec return it; } - reference set_parent(reference j) + reference set_parent(reference j, std::size_t old_capacity = -1) { #if JSON_DIAGNOSTICS + if (old_capacity != -1) + { + // see https://github.com/nlohmann/json/issues/2838 + JSON_ASSERT(type() == value_t::array); + if (JSON_HEDLEY_UNLIKELY(m_value.array->capacity() != old_capacity)) + { + // capacity has changed: update all parents + set_parents(); + return j; + } + } + j.m_parent = this; #else static_cast(j); @@ -5371,18 +5383,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (move semantics) - const auto capacity = m_value.array->capacity(); + const auto old_capacity = m_value.array->capacity(); m_value.array->push_back(std::move(val)); - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of last element is sufficient - set_parent(m_value.array->back()); - } - else - { - // capacity has changed: update all elements' parents - set_parents(); - } + set_parent(m_value.array->back(), old_capacity); // if val is moved from, basic_json move constructor marks it null so we do not call the destructor } @@ -5417,18 +5420,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array - const auto capacity = m_value.array->capacity(); + const auto old_capacity = m_value.array->capacity(); m_value.array->push_back(val); - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of last element is sufficient - set_parent(m_value.array->back()); - } - else - { - // capacity has changed: update all elements' parents - set_parents(); - } + set_parent(m_value.array->back(), old_capacity); } /*! @@ -5582,18 +5576,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (perfect forwarding) - const auto capacity = m_value.array->capacity(); + const auto old_capacity = m_value.array->capacity(); m_value.array->emplace_back(std::forward(args)...); - - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of last element is sufficient - return set_parent(m_value.array->back()); - } - - // capacity has changed: update all elements' parents - set_parents(); - return m_value.array->back(); + return set_parent(m_value.array->back(), old_capacity); } /*! diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 2500eb9cb..3b5a7a75d 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -18339,9 +18339,21 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec return it; } - reference set_parent(reference j) + reference set_parent(reference j, std::size_t old_capacity = -1) { #if JSON_DIAGNOSTICS + if (old_capacity != -1) + { + // see https://github.com/nlohmann/json/issues/2838 + JSON_ASSERT(type() == value_t::array); + if (JSON_HEDLEY_UNLIKELY(m_value.array->capacity() != old_capacity)) + { + // capacity has changed: update all parents + set_parents(); + return j; + } + } + j.m_parent = this; #else static_cast(j); @@ -22406,18 +22418,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (move semantics) - const auto capacity = m_value.array->capacity(); + const auto old_capacity = m_value.array->capacity(); m_value.array->push_back(std::move(val)); - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of last element is sufficient - set_parent(m_value.array->back()); - } - else - { - // capacity has changed: update all elements' parents - set_parents(); - } + set_parent(m_value.array->back(), old_capacity); // if val is moved from, basic_json move constructor marks it null so we do not call the destructor } @@ -22452,18 +22455,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array - const auto capacity = m_value.array->capacity(); + const auto old_capacity = m_value.array->capacity(); m_value.array->push_back(val); - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of last element is sufficient - set_parent(m_value.array->back()); - } - else - { - // capacity has changed: update all elements' parents - set_parents(); - } + set_parent(m_value.array->back(), old_capacity); } /*! @@ -22617,18 +22611,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } // add element to array (perfect forwarding) - const auto capacity = m_value.array->capacity(); + const auto old_capacity = m_value.array->capacity(); m_value.array->emplace_back(std::forward(args)...); - - if (capacity == m_value.array->capacity()) - { - // capacity has not changed: updating parent of last element is sufficient - return set_parent(m_value.array->back()); - } - - // capacity has changed: update all elements' parents - set_parents(); - return m_value.array->back(); + return set_parent(m_value.array->back(), old_capacity); } /*! From a711e1f5a721b12987b5748da37d3b8c2992764b Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Tue, 13 Jul 2021 15:37:57 +0200 Subject: [PATCH 5/5] :rotating_light: fix warnings --- include/nlohmann/json.hpp | 5 +++-- single_include/nlohmann/json.hpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index d18f082b4..a337c1c69 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -1304,10 +1304,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec return it; } - reference set_parent(reference j, std::size_t old_capacity = -1) + reference set_parent(reference j, std::size_t old_capacity = std::size_t(-1)) { #if JSON_DIAGNOSTICS - if (old_capacity != -1) + if (old_capacity != std::size_t(-1)) { // see https://github.com/nlohmann/json/issues/2838 JSON_ASSERT(type() == value_t::array); @@ -1322,6 +1322,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec j.m_parent = this; #else static_cast(j); + static_cast(old_capacity); #endif return j; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 3b5a7a75d..429964dd7 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -18339,10 +18339,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec return it; } - reference set_parent(reference j, std::size_t old_capacity = -1) + reference set_parent(reference j, std::size_t old_capacity = std::size_t(-1)) { #if JSON_DIAGNOSTICS - if (old_capacity != -1) + if (old_capacity != std::size_t(-1)) { // see https://github.com/nlohmann/json/issues/2838 JSON_ASSERT(type() == value_t::array); @@ -18357,6 +18357,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec j.m_parent = this; #else static_cast(j); + static_cast(old_capacity); #endif return j; }