From 6285225fd068df42d043721f3bef65fca48c59fb Mon Sep 17 00:00:00 2001 From: ljcjclljc <169005916+ljcjclljc@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:21:19 +0800 Subject: [PATCH] Fix integer comparison bug (#5211) * Fix integer comparison bug Signed-off-by: ljccjlljc <939159710@qq.com> * commit Signed-off-by: ljccjlljc <939159710@qq.com> * Remove generated CI artifacts and update amalgamation Signed-off-by: ljccjlljc <939159710@qq.com> * Silence cpplint braces warning in comparison macro Signed-off-by: ljccjlljc <939159710@qq.com> * Update amalgamation after cpplint fix Signed-off-by: ljccjlljc <939159710@qq.com> * Add mixed signed and unsigned comparison regression test Signed-off-by: ljccjlljc <939159710@qq.com> * Clarify mixed signed and unsigned comparison handling Signed-off-by: ljccjlljc <939159710@qq.com> * Expand mixed signed and unsigned comparison tests Signed-off-by: ljccjlljc <939159710@qq.com> --------- Signed-off-by: ljccjlljc <939159710@qq.com> --- include/nlohmann/json.hpp | 16 +++++-- single_include/nlohmann/json.hpp | 16 +++++-- tests/src/unit-comparison.cpp | 71 ++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index a460bb29f..235e6b737 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3652,6 +3652,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // note parentheses around operands are necessary; see // https://github.com/nlohmann/json/issues/1530 + // Mixed signed/unsigned integer comparisons check whether the signed value + // is negative before casting. If it is, the comparison is performed with + // the fixed values -1 and 1, which preserves the ordering relationship + // because any negative signed value is smaller than any unsigned value. + // Otherwise, the non-negative signed value is cast to unsigned before the + // comparison to avoid wraparound. #define JSON_IMPLEMENT_OPERATOR(op, null_result, unordered_result, default_result) \ const auto lhs_type = lhs.type(); \ const auto rhs_type = rhs.type(); \ @@ -3710,12 +3716,16 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } \ else if (lhs_type == value_t::number_unsigned && rhs_type == value_t::number_integer) \ { \ - return static_cast(lhs.m_data.m_value.number_unsigned) op rhs.m_data.m_value.number_integer; \ + return (rhs.m_data.m_value.number_integer < 0) \ + ? (number_integer_t(1) op number_integer_t(-1)) \ + : (lhs.m_data.m_value.number_unsigned op static_cast(rhs.m_data.m_value.number_integer)); \ } \ else if (lhs_type == value_t::number_integer && rhs_type == value_t::number_unsigned) \ { \ - return lhs.m_data.m_value.number_integer op static_cast(rhs.m_data.m_value.number_unsigned); \ - } \ + return (lhs.m_data.m_value.number_integer < 0) \ + ? (number_integer_t(-1) op number_integer_t(1)) \ + : (static_cast(lhs.m_data.m_value.number_integer) op rhs.m_data.m_value.number_unsigned); \ + } \ else if(compares_unordered(lhs, rhs))\ {\ return (unordered_result);\ diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 724865e11..fff350ee4 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -24988,6 +24988,12 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // note parentheses around operands are necessary; see // https://github.com/nlohmann/json/issues/1530 + // Mixed signed/unsigned integer comparisons check whether the signed value + // is negative before casting. If it is, the comparison is performed with + // the fixed values -1 and 1, which preserves the ordering relationship + // because any negative signed value is smaller than any unsigned value. + // Otherwise, the non-negative signed value is cast to unsigned before the + // comparison to avoid wraparound. #define JSON_IMPLEMENT_OPERATOR(op, null_result, unordered_result, default_result) \ const auto lhs_type = lhs.type(); \ const auto rhs_type = rhs.type(); \ @@ -25046,12 +25052,16 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } \ else if (lhs_type == value_t::number_unsigned && rhs_type == value_t::number_integer) \ { \ - return static_cast(lhs.m_data.m_value.number_unsigned) op rhs.m_data.m_value.number_integer; \ + return (rhs.m_data.m_value.number_integer < 0) \ + ? (number_integer_t(1) op number_integer_t(-1)) \ + : (lhs.m_data.m_value.number_unsigned op static_cast(rhs.m_data.m_value.number_integer)); \ } \ else if (lhs_type == value_t::number_integer && rhs_type == value_t::number_unsigned) \ { \ - return lhs.m_data.m_value.number_integer op static_cast(rhs.m_data.m_value.number_unsigned); \ - } \ + return (lhs.m_data.m_value.number_integer < 0) \ + ? (number_integer_t(-1) op number_integer_t(1)) \ + : (static_cast(lhs.m_data.m_value.number_integer) op rhs.m_data.m_value.number_unsigned); \ + } \ else if(compares_unordered(lhs, rhs))\ {\ return (unordered_result);\ diff --git a/tests/src/unit-comparison.cpp b/tests/src/unit-comparison.cpp index d9df1a3a6..31fbdc57a 100644 --- a/tests/src/unit-comparison.cpp +++ b/tests/src/unit-comparison.cpp @@ -15,6 +15,8 @@ #include "doctest_compatibility.h" +#include + #define JSON_TESTS_PRIVATE #include using nlohmann::json; @@ -255,6 +257,75 @@ TEST_CASE("lexicographical comparison operators") {f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_, f_}, // 21 }; + SECTION("signed/unsigned mixed comparison above INT64_MAX") + { + const json above_int64_max = static_cast((std::numeric_limits::max)()) + 1ULL; + const json max_uint64 = (std::numeric_limits::max)(); + const json negative_one = -1; + const json one = 1; + const json max_int64 = (std::numeric_limits::max)(); + + CHECK_FALSE(above_int64_max == negative_one); + CHECK(above_int64_max != negative_one); + CHECK(negative_one < above_int64_max); + CHECK(negative_one <= above_int64_max); + CHECK_FALSE(negative_one > above_int64_max); + CHECK_FALSE(negative_one >= above_int64_max); + CHECK_FALSE(above_int64_max < negative_one); + CHECK_FALSE(above_int64_max <= negative_one); + CHECK(above_int64_max > negative_one); + CHECK(above_int64_max >= negative_one); + CHECK(negative_one != above_int64_max); + CHECK_FALSE(negative_one == above_int64_max); + + CHECK_FALSE(max_uint64 == negative_one); + CHECK(max_uint64 != negative_one); + CHECK(negative_one < max_uint64); + CHECK(negative_one <= max_uint64); + CHECK_FALSE(negative_one > max_uint64); + CHECK_FALSE(negative_one >= max_uint64); + CHECK_FALSE(max_uint64 < negative_one); + CHECK_FALSE(max_uint64 <= negative_one); + CHECK(max_uint64 > negative_one); + CHECK(max_uint64 >= negative_one); + CHECK(negative_one != max_uint64); + CHECK_FALSE(negative_one == max_uint64); + + CHECK_FALSE(one == above_int64_max); + CHECK(one != above_int64_max); + CHECK(one < above_int64_max); + CHECK(one <= above_int64_max); + CHECK_FALSE(one > above_int64_max); + CHECK_FALSE(one >= above_int64_max); + CHECK_FALSE(above_int64_max < one); + CHECK_FALSE(above_int64_max <= one); + CHECK(above_int64_max > one); + CHECK(above_int64_max >= one); + + CHECK_FALSE(max_int64 == above_int64_max); + CHECK(max_int64 != above_int64_max); + CHECK(max_int64 < above_int64_max); + CHECK(max_int64 <= above_int64_max); + CHECK_FALSE(max_int64 > above_int64_max); + CHECK_FALSE(max_int64 >= above_int64_max); + CHECK_FALSE(above_int64_max < max_int64); + CHECK_FALSE(above_int64_max <= max_int64); + CHECK(above_int64_max > max_int64); + CHECK(above_int64_max >= max_int64); + +#if JSON_HAS_THREE_WAY_COMPARISON + // JSON_HAS_CPP_20 (do not remove; see note at top of file) + CHECK((negative_one <=> above_int64_max) == std::partial_ordering::less); // *NOPAD* + CHECK((above_int64_max <=> negative_one) == std::partial_ordering::greater); // *NOPAD* + CHECK((negative_one <=> max_uint64) == std::partial_ordering::less); // *NOPAD* + CHECK((max_uint64 <=> negative_one) == std::partial_ordering::greater); // *NOPAD* + CHECK((one <=> above_int64_max) == std::partial_ordering::less); // *NOPAD* + CHECK((above_int64_max <=> one) == std::partial_ordering::greater); // *NOPAD* + CHECK((max_int64 <=> above_int64_max) == std::partial_ordering::less); // *NOPAD* + CHECK((above_int64_max <=> max_int64) == std::partial_ordering::greater); // *NOPAD* +#endif + } + SECTION("compares unordered") { std::vector> expected =