Merge 803c16e into 6ee9e5f

nlohmann · Jul 3, 2020 · 5bb81a5 · 5bb81a5
2 parents 6ee9e5f + 803c16e
commit 5bb81a5
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 25 deletions.
diff --git a/README.md b/README.md
@@ -1527,7 +1527,7 @@ This library will not support comments in the future. If you wish to use comment
 
 ### Order of object keys
 
-By default, the library does not preserve the **insertion order of object elements**. This is standards-compliant, as the [JSON standard](https://tools.ietf.org/html/rfc8259.html) defines objects as "an unordered collection of zero or more name/value pairs". If you do want to preserve the insertion order, you can specialize the object type with containers like [`tsl::ordered_map`](https://github.com/Tessil/ordered-map) ([integration](https://github.com/nlohmann/json/issues/546#issuecomment-304447518)) or [`nlohmann::fifo_map`](https://github.com/nlohmann/fifo_map) ([integration](https://github.com/nlohmann/json/issues/485#issuecomment-333652309)).
+By default, the library does not preserve the **insertion order of object elements**. This is standards-compliant, as the [JSON standard](https://tools.ietf.org/html/rfc8259.html) defines objects as "an unordered collection of zero or more name/value pairs". If you do want to preserve the insertion order, you can try the new [`nlohmann::ordered_json`](https://github.com/nlohmann/json/issues/2179) specialization, or use a more sophisticated ordered map like [`tsl::ordered_map`](https://github.com/Tessil/ordered-map) ([integration](https://github.com/nlohmann/json/issues/546#issuecomment-304447518)) or [`nlohmann::fifo_map`](https://github.com/nlohmann/fifo_map) ([integration](https://github.com/nlohmann/json/issues/485#issuecomment-333652309)).
 
 ### Memory Release
 

diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp
@@ -496,8 +496,8 @@ class basic_json
     using object_t = ObjectType<StringType,
           basic_json,
           object_comparator_t,
-          AllocatorType<std::pair<const StringType,
-          basic_json>>>;
+          // Note: this forces object_t::value_type to match std::map's
+          AllocatorType<std::pair<const StringType, basic_json>>>;
 
     /*!
     @brief a type for an array

diff --git a/include/nlohmann/json_fwd.hpp b/include/nlohmann/json_fwd.hpp
@@ -61,7 +61,7 @@ uses the standard template types.
 */
 using json = basic_json<>;
 
-template<class Key, class T, class IgnoredLess, class Allocator, class Container>
+template<class Key, class T, class IgnoredLess, class Allocator>
 struct ordered_map;
 
 /*!

diff --git a/include/nlohmann/ordered_map.hpp b/include/nlohmann/ordered_map.hpp
@@ -11,17 +11,26 @@ namespace nlohmann
 /// ordered_map: a minimal map-like container that preserves insertion order
 /// for use within nlohmann::basic_json<ordered_map>
 template <class Key, class T, class IgnoredLess = std::less<Key>,
-          class Allocator = std::allocator<std::pair<Key, T>>,
-          class Container = std::vector<std::pair<Key, T>, Allocator>>
-struct ordered_map : Container
+          class Allocator = std::allocator<std::pair<const Key, T>>>
+struct ordered_map : std::vector<std::pair<const Key, T>, Allocator>
 {
     using key_type = Key;
     using mapped_type = T;
-    using value_type = typename Container::value_type;
-    using size_type = typename Container::size_type;
-    using Container::Container;
+    using Container = std::vector<std::pair<const Key, T>, Allocator>;
+    using typename Container::iterator;
+    using typename Container::size_type;
+    using typename Container::value_type;
 
-    std::pair<typename Container::iterator, bool> emplace(key_type&& key, T&& t)
+    // Explicit constructors instead of `using Container::Container`
+    // otherwise older compilers choke on it (GCC <= 5.5, xcode <= 9.4)
+    ordered_map(const Allocator& alloc = Allocator()) : Container{alloc} {}
+    template <class It>
+    ordered_map(It first, It last, const Allocator& alloc = Allocator())
+        : Container{first, last, alloc} {}
+    ordered_map(std::initializer_list<T> init, const Allocator& alloc = Allocator() )
+        : Container{init, alloc} {}
+
+    std::pair<iterator, bool> emplace(key_type&& key, T&& t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -45,7 +54,14 @@ struct ordered_map : Container
         {
             if (it->first == key)
             {
-                Container::erase(it);
+                // Since we cannot move const Keys, re-construct them in place
+                for (auto next = it; ++next != this->end(); ++it)
+                {
+                    // *it = std::move(*next); // deleted
+                    it->~value_type(); // Destroy but keep allocation
+                    new (&*it) value_type{std::move(*next)};
+                }
+                Container::pop_back();
                 return 1;
             }
         }

diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
@@ -2773,7 +2773,7 @@ uses the standard template types.
 */
 using json = basic_json<>;
 
-template<class Key, class T, class IgnoredLess, class Allocator, class Container>
+template<class Key, class T, class IgnoredLess, class Allocator>
 struct ordered_map;
 
 /*!
@@ -15880,17 +15880,26 @@ namespace nlohmann
 /// ordered_map: a minimal map-like container that preserves insertion order
 /// for use within nlohmann::basic_json<ordered_map>
 template <class Key, class T, class IgnoredLess = std::less<Key>,
-          class Allocator = std::allocator<std::pair<Key, T>>,
-          class Container = std::vector<std::pair<Key, T>, Allocator>>
-struct ordered_map : Container
+          class Allocator = std::allocator<std::pair<const Key, T>>>
+struct ordered_map : std::vector<std::pair<const Key, T>, Allocator>
 {
     using key_type = Key;
     using mapped_type = T;
-    using value_type = typename Container::value_type;
-    using size_type = typename Container::size_type;
-    using Container::Container;
-
-    std::pair<typename Container::iterator, bool> emplace(key_type&& key, T&& t)
+    using Container = std::vector<std::pair<const Key, T>, Allocator>;
+    using typename Container::iterator;
+    using typename Container::size_type;
+    using typename Container::value_type;
+
+    // Explicit constructors instead of `using Container::Container`
+    // otherwise older compilers choke on it (GCC <= 5.5, xcode <= 9.4)
+    ordered_map(const Allocator& alloc = Allocator()) : Container{alloc} {}
+    template <class It>
+    ordered_map(It first, It last, const Allocator& alloc = Allocator())
+        : Container{first, last, alloc} {}
+    ordered_map(std::initializer_list<T> init, const Allocator& alloc = Allocator() )
+        : Container{init, alloc} {}
+
+    std::pair<iterator, bool> emplace(key_type&& key, T&& t)
     {
         for (auto it = this->begin(); it != this->end(); ++it)
         {
@@ -15914,7 +15923,14 @@ struct ordered_map : Container
         {
             if (it->first == key)
             {
-                Container::erase(it);
+                // Since we cannot move const Keys, re-construct them in place
+                for (auto next = it; ++next != this->end(); ++it)
+                {
+                    // *it = std::move(*next); // deleted
+                    it->~value_type(); // Destroy but keep allocation
+                    new (&*it) value_type{std::move(*next)};
+                }
+                Container::pop_back();
                 return 1;
             }
         }
@@ -16049,7 +16065,7 @@ class basic_json
         InputAdapterType adapter,
         detail::parser_callback_t<basic_json>cb = nullptr,
         bool allow_exceptions = true
-    )
+                                 )
     {
         return ::nlohmann::detail::parser<basic_json, InputAdapterType>(std::move(adapter), std::move(cb), allow_exceptions);
     }
@@ -16347,8 +16363,8 @@ class basic_json
     using object_t = ObjectType<StringType,
           basic_json,
           object_comparator_t,
-          AllocatorType<std::pair<const StringType,
-          basic_json>>>;
+          // Note: this forces object_t::value_type to match std::map's
+          AllocatorType<std::pair<const StringType, basic_json>>>;
 
     /*!
     @brief a type for an array

diff --git a/test/src/unit-ordered_json.cpp b/test/src/unit-ordered_json.cpp
@@ -58,4 +58,15 @@ TEST_CASE("ordered_json")
 
     CHECK(j.dump() == "{\"element2\":2,\"element3\":3}");
     CHECK(oj.dump() == "{\"element3\":3,\"element2\":2}");
+
+    // There are no dup keys cause constructor calls emplace...
+    json multi {{"z", 1}, {"m", 2}, {"m", 3}, {"y", 4}, {"m", 5}};
+    CHECK(multi.size() == 3);
+    CHECK(multi.dump() == "{\"m\":2,\"y\":4,\"z\":1}");
+
+    ordered_json multi_ordered {{"z", 1}, {"m", 2}, {"m", 3}, {"y", 4}, {"m", 5}};
+    CHECK(multi_ordered.size() == 3);
+    CHECK(multi_ordered.dump() == "{\"z\":1,\"m\":2,\"y\":4}");
+    CHECK(multi_ordered.erase("m") == 1);
+    CHECK(multi_ordered.dump() == "{\"z\":1,\"y\":4}");
 }