Use the reserve_array helper in the initializer_list insert fix

The previous commit called array_t::reserve() directly on the
temporary buffer used to copy an ilist's values before inserting.
std::deque, a documented ArrayType (tests/src/unit-custom-array-type.cpp),
has no reserve(), so insert(pos, initializer_list) no longer compiled
for it. Use the existing detail::reserve_array() SFINAE helper (already
used by the SAX DOM parser) instead, which leaves array types without
reserve() untouched.

Added a regression check that deque_json::insert(pos, {...}) compiles
and handles the aliasing case from #5656.

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

docs/mkdocs/docs/api/basic_json/insert.md

@@ -195,5 +195,7 @@ Strong exception safety: if an exception occurs, the original value stays intact
 1. Added in version 1.0.0.
 2. Added in version 1.0.0.
 3. Added in version 1.0.0.
-4. Added in version 1.0.0.
+4. Added in version 1.0.0. Fixed in version 3.13.0 to copy the values before inserting; before, an `ilist` that
+   referred to elements of the array being inserted into could insert wrong values, because the range insert could
+   move from or shift an element before it was copied.
 5. Added in version 3.0.0.

include/nlohmann/json.hpp

@@ -4152,8 +4152,16 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
             JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this));
         }
 
+        // copy the values first: ilist may refer to elements of this array
+        array_t values;
+        detail::reserve_array(values, ilist.size(), detail::priority_tag<1> {});
+        for (const auto& element : ilist)
+        {
+            values.push_back(element.moved_or_copied());
+        }
+
         // insert to array and return iterator
-        return insert_iterator(pos, ilist.begin(), ilist.end());
+        return insert_iterator(pos, std::make_move_iterator(values.begin()), std::make_move_iterator(values.end()));
     }
 
     /// @brief inserts range of elements into object

single_include/nlohmann/json.hpp

@@ -30233,8 +30233,16 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
             JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this));
         }
 
+        // copy the values first: ilist may refer to elements of this array
+        array_t values;
+        detail::reserve_array(values, ilist.size(), detail::priority_tag<1> {});
+        for (const auto& element : ilist)
+        {
+            values.push_back(element.moved_or_copied());
+        }
+
         // insert to array and return iterator
-        return insert_iterator(pos, ilist.begin(), ilist.end());
+        return insert_iterator(pos, std::make_move_iterator(values.begin()), std::make_move_iterator(values.end()));
     }
 
     /// @brief inserts range of elements into object

tests/src/unit-custom-array-type.cpp

@@ -117,6 +117,22 @@ TEST_CASE("array type without capacity()")
         CHECK(nested.flatten().unflatten() == nested);
     }
 
+    SECTION("insert(pos, initializer_list) compiles and works without reserve()")
+    {
+        // std::deque has no reserve() either; insert(pos, ilist) must not
+        // require it (regression test for #5656, which also covers an ilist
+        // that refers to elements of the array being inserted into)
+        deque_json j = deque_json::array();
+        j.push_back("a");
+        j.push_back("b");
+        j.push_back("c");
+
+        const deque_json& cj = j;
+        auto it = j.insert(j.begin(), {cj[0], cj[1]});
+        CHECK(*it == deque_json("a"));
+        CHECK(j == deque_json({"a", "b", "a", "b", "c"}));
+    }
+
     SECTION("references stay valid while the array grows")
     {
         deque_json j = deque_json::array();

tests/src/unit-modifiers.cpp

@@ -761,6 +761,34 @@ TEST_CASE("modifiers")
             }
         }
 
+        SECTION("initializer list referring to the array's own elements (#5656)")
+        {
+            SECTION("sufficient capacity (no reallocation)")
+            {
+                json j_own = json::array();
+                j_own.get_ref<json::array_t&>().reserve(8);
+                j_own.push_back("a");
+                j_own.push_back("b");
+                j_own.push_back("c");
+
+                const json& j_own_cref = j_own;
+                auto it = j_own.insert(j_own.begin(), {j_own_cref[0], j_own_cref[1]});
+                CHECK(*it == json("a"));
+                CHECK(j_own == json({"a", "b", "a", "b", "c"}));
+            }
+
+            SECTION("insufficient capacity (reallocation)")
+            {
+                json j_own = {"a", "b", "c"};
+                j_own.get_ref<json::array_t&>().shrink_to_fit();
+
+                const json& j_own_cref = j_own;
+                auto it = j_own.insert(j_own.begin(), {j_own_cref[2]});
+                CHECK(*it == json("c"));
+                CHECK(j_own == json({"c", "a", "b", "c"}));
+            }
+        }
+
         SECTION("invalid iterator")
         {
             // pass iterator to a different array

tools/amalgamate/CHANGES.md

@@ -8,6 +8,3 @@ The following changes have been made to the code with respect to <https://github
   - membership check
   - made function from `_is_within`
   - removed unused variable `actual_path`
-- Added the optional config key `external`: include paths listed there are kept as
-  `#include` directives instead of being inlined (the first directive per path; the
-  repeated ones are commented out).

tools/amalgamate/README.md

@@ -57,11 +57,6 @@ Python v.2.7.0 or higher is required.
    amalgamation. Have a look at `test/source.c.json` and `test/include.h.json`
    to see two examples.
 
-   The optional `external` list names include paths that are kept as `#include`
-   directives instead of being inlined, e.g. `["nlohmann/json.hpp"]` for a header
-   that includes another amalgamated header. Only the first directive for each
-   of these paths is kept; the repeated ones are commented out.
-
  * The `-s, --source` option should specify the path to the source directory.
    This is useful for supporting separate source and build directories.
 

tools/amalgamate/amalgamate.py

@@ -62,10 +62,6 @@ class Amalgamation(object):
         return None
 
     def __init__(self, args):
-        # include paths that are kept as #include directives instead of
-        # being inlined (e.g. a header amalgamated on its own)
-        self.external = []
-        self.included_external = []
         with open(args.config, 'r') as f:
             config = json.loads(f.read())
             for key in config:
@@ -224,14 +220,11 @@ class TranslationUnit(object):
         while include_match:
             if not _is_within(include_match, skippable_contexts):
                 include_path = include_match.group("path")
-                if include_path in self.amalgamation.external:
-                    includes.append((include_match, None))
-                else:
-                    search_same_dir = include_match.group(1) == '"'
-                    found_included_path = self.amalgamation.find_included_file(
-                        include_path, self.file_dir if search_same_dir else None)
-                    if found_included_path:
-                        includes.append((include_match, found_included_path))
+                search_same_dir = include_match.group(1) == '"'
+                found_included_path = self.amalgamation.find_included_file(
+                    include_path, self.file_dir if search_same_dir else None)
+                if found_included_path:
+                    includes.append((include_match, found_included_path))
 
             include_match = self.include_pattern.search(self.content,
                                                         include_match.end())
@@ -242,17 +235,6 @@ class TranslationUnit(object):
         for include in includes:
             include_match, found_included_path = include
             tmp_content += self.content[prev_end:include_match.start()]
-            if found_included_path is None:
-                # an external header: keep the first directive and comment
-                # out the repeated ones
-                include_path = include_match.group("path")
-                if include_path in self.amalgamation.included_external:
-                    tmp_content += "// {0}".format(include_match.group(0))
-                else:
-                    self.amalgamation.included_external.append(include_path)
-                    tmp_content += include_match.group(0)
-                prev_end = include_match.end()
-                continue
             tmp_content += "// {0}\n".format(include_match.group(0))
             if found_included_path not in self.amalgamation.included_files:
                 t = TranslationUnit(found_included_path, self.amalgamation, False)

tools/amalgamate/config_json_view.json

@@ -1,9 +0,0 @@
-{
-	"project": "JSON for Modern C++",
-	"target": "single_include/nlohmann/json_view.hpp",
-	"sources": [
-		"include/nlohmann/json_view.hpp"
-	],
-	"include_paths": ["include"],
-	"external": ["nlohmann/json.hpp"]
-}