diff --git a/docs/mkdocs/docs/api/basic_json/operator_spaceship.md b/docs/mkdocs/docs/api/basic_json/operator_spaceship.md index 9e91d0d2d..2c2a291cf 100644 --- a/docs/mkdocs/docs/api/basic_json/operator_spaceship.md +++ b/docs/mkdocs/docs/api/basic_json/operator_spaceship.md @@ -96,5 +96,8 @@ Linear. ## Version history -1. Added in version 3.11.0. +1. Added in version 3.11.0. Fixed in version 3.13.0 to keep the result independent of nesting depth: before, a pair + of binary values with the same bytes but a different subtype - unequal by `operator==`, yet equivalent by + `operator<=>` - could end an ordered comparison as `unordered` only past 128 levels of nesting, or at every depth + with [`JSON_NO_THREAD_LOCAL`](../macros/json_no_thread_local.md) defined. 2. Added in version 3.11.0. diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 500fcddf2..25dd3d2e2 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -1307,15 +1307,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec */ template static compare_result compare_leaves(const_reference lhs, const_reference rhs) noexcept + { + return compare_leaves(lhs, rhs, std::integral_constant {}); + } + + /// @brief compare two leaves that are only being checked for equality + static compare_result compare_leaves(const_reference lhs, const_reference rhs, std::false_type /*ordered*/) noexcept { if (lhs == rhs) { return compare_result::equal; } - return order_leaves(lhs, rhs, std::integral_constant {}); + return order_leaves(lhs, rhs, std::false_type {}); } +#if JSON_HAS_THREE_WAY_COMPARISON + /*! + @brief compare two leaves that are being ordered, for operator<=> + + Reached only from operator<=>, so the leaves must be classified exactly + as operator<=> classifies them - which is not the same as asking + == and then order_leaves(), the way the other overload does it. The two + disagree on a binary value: == also compares the subtype, but <=> compares + only the bytes, through std::vector::operator<=>. Using <=> + itself here keeps a leaf pair classified the same way regardless of how + deep it is nested - == first would again call operator<=> a level down + through order_leaves(), but call it after a mismatching == already ended + the comparison for a pair that <=> alone would still call equivalent. + */ + static compare_result compare_leaves(const_reference lhs, const_reference rhs, std::true_type /*ordered*/) noexcept + { + const std::partial_ordering order = lhs <=> rhs; // *NOPAD* + if (order == 0) + { + return compare_result::equal; + } + if (order < 0) + { + return compare_result::less; + } + if (order > 0) + { + return compare_result::greater; + } + return compare_result::unordered; + } +#else + /// @brief compare two leaves that are being ordered, for operator< + static compare_result compare_leaves(const_reference lhs, const_reference rhs, std::true_type /*ordered*/) noexcept + { + if (lhs == rhs) + { + return compare_result::equal; + } + + return order_leaves(lhs, rhs, std::true_type {}); + } +#endif + /*! @brief compare two object keys diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 576498738..0630f38d8 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -27388,15 +27388,65 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec */ template static compare_result compare_leaves(const_reference lhs, const_reference rhs) noexcept + { + return compare_leaves(lhs, rhs, std::integral_constant {}); + } + + /// @brief compare two leaves that are only being checked for equality + static compare_result compare_leaves(const_reference lhs, const_reference rhs, std::false_type /*ordered*/) noexcept { if (lhs == rhs) { return compare_result::equal; } - return order_leaves(lhs, rhs, std::integral_constant {}); + return order_leaves(lhs, rhs, std::false_type {}); } +#if JSON_HAS_THREE_WAY_COMPARISON + /*! + @brief compare two leaves that are being ordered, for operator<=> + + Reached only from operator<=>, so the leaves must be classified exactly + as operator<=> classifies them - which is not the same as asking + == and then order_leaves(), the way the other overload does it. The two + disagree on a binary value: == also compares the subtype, but <=> compares + only the bytes, through std::vector::operator<=>. Using <=> + itself here keeps a leaf pair classified the same way regardless of how + deep it is nested - == first would again call operator<=> a level down + through order_leaves(), but call it after a mismatching == already ended + the comparison for a pair that <=> alone would still call equivalent. + */ + static compare_result compare_leaves(const_reference lhs, const_reference rhs, std::true_type /*ordered*/) noexcept + { + const std::partial_ordering order = lhs <=> rhs; // *NOPAD* + if (order == 0) + { + return compare_result::equal; + } + if (order < 0) + { + return compare_result::less; + } + if (order > 0) + { + return compare_result::greater; + } + return compare_result::unordered; + } +#else + /// @brief compare two leaves that are being ordered, for operator< + static compare_result compare_leaves(const_reference lhs, const_reference rhs, std::true_type /*ordered*/) noexcept + { + if (lhs == rhs) + { + return compare_result::equal; + } + + return order_leaves(lhs, rhs, std::true_type {}); + } +#endif + /*! @brief compare two object keys diff --git a/tests/src/unit-comparison.cpp b/tests/src/unit-comparison.cpp index 69c0103c9..d70ae2134 100644 --- a/tests/src/unit-comparison.cpp +++ b/tests/src/unit-comparison.cpp @@ -952,3 +952,51 @@ TEST_CASE("containers are compared element by element") } } } + +#if JSON_HAS_THREE_WAY_COMPARISON +// JSON_HAS_CPP_20 (do not remove; see note at top of file) +TEST_CASE("operator<=> of binary values with a different subtype does not depend on nesting depth") +{ + // #5654: std::vector::operator<=>, which the binary type's + // own operator<=> uses, ignores the subtype that operator== checks. So a + // pair of binary values with the same bytes but a different subtype is + // unequal, yet <=>-equivalent - the same inconsistency between == and <=> + // that a NaN has. Within the nesting bound, an array compares itself + // with std::vector's own operator<=>, which treats an equivalent pair as + // undecided and lets the next element decide, same as + // std::lexicographical_compare_three_way does. Past the bound, + // compare_iteratively() takes over and must classify the pair the + // same way, or the result of operator<=> - and of <, which C++20 derives + // from it - depends on how deeply the values are nested. + const json a = json::array({json::binary({1}, 1), 1}); + const json b = json::array({json::binary({1}, 2), 2}); + + // the root inconsistency: unequal, yet <=>-equivalent + CHECK_FALSE(a[0] == b[0]); + CHECK((a[0] <=> b[0]) == std::partial_ordering::equivalent); // *NOPAD* + + const auto deep = [](const json & j, const std::size_t depth) + { + json result = j; + for (std::size_t i = 0; i < depth; ++i) + { + result = json::array({std::move(result)}); + } + return result; + }; + + // 127 levels stay within nesting_depth_limit() (128); 128 and 200 do not, + // and must still agree with the levels that do + for (const std::size_t depth : std::vector {0, 127, 128, 200}) + { + CAPTURE(depth); + const json x = deep(a, depth); + const json y = deep(b, depth); + CHECK((x <=> y) == std::partial_ordering::less); // *NOPAD* + CHECK((y <=> x) == std::partial_ordering::greater); // *NOPAD* + CHECK(x < y); + CHECK(y > x); + CHECK_FALSE(y < x); + } +} +#endif