From 137a40b9aa3dff087f080c3f6ed52159e24b4b2f Mon Sep 17 00:00:00 2001 From: Qatadaha Bin Matloob <79017227+qatcod@users.noreply.github.com> Date: Sat, 5 Sep 2026 12:43:07 +0500 Subject: [PATCH] Compare integers with floats exactly instead of widening the integer (#5459) The mixed number arms of JSON_IMPLEMENT_OPERATOR cast the integer to number_float_t before comparing. Past the float's mantissa that cast is lossy: 2^63-2 and 2^63-1 both round to 2^63, so each compares equal to that float while differing from each other. Equality is therefore intransitive and the ordering is not a strict weak ordering, which makes std::sort over such values, or using them as keys in std::set or std::map, undefined behavior. Compare the two exactly instead. The integer's range is a power of two the float represents exactly, so a float outside it is ordered by magnitude alone; inside it, truncating the float is exact, and the integer parts and then any fractional part decide. The helper hands back a pair whose comparison with the original operator reproduces that ordering, which keeps every operator's return type as it was, including partial_ordering for the spaceship. A NaN operand is returned in both members, so NaN stays false for the relational operators and unordered for <=>. Values a float represents exactly still compare equal, so json(1) == json(1.0) is unchanged. Signed-off-by: qatcod <79017227+qatcod@users.noreply.github.com> --- include/nlohmann/detail/value_t.hpp | 65 +++++++++++++++++++++++++ include/nlohmann/json.hpp | 8 ++-- single_include/nlohmann/json.hpp | 73 +++++++++++++++++++++++++++-- tests/src/unit-comparison.cpp | 51 ++++++++++++++++++++ 4 files changed, 189 insertions(+), 8 deletions(-) diff --git a/include/nlohmann/detail/value_t.hpp b/include/nlohmann/detail/value_t.hpp index 06aefa5ac..9fc217bcc 100644 --- a/include/nlohmann/detail/value_t.hpp +++ b/include/nlohmann/detail/value_t.hpp @@ -9,9 +9,12 @@ #pragma once #include // array +#include // isnan, ldexp, trunc #include // size_t #include // uint8_t +#include // numeric_limits #include // string +#include // is_signed #include #if JSON_HAS_THREE_WAY_COMPARISON @@ -114,5 +117,67 @@ inline bool operator<(const value_t lhs, const value_t rhs) noexcept } #endif + +/*! +@brief compare an integer with a floating point number without precision loss + +Widening the integer to the floating point type loses precision beyond the +float's mantissa, which makes equality intransitive: both 2^63-2 and 2^63-1 +round to 2^63, so each compares equal to that float while differing from each +other. Ordering built on that is not a strict weak ordering, so sorting such +values, or using them as keys in an ordered container, is undefined behavior. + +Returns a value to be compared against zero with the original operator, which +reproduces the exact ordering. A NaN operand is returned as is, so comparing it +against zero keeps NaN's semantics: false for the relational operators and +unordered for `<=>`. +*/ +template +FloatType compare_integer_with_float(const IntegerType i, const FloatType f) noexcept +{ + const auto ordered = [](int c) noexcept + { + return static_cast(c); + }; + + if (std::isnan(f)) + { + return f; + } + + // values of IntegerType lie in [-bound, bound) when signed and in + // [0, bound) when unsigned; digits excludes the sign bit, so bound is a + // power of two that the float represents exactly + const FloatType bound = std::ldexp(static_cast(1), std::numeric_limits::digits); + if (f >= bound) + { + return ordered(-1); + } + if (std::is_signed::value ? (f < -bound) : (f < static_cast(0))) + { + return ordered(1); + } + + // f is now within the integer's range, so truncating it is exact + const FloatType truncated = std::trunc(f); + const auto as_integer = static_cast(truncated); + if (i != as_integer) + { + return ordered(i < as_integer ? -1 : 1); + } + + // the integer parts agree, so any fractional part decides + const FloatType fraction = f - truncated; + if (fraction > static_cast(0)) + { + return ordered(-1); + } + if (fraction < static_cast(0)) + { + return ordered(1); + } + return ordered(0); +} + } // namespace detail NLOHMANN_JSON_NAMESPACE_END diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 4e057068c..969942183 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3703,19 +3703,19 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } \ else if (lhs_type == value_t::number_integer && rhs_type == value_t::number_float) \ { \ - return static_cast(lhs.m_data.m_value.number_integer) op rhs.m_data.m_value.number_float; \ + return (detail::compare_integer_with_float(lhs.m_data.m_value.number_integer, rhs.m_data.m_value.number_float)) op (static_cast(0)); \ } \ else if (lhs_type == value_t::number_float && rhs_type == value_t::number_integer) \ { \ - return lhs.m_data.m_value.number_float op static_cast(rhs.m_data.m_value.number_integer); \ + return (static_cast(0)) op (detail::compare_integer_with_float(rhs.m_data.m_value.number_integer, lhs.m_data.m_value.number_float)); \ } \ else if (lhs_type == value_t::number_unsigned && rhs_type == value_t::number_float) \ { \ - return static_cast(lhs.m_data.m_value.number_unsigned) op rhs.m_data.m_value.number_float; \ + return (detail::compare_integer_with_float(lhs.m_data.m_value.number_unsigned, rhs.m_data.m_value.number_float)) op (static_cast(0)); \ } \ else if (lhs_type == value_t::number_float && rhs_type == value_t::number_unsigned) \ { \ - return lhs.m_data.m_value.number_float op static_cast(rhs.m_data.m_value.number_unsigned); \ + return (static_cast(0)) op (detail::compare_integer_with_float(rhs.m_data.m_value.number_unsigned, lhs.m_data.m_value.number_float)); \ } \ else if (lhs_type == value_t::number_unsigned && rhs_type == value_t::number_integer) \ { \ diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 4c346b412..d5a808d0b 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -223,9 +223,12 @@ #include // array +#include // isnan, ldexp, trunc #include // size_t #include // uint8_t +#include // numeric_limits #include // string +#include // is_signed // #include // __ _____ _____ _____ @@ -3283,6 +3286,68 @@ inline bool operator<(const value_t lhs, const value_t rhs) noexcept } #endif + +/*! +@brief compare an integer with a floating point number without precision loss + +Widening the integer to the floating point type loses precision beyond the +float's mantissa, which makes equality intransitive: both 2^63-2 and 2^63-1 +round to 2^63, so each compares equal to that float while differing from each +other. Ordering built on that is not a strict weak ordering, so sorting such +values, or using them as keys in an ordered container, is undefined behavior. + +Returns a value to be compared against zero with the original operator, which +reproduces the exact ordering. A NaN operand is returned as is, so comparing it +against zero keeps NaN's semantics: false for the relational operators and +unordered for `<=>`. +*/ +template +FloatType compare_integer_with_float(const IntegerType i, const FloatType f) noexcept +{ + const auto ordered = [](int c) noexcept + { + return static_cast(c); + }; + + if (std::isnan(f)) + { + return f; + } + + // values of IntegerType lie in [-bound, bound) when signed and in + // [0, bound) when unsigned; digits excludes the sign bit, so bound is a + // power of two that the float represents exactly + const FloatType bound = std::ldexp(static_cast(1), std::numeric_limits::digits); + if (f >= bound) + { + return ordered(-1); + } + if (std::is_signed::value ? (f < -bound) : (f < static_cast(0))) + { + return ordered(1); + } + + // f is now within the integer's range, so truncating it is exact + const FloatType truncated = std::trunc(f); + const auto as_integer = static_cast(truncated); + if (i != as_integer) + { + return ordered(i < as_integer ? -1 : 1); + } + + // the integer parts agree, so any fractional part decides + const FloatType fraction = f - truncated; + if (fraction > static_cast(0)) + { + return ordered(-1); + } + if (fraction < static_cast(0)) + { + return ordered(1); + } + return ordered(0); +} + } // namespace detail NLOHMANN_JSON_NAMESPACE_END @@ -25057,19 +25122,19 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } \ else if (lhs_type == value_t::number_integer && rhs_type == value_t::number_float) \ { \ - return static_cast(lhs.m_data.m_value.number_integer) op rhs.m_data.m_value.number_float; \ + return (detail::compare_integer_with_float(lhs.m_data.m_value.number_integer, rhs.m_data.m_value.number_float)) op (static_cast(0)); \ } \ else if (lhs_type == value_t::number_float && rhs_type == value_t::number_integer) \ { \ - return lhs.m_data.m_value.number_float op static_cast(rhs.m_data.m_value.number_integer); \ + return (static_cast(0)) op (detail::compare_integer_with_float(rhs.m_data.m_value.number_integer, lhs.m_data.m_value.number_float)); \ } \ else if (lhs_type == value_t::number_unsigned && rhs_type == value_t::number_float) \ { \ - return static_cast(lhs.m_data.m_value.number_unsigned) op rhs.m_data.m_value.number_float; \ + return (detail::compare_integer_with_float(lhs.m_data.m_value.number_unsigned, rhs.m_data.m_value.number_float)) op (static_cast(0)); \ } \ else if (lhs_type == value_t::number_float && rhs_type == value_t::number_unsigned) \ { \ - return lhs.m_data.m_value.number_float op static_cast(rhs.m_data.m_value.number_unsigned); \ + return (static_cast(0)) op (detail::compare_integer_with_float(rhs.m_data.m_value.number_unsigned, lhs.m_data.m_value.number_float)); \ } \ else if (lhs_type == value_t::number_unsigned && rhs_type == value_t::number_integer) \ { \ diff --git a/tests/src/unit-comparison.cpp b/tests/src/unit-comparison.cpp index 31fbdc57a..68d20aca9 100644 --- a/tests/src/unit-comparison.cpp +++ b/tests/src/unit-comparison.cpp @@ -326,6 +326,57 @@ TEST_CASE("lexicographical comparison operators") #endif } + SECTION("integer/float mixed comparison is exact") + { + // Widening the integer to a double loses precision past the + // mantissa, so 2^63-2 and 2^63-1 both used to compare equal to the + // double 2^63 while differing from each other. That makes equality + // intransitive and the ordering not a strict weak ordering. + const json below_two_63 = static_cast(9223372036854775806LL); + const json max_int64 = (std::numeric_limits::max)(); + const json two_63 = 9223372036854775808.0; + + CHECK_FALSE(below_two_63 == two_63); + CHECK_FALSE(max_int64 == two_63); + CHECK(below_two_63 != max_int64); + CHECK(below_two_63 < max_int64); + CHECK(below_two_63 < two_63); + CHECK(max_int64 < two_63); + CHECK(two_63 > max_int64); + CHECK_FALSE(two_63 < max_int64); + + // the same past the unsigned range + const json max_uint64 = (std::numeric_limits::max)(); + const json two_64 = 18446744073709551616.0; + CHECK_FALSE(max_uint64 == two_64); + CHECK(max_uint64 < two_64); + CHECK(two_64 > max_uint64); + + // values a double represents exactly still compare equal + CHECK(json(1) == json(1.0)); + CHECK(json(1u) == json(1.0)); + CHECK(json(-3) == json(-3.0)); + CHECK(json(1) < json(1.5)); + CHECK(json(1.5) < json(2)); + CHECK(json(2) > json(1.5)); + + // a NaN operand stays unordered against either integer kind + CHECK_FALSE(json(1) == json(nan)); + CHECK_FALSE(json(1) < json(nan)); + CHECK_FALSE(json(nan) < json(1)); + CHECK_FALSE(json(1u) == json(nan)); + +#if JSON_HAS_THREE_WAY_COMPARISON + // JSON_HAS_CPP_20 (do not remove; see note at top of file) + CHECK((max_int64 <=> two_63) == std::partial_ordering::less); // *NOPAD* + CHECK((two_63 <=> max_int64) == std::partial_ordering::greater); // *NOPAD* + CHECK((below_two_63 <=> max_int64) == std::partial_ordering::less); // *NOPAD* + CHECK((max_uint64 <=> two_64) == std::partial_ordering::less); // *NOPAD* + CHECK((json(1) <=> json(1.0)) == std::partial_ordering::equivalent); // *NOPAD* + CHECK((json(1) <=> json(nan)) == std::partial_ordering::unordered); // *NOPAD* +#endif + } + SECTION("compares unordered") { std::vector> expected =