Fix integer comparison bug

[ljcjclljc]

This PR provides a minimal implementation for #5210.
Fiex #5210

Background

When comparing number_unsigned and number_integer, the current implementation converts the unsigned value to number_integer_t before applying the operator.

This is problematic for values above INT64_MAX, because converting such uint64_t values to number_integer_t changes them into the negative signed range. As a result, valid mixed signed/unsigned comparisons can return mathematically incorrect results.

Problem

The affected logic is in the mixed comparison branches of JSON_IMPLEMENT_OPERATOR in include/nlohmann/json.hpp.

Before this change, comparisons such as these could fail:

• json(-1) < json(18446744073709551615ULL) returned false
• json(18446744073709551615ULL) == json(-1) returned true

This is not limited to one construction path. The same incorrect behavior can be observed after direct construction, object assignment, array insertion, initializer-list construction, nested access, and json::parse().

Solution

This PR keeps the existing macro shape and applies the smallest possible fix in the mixed number_unsigned / number_integer comparison branches:

• If the signed operand is negative, return the mathematically correct ordering result directly.
• If the signed operand is non-negative, cast it to number_unsigned_t and compare in the unsigned domain.

This avoids the unsafe uint64_t -> int64_t conversion while preserving the existing comparison structure.

Scope

This PR is intentionally minimal:

• no refactor of the comparison macro
• no change to the public API
• no unrelated behavior changes

It only fixes the signed/unsigned mixed comparison bug described in #5210.

• The changes are described in detail, both the what and why.
• If applicable, an https://github.com/nlohmann/json/issues is referenced.
• The https://coveralls.io/github/nlohmann/json remained at 100%. A test case for every new line of code.
• If applicable, the https://json.nlohmann.me is updated.
• The source code is amalgamated by running make amalgamate.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @ljcjclljc
Please read and follow the Contribution Guidelines.

[gregmarr]

This seems reasonable at a low level. There may be some cases that should be documented where a < b and json(a) < json(b) will provide different results. Those are the same cases where the compiler should have been warning you not to do that a < b, so that's probably okay, but good to call it out explicitly.

[gregmarr]

This is a good change, aligns more with the C++ promotion rules.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @ljcjclljc
Please read and follow the Contribution Guidelines.

[gregmarr]

IMO, this needs a lot more documentation, both in the main documentation section, and somewhere that will show up in the changes section of the readme as a potentially breaking change, with the appropriate rationale, as described in the linked issue.

[gregmarr]

Since we probably don't want to put a comment inside the macro explaining the actual op, should there be one here?

  // This is done by checking for the signed value being less than zero.
  // If it is, the comparison is performed using `0` for the signed value
  // and `1` for the unsigned value.  Since the negative signed value will
  // always be less than the unsigned value, any two fixed values that 
  // have the same relationship will produce the right result.
  // If the signed value is not less than zero, it is cast to unsigned
  // before doing the comparison.

I'm wondering if -1 and 1 would be clearer than 0 and 1. That way you can explicitly see the negative sign.

[nlohmann]

Some comments. Will review later.

[nlohmann]

Looks good to me.

[nlohmann]

For the release notes, I would add the following to the changelog:

Fixed incorrect results when comparing a number_unsigned value greater than INT64_MAX against a number_integer value. Mixed signed/unsigned comparisons previously cast the unsigned operand to the signed type, wrapping large values into the negative range (e.g. json(-1) < json(18446744073709551615ULL) returned false, and json(18446744073709551615ULL) == json(-1) returned true). Comparisons are now performed in a domain that preserves the mathematical result (#5210, #5211).

Furthermore, I would add somewhere notable at the top:

Comparison operators (==, !=, <, <=, >, >=, <=>) now return the mathematically correct result when a number_unsigned value above INT64_MAX is compared with a negative number_integer. Values ≤ INT64_MAX are unaffected. Code that relied on the previous (incorrect) ordering — e.g. basic_json values used as keys in ordered containers — may observe different ordering or equality after upgrading.

@gregmarr @ljcjclljc Are you OK with this?

@gregmarr Anything you would like to add or discuss for this PR - #5211 (comment) is still open.

[ljcjclljc]

I fully agree with the wording you drafted for the changelog and top highlight notice. It clearly explains the root cause, wrong examples, new correct comparison logic, and reminds users about possible ordering changes after upgrade. No further revisions or discussions needed from my side for PR #5211.