diff --git a/include/nlohmann/detail/meta/type_traits.hpp b/include/nlohmann/detail/meta/type_traits.hpp index 6f8bf2a3d..bc253ca9b 100644 --- a/include/nlohmann/detail/meta/type_traits.hpp +++ b/include/nlohmann/detail/meta/type_traits.hpp @@ -189,6 +189,37 @@ struct actual_object_comparator template using actual_object_comparator_t = typename actual_object_comparator::type; +template +using detect_key_comp = decltype(std::declval().key_comp()); + +// whether ObjectType can be constructed from a pair of Iterator together with +// a copy of its own comparator, the way std::map can: it needs a nested +// key_compare, a const key_comp() convertible to it, and a matching +// (Iterator, Iterator, const key_compare&) constructor. +// +// used to preserve a stateful comparator when a copy is built from a range +// past the iterative deep copy's nesting bound (see copy_object_level); an +// object type that does not satisfy this, such as nlohmann::ordered_map +// (which has key_compare for its std::map-like interface, but no key_comp()), +// keeps default-constructing its comparator, just as it always has +template +struct is_comparator_constructible_object_type_impl : std::false_type {}; + +template +struct is_comparator_constructible_object_type_impl < + ObjectType, Iterator, enable_if_t::value >> +{ + using key_compare = typename ObjectType::key_compare; + + static constexpr bool value = + is_detected_convertible::value && + std::is_constructible::value; +}; + +template +struct is_comparator_constructible_object_type + : is_comparator_constructible_object_type_impl {}; + ///////////////// // char_traits // ///////////////// diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 500fcddf2..0c4df6f0a 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -1127,6 +1127,27 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } } + /// @brief create the object type from a range, preserving @a src_object's + /// comparator when the object type supports it + /// Enabled for object types that provide a key_comp() and a matching + /// range-plus-comparator constructor, such as std::map. Other object + /// types, such as nlohmann::ordered_map, fall back to the plain range + /// constructor and default-construct their comparator, just as they + /// always have (@ref detail::is_comparator_constructible_object_type). + template < typename Iterator, detail::enable_if_t < + detail::is_comparator_constructible_object_type::value, int > = 0 > + static object_t* create_object_with_comparator(const object_t& src_object, Iterator first, Iterator last) + { + return create(first, last, src_object.key_comp()); + } + + template < typename Iterator, detail::enable_if_t < + !detail::is_comparator_constructible_object_type::value, int > = 0 > + static object_t* create_object_with_comparator(const object_t& /*src_object*/, Iterator first, Iterator last) + { + return create(first, last); + } + /// @brief create the copy of the object @a src in @a dst /// @note structured values are appended to @a worklist instead static void copy_object_level(const basic_json& src, basic_json& dst, @@ -1144,7 +1165,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec scratch.emplace_back(element.first, basic_json()); } - dst.m_data.m_value.object = create(std::make_move_iterator(scratch.begin()), + dst.m_data.m_value.object = create_object_with_comparator(src_object, + std::make_move_iterator(scratch.begin()), std::make_move_iterator(scratch.end())); scratch.clear(); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 576498738..7ad15523f 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -4199,6 +4199,37 @@ struct actual_object_comparator template using actual_object_comparator_t = typename actual_object_comparator::type; +template +using detect_key_comp = decltype(std::declval().key_comp()); + +// whether ObjectType can be constructed from a pair of Iterator together with +// a copy of its own comparator, the way std::map can: it needs a nested +// key_compare, a const key_comp() convertible to it, and a matching +// (Iterator, Iterator, const key_compare&) constructor. +// +// used to preserve a stateful comparator when a copy is built from a range +// past the iterative deep copy's nesting bound (see copy_object_level); an +// object type that does not satisfy this, such as nlohmann::ordered_map +// (which has key_compare for its std::map-like interface, but no key_comp()), +// keeps default-constructing its comparator, just as it always has +template +struct is_comparator_constructible_object_type_impl : std::false_type {}; + +template +struct is_comparator_constructible_object_type_impl < + ObjectType, Iterator, enable_if_t::value >> +{ + using key_compare = typename ObjectType::key_compare; + + static constexpr bool value = + is_detected_convertible::value && + std::is_constructible::value; +}; + +template +struct is_comparator_constructible_object_type + : is_comparator_constructible_object_type_impl {}; + ///////////////// // char_traits // ///////////////// @@ -27208,6 +27239,27 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } } + /// @brief create the object type from a range, preserving @a src_object's + /// comparator when the object type supports it + /// Enabled for object types that provide a key_comp() and a matching + /// range-plus-comparator constructor, such as std::map. Other object + /// types, such as nlohmann::ordered_map, fall back to the plain range + /// constructor and default-construct their comparator, just as they + /// always have (@ref detail::is_comparator_constructible_object_type). + template < typename Iterator, detail::enable_if_t < + detail::is_comparator_constructible_object_type::value, int > = 0 > + static object_t* create_object_with_comparator(const object_t& src_object, Iterator first, Iterator last) + { + return create(first, last, src_object.key_comp()); + } + + template < typename Iterator, detail::enable_if_t < + !detail::is_comparator_constructible_object_type::value, int > = 0 > + static object_t* create_object_with_comparator(const object_t& /*src_object*/, Iterator first, Iterator last) + { + return create(first, last); + } + /// @brief create the copy of the object @a src in @a dst /// @note structured values are appended to @a worklist instead static void copy_object_level(const basic_json& src, basic_json& dst, @@ -27225,7 +27277,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec scratch.emplace_back(element.first, basic_json()); } - dst.m_data.m_value.object = create(std::make_move_iterator(scratch.begin()), + dst.m_data.m_value.object = create_object_with_comparator(src_object, + std::make_move_iterator(scratch.begin()), std::make_move_iterator(scratch.end())); scratch.clear(); diff --git a/tests/src/unit-comparison.cpp b/tests/src/unit-comparison.cpp index 69c0103c9..641f116ed 100644 --- a/tests/src/unit-comparison.cpp +++ b/tests/src/unit-comparison.cpp @@ -17,6 +17,7 @@ #include +#include #include #include #include @@ -825,6 +826,46 @@ Json nest(Json j, const std::size_t depth) } return j; } + +// a std::map comparator with state: case-insensitive, unless constructed +// case-sensitive. Used to check that copying an object copies the original's +// comparator rather than default-constructing a new one (see #5649). +struct key_case_less +{ + key_case_less() = default; + explicit key_case_less(const bool cs) noexcept : case_sensitive(cs) {} + + bool operator()(const std::string& a, const std::string& b) const + { + if (case_sensitive) + { + return a < b; + } + return std::lexicographical_compare(a.begin(), a.end(), b.begin(), b.end(), + [](unsigned char x, unsigned char y) + { + return std::tolower(x) < std::tolower(y); + }); + } + + bool case_sensitive = false; +}; + +template +using key_case_map = std::map; +using key_case_json = nlohmann::basic_json; + +// the innermost value of a chain of single-element arrays +template +const Json& innermost(const Json& j) +{ + const Json* p = &j; + while (p->is_array()) + { + p = &(*p)[0]; + } + return *p; +} } // namespace TEST_CASE("equality of objects whose entries have no fixed order") @@ -872,6 +913,47 @@ TEST_CASE("equality of objects whose entries have no fixed order") } } +TEST_CASE("copying an object preserves its comparator's state") +{ + // Past the iterative deep copy's nesting bound, an object copy used to be + // built with a default-constructed comparator instead of a copy of the + // original's. For an object type whose comparator carries state - here, a + // std::map that compares keys case-sensitively only when created that way + // - this reordered the copy's keys and could even drop entries that the + // original's comparator kept distinct (see #5649). + key_case_json object = key_case_json::object_t(key_case_less(true)); // case-sensitive + object["b"] = 1; + object["B"] = 2; + object["a"] = 3; + REQUIRE(object.dump() == R"({"B":2,"a":3,"b":1})"); + + for (const std::size_t depth : std::vector {0, 127, 128, 200}) + { + CAPTURE(depth); + + key_case_json original = object; + for (std::size_t i = 0; i < depth; ++i) + { + original = key_case_json::array({std::move(original)}); + } + + { + const key_case_json copy = original; // NOLINT(performance-unnecessary-copy-initialization) + CHECK(innermost(copy).size() == 3); + CHECK(innermost(copy).dump() == R"({"B":2,"a":3,"b":1})"); + CHECK(copy == original); + } + + { + key_case_json copy = key_case_json::array(); + copy = original; + CHECK(innermost(copy).size() == 3); + CHECK(innermost(copy).dump() == R"({"B":2,"a":3,"b":1})"); + CHECK(copy == original); + } + } +} + TEST_CASE("containers are compared element by element") { // Containers nested deeper than a bound are compared without the call