From 43afb5bebc25764e3fb4fe885a60bfd69aa5f1b0 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 28 Aug 2026 18:17:15 +0000 Subject: [PATCH] Do not instantiate a hash map with an incomplete basic_json in the tests object_t is probed for key_compare inside the definition of basic_json, so it is instantiated while basic_json is still incomplete. Whether a hash map survives that depends on the standard library: libstdc++ 9 needs the size of the mapped type to instantiate std::unordered_map's node type and rejects the adapter, which broke the GCC 9 builds. The test now derives its no-key_compare object type from std::map -- which does cope -- and shadows the inherited key_compare member type with an entity that is not a type, so the library's probe finds none, exactly as for a hash map. The unflatten() order-independence checks in unit-json_pointer already cover the behaviour that the unordered object type was there for. The limitation is documented for std::unordered_map. Also address two Clang-Tidy findings the earlier commits introduced: erase_from_object() declares its iterator with auto, and at(size_type) checks the type first and then falls through to the return instead of throwing from an else branch. Signed-off-by: Niels Lohmann --- .../features/types/template_parameters.md | 7 ++- include/nlohmann/json.hpp | 36 ++++++------ single_include/nlohmann/json.hpp | 36 ++++++------ tests/src/unit-custom-object-type.cpp | 56 ++++++++++--------- 4 files changed, 71 insertions(+), 64 deletions(-) diff --git a/docs/mkdocs/docs/features/types/template_parameters.md b/docs/mkdocs/docs/features/types/template_parameters.md index 032bad35a..610c630b0 100644 --- a/docs/mkdocs/docs/features/types/template_parameters.md +++ b/docs/mkdocs/docs/features/types/template_parameters.md @@ -148,6 +148,11 @@ struct unordered_map_object using unordered_json = nlohmann::basic_json; ``` +Whether `#!cpp std::unordered_map` can be instantiated at all depends on the standard library: `object_t` is formed +while `basic_json` is still incomplete (see the warning above), and libstdc++ 9 needs the size of the mapped type to +instantiate the hash map's node type, so the adapter does not compile there. Newer libstdc++ versions, and the hash +maps listed below, do not have that problem. + The adapter above works verbatim for Abseil's, Boost's, `phmap`'s and `gtl`'s hash maps, which all place the hash function third and take a `#!cpp std::pair` allocator fifth. Two need a different adapter: @@ -208,7 +213,7 @@ The library does not sort or de-duplicate keys itself; the behavior described in | [`nlohmann::ordered_map`](../../api/ordered_map.md) | used by [`ordered_json`](../../api/ordered_json.md); keeps insertion order | | [`nlohmann::fifo_map`](https://github.com/nlohmann/fifo_map) | keeps insertion order; adapter puts `fifo_map_compare` in the comparator slot | | `boost::container::map`, `boost::container::flat_map` | no adapter needed | -| `#!cpp std::unordered_map` | through the adapter above | +| `#!cpp std::unordered_map` | through the adapter above; not with libstdc++ 9, see the note | | `boost::unordered_map`, `boost::unordered_flat_map`, `boost::unordered_node_map` | through the adapter above | | `absl::flat_hash_map`, `absl::node_hash_map` | through the adapter above; `flat_hash_map` moves mapped values on rehash | | `phmap::flat_hash_map`, `phmap::node_hash_map`, `gtl::flat_hash_map` | through the adapter above | diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3e94df97e..fe57b2766 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -798,7 +798,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::is_void().erase(std::declval()))>::value, int > = 0 > typename object_t::iterator erase_from_object(It pos) { - typename object_t::iterator next = std::next(pos); + auto next = std::next(pos); m_data.m_value.object->erase(pos); return next; } @@ -2063,18 +2063,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec reference at(size_type idx) { // at only works for arrays - if (JSON_HEDLEY_LIKELY(is_array())) - { - if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) - { - JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); - } - return set_parent((*m_data.m_value.array)[idx]); - } - else + if (JSON_HEDLEY_UNLIKELY(!is_array())) { JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); } + + if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) + { + JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); + } + + return set_parent((*m_data.m_value.array)[idx]); } /// @brief access specified array element with bounds checking @@ -2082,18 +2081,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const_reference at(size_type idx) const { // at only works for arrays - if (JSON_HEDLEY_LIKELY(is_array())) - { - if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) - { - JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); - } - return (*m_data.m_value.array)[idx]; - } - else + if (JSON_HEDLEY_UNLIKELY(!is_array())) { JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); } + + if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) + { + JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); + } + + return (*m_data.m_value.array)[idx]; } /// @brief access specified object element with bounds checking diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 50d8e80a5..0543c68ac 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -22264,7 +22264,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec std::is_void().erase(std::declval()))>::value, int > = 0 > typename object_t::iterator erase_from_object(It pos) { - typename object_t::iterator next = std::next(pos); + auto next = std::next(pos); m_data.m_value.object->erase(pos); return next; } @@ -23529,18 +23529,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec reference at(size_type idx) { // at only works for arrays - if (JSON_HEDLEY_LIKELY(is_array())) - { - if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) - { - JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); - } - return set_parent((*m_data.m_value.array)[idx]); - } - else + if (JSON_HEDLEY_UNLIKELY(!is_array())) { JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); } + + if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) + { + JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); + } + + return set_parent((*m_data.m_value.array)[idx]); } /// @brief access specified array element with bounds checking @@ -23548,18 +23547,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const_reference at(size_type idx) const { // at only works for arrays - if (JSON_HEDLEY_LIKELY(is_array())) - { - if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) - { - JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); - } - return (*m_data.m_value.array)[idx]; - } - else + if (JSON_HEDLEY_UNLIKELY(!is_array())) { JSON_THROW(type_error::create(304, detail::concat("cannot use at() with ", type_name()), this)); } + + if (JSON_HEDLEY_UNLIKELY(idx >= m_data.m_value.array->size())) + { + JSON_THROW(out_of_range::create(401, detail::concat("array index ", std::to_string(idx), " is out of range"), this)); + } + + return (*m_data.m_value.array)[idx]; } /// @brief access specified object element with bounds checking diff --git a/tests/src/unit-custom-object-type.cpp b/tests/src/unit-custom-object-type.cpp index 3f37ed73c..c36dafff1 100644 --- a/tests/src/unit-custom-object-type.cpp +++ b/tests/src/unit-custom-object-type.cpp @@ -11,29 +11,36 @@ #include #include -#include #include #include #include -#include #include #include namespace { -// An ObjectType that does *not* define a key_compare member type. It adapts -// std::unordered_map to the template argument order expected by basic_json, -// where the third argument is a comparator rather than a hash function. -template -struct unordered_map_object - : std::unordered_map, std::equal_to, Allocator> +// An ObjectType that does *not* define a key_compare member type, which is +// what every hash map looks like to the library. +// +// A hash map is deliberately not used here: object_t is probed for +// key_compare inside the definition of basic_json, that is, while basic_json +// is still an incomplete type, and whether a hash map can be instantiated +// with an incomplete mapped type depends on the standard library (libstdc++ 9 +// needs the size of the mapped type for its node type and rejects it). So the +// object type is built from std::map, and the inherited key_compare member +// type is shadowed by an entity that is not a type -- the library's probe +// then finds no type, exactly as for a hash map. +template +struct no_key_compare_map : std::map { - using base_t = std::unordered_map, std::equal_to, Allocator>; + using base_t = std::map; using base_t::base_t; + + enum { key_compare }; // shadows base_t::key_compare, which is a type }; -using unordered_json = nlohmann::basic_json; +using no_key_compare_json = nlohmann::basic_json; // An ObjectType whose erase(iterator) returns void rather than the following // iterator, as for instance Abseil's hash maps do @@ -109,8 +116,8 @@ TEST_CASE("object type without key_compare") { SECTION("object_comparator_t falls back to default_object_comparator_t") { - CHECK(std::is_same < unordered_json::object_comparator_t, - unordered_json::default_object_comparator_t >::value); + CHECK(std::is_same < no_key_compare_json::object_comparator_t, + no_key_compare_json::default_object_comparator_t >::value); } SECTION("object types defining key_compare are unaffected") @@ -123,7 +130,7 @@ TEST_CASE("object type without key_compare") SECTION("creating and accessing values") { - unordered_json j; + no_key_compare_json j; j["one"] = 1; j["two"] = "zwei"; j["three"]["nested"] = true; @@ -142,39 +149,38 @@ TEST_CASE("object type without key_compare") SECTION("serialization and deserialization") { - const auto j = unordered_json::parse(R"({"a":[1,2,3],"b":{"c":null}})"); + const auto j = no_key_compare_json::parse(R"({"a":[1,2,3],"b":{"c":null}})"); CHECK(j["a"].size() == 3); CHECK(j["a"][2] == 3); CHECK(j["b"]["c"].is_null()); - CHECK(unordered_json::parse(j.dump()) == j); + CHECK(no_key_compare_json::parse(j.dump()) == j); } SECTION("binary formats") { - const auto j = unordered_json::parse(R"({"a":[1,2,3],"b":"x"})"); - CHECK(unordered_json::from_cbor(unordered_json::to_cbor(j)) == j); - CHECK(unordered_json::from_msgpack(unordered_json::to_msgpack(j)) == j); + const auto j = no_key_compare_json::parse(R"({"a":[1,2,3],"b":"x"})"); + CHECK(no_key_compare_json::from_cbor(no_key_compare_json::to_cbor(j)) == j); + CHECK(no_key_compare_json::from_msgpack(no_key_compare_json::to_msgpack(j)) == j); } - SECTION("flatten and unflatten do not depend on the iteration order") + SECTION("flatten and unflatten") { - // the flattened object is iterated in an unspecified order, so - // unflatten() must not decide between array and object based on - // whichever reference token it happens to see first - const auto j = unordered_json::parse( + // "o" has a key that looks like an array index, so unflatten() must + // not turn it into an array + const auto j = no_key_compare_json::parse( R"({"c":[1,2,3],"d":{"e":"s"},"n":[[0,1],[2]],"o":{"2":"x"}})"); CHECK(j.flatten().unflatten() == j); } SECTION("conversion to and from nlohmann::json") { - const auto j = unordered_json::parse(R"({"a":1,"b":[true,null]})"); + const auto j = no_key_compare_json::parse(R"({"a":1,"b":[true,null]})"); const nlohmann::json converted(j); CHECK(converted.is_object()); CHECK(converted["a"] == 1); CHECK(converted["b"][0] == true); CHECK(converted["b"][1].is_null()); - CHECK(unordered_json(converted) == j); + CHECK(no_key_compare_json(converted) == j); } }