Make insert(pos, basic_json&&) move its argument instead of copying it

insert(const_iterator pos, basic_json&& val) delegated to
insert(pos, val), but val is a named rvalue reference, so inside the
function it is an lvalue: the call always resolved to
insert(const_iterator, const basic_json&) and deep-copied the value.
This has been the case since the overload was introduced, in every
release. push_back(basic_json&&), by contrast, already moves.

Give the rvalue overload its own body with the same two checks
(type_error.309, invalid_iterator.202), then move the argument into a
local before inserting it. Moving into a local first, rather than
inserting std::move(val) directly, keeps this safe even when val
aliases an element of the same array (e.g.
arr.insert(arr.begin(), std::move(arr[1]))), since
std::vector::insert(pos, T&&) is not guaranteed to handle an argument
that aliases one of its own elements.

This is a deliberate, small behavior change: the moved-from argument
now ends up null afterwards, the same as after push_back(&&), instead
of keeping its old value unchanged. No signature changes, so the
public API and ABI are unaffected. Add unit-modifiers coverage for
the moved-from state and for self-aliasing insertion, both with and
without reallocation of the underlying array.

Part of #5724

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

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

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

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<const std::string&>().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<const std::string&>().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<json::array_t&>().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<json::array_t&>().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()")