Keep external headers as #include when amalgamating

A header that builds on json.hpp (such as the planned json_view.hpp) must
not inline json.hpp: its single-header version would contain a second copy
of the library, and that copy would change with every library change.

The optional config key "external" lists include paths that are kept as
#include directives. Only the first directive per path is kept; repeated
ones are commented out, as the tool already does for inlined headers.
config_json_view.json uses it for json_view.hpp; the existing configs do
not set it, and json.hpp and json_fwd.hpp regenerate byte-identically.

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

docs/mkdocs/docs/api/basic_json/emplace.md

@@ -69,5 +69,3 @@ Logarithmic in the size of the container, O(log(`size()`)).
 ## Version history
 
 - Since version 2.0.8.
-- Fixed in version 3.13.0: for [`ordered_json`](../ordered_json.md), the value could previously only be passed as an
-  rvalue; it can now also be passed as an lvalue or a `#!cpp const` lvalue, matching the behavior of `json`.

include/nlohmann/ordered_map.hpp

@@ -70,9 +70,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
         return *this;
     }
 
-    template < class V, detail::enable_if_t <
-                   detail::is_constructible < T, V&& >::value, int > = 0 >
-    std::pair<iterator, bool> emplace(const key_type& key, V && t)
+    std::pair<iterator, bool> emplace(const key_type& key, T&& t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -81,14 +79,13 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        Container::emplace_back(key, std::forward<V>(t));
+        Container::emplace_back(key, std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 
-    template < class KeyType, class V, detail::enable_if_t <
-                   detail::is_usable_as_key_type<key_compare, key_type, KeyType>::value&&
-                   detail::is_constructible < T, V&& >::value, int > = 0 >
-    std::pair<iterator, bool> emplace(KeyType && key, V && t)
+    template<class KeyType, detail::enable_if_t<
+                 detail::is_usable_as_key_type<key_compare, key_type, KeyType>::value, int> = 0>
+    std::pair<iterator, bool> emplace(KeyType && key, T && t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -97,7 +94,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        Container::emplace_back(std::forward<KeyType>(key), std::forward<V>(t));
+        Container::emplace_back(std::forward<KeyType>(key), std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 

single_include/nlohmann/json.hpp

@@ -25832,9 +25832,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
         return *this;
     }
 
-    template < class V, detail::enable_if_t <
-                   detail::is_constructible < T, V&& >::value, int > = 0 >
-    std::pair<iterator, bool> emplace(const key_type& key, V && t)
+    std::pair<iterator, bool> emplace(const key_type& key, T&& t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -25843,14 +25841,13 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        Container::emplace_back(key, std::forward<V>(t));
+        Container::emplace_back(key, std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 
-    template < class KeyType, class V, detail::enable_if_t <
-                   detail::is_usable_as_key_type<key_compare, key_type, KeyType>::value&&
-                   detail::is_constructible < T, V&& >::value, int > = 0 >
-    std::pair<iterator, bool> emplace(KeyType && key, V && t)
+    template<class KeyType, detail::enable_if_t<
+                 detail::is_usable_as_key_type<key_compare, key_type, KeyType>::value, int> = 0>
+    std::pair<iterator, bool> emplace(KeyType && key, T && t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -25859,7 +25856,7 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
                 return {it, false};
             }
         }
-        Container::emplace_back(std::forward<KeyType>(key), std::forward<V>(t));
+        Container::emplace_back(std::forward<KeyType>(key), std::forward<T>(t));
         return {std::prev(this->end()), true};
     }
 

tests/src/unit-ordered_json.cpp

@@ -196,44 +196,3 @@ TEST_CASE("regression test - diff() must account for ordered_json member order")
         CHECK(a.patch(p) == b);
     }
 }
-
-TEST_CASE("regression test for issue #5673 - ordered_json::emplace with a non-rvalue value")
-{
-    SECTION("lvalue value")
-    {
-        ordered_json oj = ordered_json::object();
-        ordered_json value = 1;
-        auto res = oj.emplace("a", value);
-        CHECK(res.second == true);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-
-    SECTION("const lvalue value")
-    {
-        ordered_json oj = ordered_json::object();
-        const ordered_json value = 1;
-        auto res = oj.emplace("a", value);
-        CHECK(res.second == true);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-
-    SECTION("rvalue value")
-    {
-        ordered_json oj = ordered_json::object();
-        auto res = oj.emplace("a", ordered_json(1));
-        CHECK(res.second == true);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-
-    SECTION("existing key is not overwritten (std::map-compatible semantics)")
-    {
-        ordered_json oj = ordered_json::object();
-        ordered_json value = 1;
-        oj.emplace("a", value);
-
-        ordered_json other_value = 2;
-        auto res = oj.emplace("a", other_value);
-        CHECK(res.second == false);
-        CHECK(oj.dump() == "{\"a\":1}");
-    }
-}

tests/src/unit-ordered_map.cpp

@@ -312,77 +312,4 @@ TEST_CASE("ordered_map")
             CHECK(om.size() == 4);
         }
     }
-
-    SECTION("emplace")
-    {
-        // regression test for issue #5673: the mapped-value parameter must
-        // accept lvalues and const lvalues, not just rvalues
-        ordered_map<std::string, std::string> om;
-        om["eins"] = "one";
-        om["zwei"] = "two";
-        om["drei"] = "three";
-
-        SECTION("with T&& (rvalue)")
-        {
-            auto res1 = om.emplace("eins", std::string("1"));
-            CHECK(res1.first == om.begin());
-            CHECK(res1.second == false);
-            CHECK(om.size() == 3);
-            CHECK(om.at("eins") == "one"); // existing key is not overwritten
-
-            auto res4 = om.emplace("vier", std::string("four"));
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-        }
-
-        SECTION("with T& (lvalue)")
-        {
-            std::string one = "1";
-            std::string four = "four";
-
-            auto res1 = om.emplace("eins", one);
-            CHECK(res1.first == om.begin());
-            CHECK(res1.second == false);
-            CHECK(om.size() == 3);
-            CHECK(om.at("eins") == "one"); // existing key is not overwritten
-
-            auto res4 = om.emplace("vier", four);
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-            CHECK(four == "four"); // source was copied, not moved from
-        }
-
-        SECTION("with const T&")
-        {
-            const std::string one = "1";
-            const std::string four = "four";
-
-            auto res1 = om.emplace("eins", one);
-            CHECK(res1.first == om.begin());
-            CHECK(res1.second == false);
-            CHECK(om.size() == 3);
-
-            auto res4 = om.emplace("vier", four);
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-        }
-
-        SECTION("with key of key_type (non-template overload)")
-        {
-            const std::string key_vier{"vier"};
-            std::string four = "four";
-
-            auto res4 = om.emplace(key_vier, four);
-            CHECK(res4.first == om.begin() + 3);
-            CHECK(res4.second == true);
-            CHECK(om.size() == 4);
-            CHECK(om.at("vier") == "four");
-        }
-    }
 }

tools/amalgamate/CHANGES.md

@@ -8,3 +8,6 @@ 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,6 +57,11 @@ 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,6 +62,10 @@ 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:
@@ -220,11 +224,14 @@ class TranslationUnit(object):
         while include_match:
             if not _is_within(include_match, skippable_contexts):
                 include_path = include_match.group("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))
+                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))
 
             include_match = self.include_pattern.search(self.content,
                                                         include_match.end())
@@ -235,6 +242,17 @@ 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

@@ -0,0 +1,9 @@
+{
+	"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"]
+}