diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index f55aad1aa..0c7a95baa 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4043,7 +4043,22 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/insert/ iterator insert(const_iterator pos, basic_json&& val) // NOLINT(performance-unnecessary-value-param) { - return insert(std::move(pos), val); + // insert only works for arrays + if (JSON_HEDLEY_LIKELY(is_array())) + { + // check if iterator pos fits to this JSON value + if (JSON_HEDLEY_UNLIKELY(pos.m_object != this)) + { + JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this)); + } + + // moving into a local first keeps this safe even if val aliases + // an element of this array + basic_json tmp(std::move(val)); + return insert_iterator(pos, std::move(tmp)); + } + + JSON_THROW(type_error::create(309, detail::concat("cannot use insert() with ", type_name()), this)); } /// @brief inserts copies of element into array diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 64d1b4e36..2971c2f23 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -30123,7 +30123,22 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/insert/ iterator insert(const_iterator pos, basic_json&& val) // NOLINT(performance-unnecessary-value-param) { - return insert(std::move(pos), val); + // insert only works for arrays + if (JSON_HEDLEY_LIKELY(is_array())) + { + // check if iterator pos fits to this JSON value + if (JSON_HEDLEY_UNLIKELY(pos.m_object != this)) + { + JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this)); + } + + // moving into a local first keeps this safe even if val aliases + // an element of this array + basic_json tmp(std::move(val)); + return insert_iterator(pos, std::move(tmp)); + } + + JSON_THROW(type_error::create(309, detail::concat("cannot use insert() with ", type_name()), this)); } /// @brief inserts copies of element into array diff --git a/tests/src/unit-modifiers.cpp b/tests/src/unit-modifiers.cpp index c878ec15c..794de1b41 100644 --- a/tests/src/unit-modifiers.cpp +++ b/tests/src/unit-modifiers.cpp @@ -618,6 +618,49 @@ TEST_CASE("modifiers") } } + SECTION("rvalue at position moves rather than copies") + { + // regression test: insert(pos, basic_json&&) used to forward to + // insert(pos, const basic_json&) because the named rvalue + // reference parameter is itself an lvalue, so it always + // deep-copied its argument instead of moving it + json j_big = std::string(1000, 'x'); + const auto* const original_buffer = j_big.get_ref().data(); + + auto it = j_array.insert(j_array.begin(), std::move(j_big)); + CHECK(j_array.size() == 5); + CHECK(*it == json(std::string(1000, 'x'))); + CHECK((*it).get_ref().data() == original_buffer); + + // the moved-from value is null, the same as after push_back(&&) + CHECK(j_big.is_null()); // NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved) + } + + SECTION("self-aliasing insertion") + { + SECTION("without reallocation") + { + json j_self = {1, 2, 3, 4}; + j_self.get_ref().reserve(j_self.size() + 1); + + auto it = j_self.insert(j_self.begin(), std::move(j_self[1])); + CHECK(j_self.size() == 5); + CHECK(*it == json(2)); + CHECK(j_self == json({2, 1, nullptr, 3, 4})); + } + + SECTION("with reallocation") + { + json j_self = {1, 2, 3, 4}; + j_self.get_ref().shrink_to_fit(); + + auto it = j_self.insert(j_self.begin(), std::move(j_self[1])); + CHECK(j_self.size() == 5); + CHECK(*it == json(2)); + CHECK(j_self == json({2, 1, nullptr, 3, 4})); + } + } + SECTION("copies at position") { SECTION("insert before begin()")