Skip to content

Add scalar-on-left overloads for legacy discarded comparisons in C++20 - #5682

Merged
nlohmann merged 1 commit into
developfrom
claude/legacy-discarded-scalar-lhs-5665
Oct 4, 2026
Merged

nlohmann merged 1 commit into
developfrom
claude/legacy-discarded-scalar-lhs-5665

Conversation

@nlohmann

Copy link
Copy Markdown
Owner

Summary

With JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON defined to 1 and C++20, 1 <= discarded and
1 >= discarded returned false instead of the documented true when the scalar is on the
left-hand side. The C++20 legacy block only defined member operators, which are candidates only
when the basic_json is the left operand. For a scalar on the left, overload resolution instead
picked the candidate rewritten from operator<=>, which does not emulate the legacy behavior and
yields std::partial_ordering::unordered.

Changes

  • Add friend bool operator<=(ScalarType, const_reference) and
    friend bool operator>=(ScalarType, const_reference) to the C++20
    JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON block, matching the scalar-on-the-left overloads
    that already exist in the C++17 branch. A non-rewritten candidate wins over the rewritten one,
    so there is no ambiguity.
  • Document the fix in the "Version history" section of the
    JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON macro page.

Tests

  • Added tests/src/unit-comparison.cpp regression test "regression Legacy discarded comparison in C++20: scalar <= discarded and scalar >= discarded yield false #5665 - scalar <= discarded
    and scalar >= discarded in C++20 legacy mode" (guarded by
    JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON), covering all four operand orders for <= and
    >= with a discarded value.
  • Confirmed the new test fails without the fix: built tests/src/unit-comparison.cpp against
    develop's headers (633de8e44) with -std=c++20 -DJSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1;
    the four scalar-on-the-left checks failed exactly as in the issue's repro.
  • With the fix, built and ran the full unit-comparison.cpp with clang++
    (-Wall -Wextra -Werror -fsanitize=address,undefined -fno-sanitize-recover=undefined) for
    -std=c++11, -std=c++17, and -std=c++20, each with
    -DJSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1, and once more for -std=c++20 without the
    macro (default mode). All configurations passed with 0 failed assertions.

Public API

No breaking changes. This only adds two new overloads inside the (deprecated, opt-in)
JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1 C++20 code path, bringing its scalar-on-the-left
behavior in line with what is already documented and with the C++17 branch.

Fixes #5665


This PR was written by Claude Code on behalf of @nlohmann.

🤖 Generated with Claude Code

With JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1 and C++20, a scalar on
the left-hand side of <= or >= (e.g., `1 <= discarded`) yielded false
instead of the documented true. The C++20 legacy block only had member
operators, which are only candidates when the basic_json is the left
operand; for a scalar on the left, overload resolution picked the
candidate rewritten from operator<=>, which does not emulate the legacy
behavior. The C++17 branch already has scalar-on-the-left friend
overloads for <= and >=; add the equivalent pair to the C++20 legacy
block.

Added a regression test to tests/src/unit-comparison.cpp covering all
four operand orders for both operators.

Fixes #5665.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Sep 29, 2026
@nlohmann nlohmann added 🚀 ready to merge Ready to merge - just waiting for CI to complete. and removed review needed It would be great if someone could review the proposed changes. labels Oct 4, 2026
@nlohmann
nlohmann merged commit df27cc3 into develop Oct 4, 2026
154 of 163 checks passed
@nlohmann
nlohmann deleted the claude/legacy-discarded-scalar-lhs-5665 branch October 4, 2026 09:46
nlohmann added a commit that referenced this pull request Oct 4, 2026
- binary_reader: rename the error_handler constructor parameter, which
  shadowed the member (-Wshadow, -Wshadow-field-in-constructor; #5746)
- basic_json(copy_construct_tag, ...): declare it noexcept when copying
  the base class is (GCC 16 -Wnoexcept; #5690)
- the scalar-on-left legacy comparison operators: noexcept only when
  converting the scalar is, like their member counterparts (#5682, #5751)
- compare_leaves: use std::is_eq/is_lt/is_gt instead of comparing a
  std::partial_ordering with 0 (-Wzero-as-null-pointer-constant; #5686)
- serializer: silence MSVC C4127 for the EnsureAscii template parameter
  (#5741, #5746)
- clang-tidy: return the sanitized reference in binary_writer, take the
  key of ordered_map::find_impl by const reference (#5727), and mark the
  switches over parse_array_index (#5728)
- ordered_map: keep <memory> for std::allocator (IWYU)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann added a commit that referenced this pull request Oct 4, 2026
* Keep the serializer conversion for objects whose keys cannot be converted

#5591 added a test converting nlohmann::json into a basic_json whose
string type cannot be constructed from std::string. That instantiates
convert_iteratively(), whose members.emplace_back(next.key(), ...) needs
exactly that key conversion, and broke the build of unit-alt-string.
Dispatch on the key's constructibility and leave such conversions to the
serializers, as the levels above the nesting bound already do (#3425).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Fix the remaining CI failures on develop

- unit-wstring: with a 16-bit wchar_t (Windows), a lone surrogate is
  reported as the ill-formed byte 0xFF since #5704; the std::wstring
  expectations still had the previous <U+0000>.
- ci_single_binaries: json_literals.hpp (#5610) and json.hpp include each
  other on purpose, and IWYU, not following the cycle, asks to replace
  json.hpp with json_fwd.hpp. Report its findings without failing the
  build, as already done for json.hpp.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Fix the library warnings and noexcept specifications from the merged PRs

- binary_reader: rename the error_handler constructor parameter, which
  shadowed the member (-Wshadow, -Wshadow-field-in-constructor; #5746)
- basic_json(copy_construct_tag, ...): declare it noexcept when copying
  the base class is (GCC 16 -Wnoexcept; #5690)
- the scalar-on-left legacy comparison operators: noexcept only when
  converting the scalar is, like their member counterparts (#5682, #5751)
- compare_leaves: use std::is_eq/is_lt/is_gt instead of comparing a
  std::partial_ordering with 0 (-Wzero-as-null-pointer-constant; #5686)
- serializer: silence MSVC C4127 for the EnsureAscii template parameter
  (#5741, #5746)
- clang-tidy: return the sanitized reference in binary_writer, take the
  key of ordered_map::find_impl by const reference (#5727), and mark the
  switches over parse_array_index (#5728)
- ordered_map: keep <memory> for std::allocator (IWYU)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Split unit-conversions.cpp so MinGW can link it

clang 18 with the MinGW linker failed to link test-conversions_cpp17
("relocation truncated to fit: IMAGE_REL_AMD64_REL32"). As windows.yml
recommends, keep the objects small by splitting the test file.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Fix the tests added by the merged PRs for all CI configurations

- discard the results of dump() and from_*() in CHECK_THROWS with
  utils::ignore_return_value (GCC -Werror=unused-result)
- give unit-bson's huge_string_t a default constructor (MSVC C2512,
  GCC 5, clang 3.5)
- unit-disabled_exceptions: use the literals namespace when the global
  UDLs are off (ci_test_noglobaludls; #5700)
- unit-binary_utf8_strict: expect the JSON pointer prefix with
  JSON_DIAGNOSTICS (#5741)
- skip the tests that rely on exceptions under JSON_NOEXCEPTION
  (#5678, #5732)
- clang-tidy and clang -Werror: static test data, CAPTURE(...);,
  const-correctness, use-after-move alias, unused conversion operator,
  a missing <iterator> include

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Title the macro examples and add JSON_STRICT_BINARY_UTF8 to the docset

The documentation style check requires "Example: ..." titles on pages with several examples (#5741, #5591) and a docset entry for every macro page.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

* Regenerate BUILD.bazel and nlohmann_json.natvis

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5746 added detail/output/error_handler.hpp and #5741 the json_abi_sbu8 ABI tag.

* Install libidn11 for the CMake 3.5.0 binary in ci_cmake_flags

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

#5733 moved ci_cmake_options from ubuntu:focal to ubuntu:24.04, which no longer ships libidn.so.11; the CMake 3.5.0 release binary links against it, so every ci_cmake_flags run has failed since. Install focal's libidn11 package for that matrix entry only.

* Suppress Infer's false STACK_VARIABLE_ADDRESS_ESCAPE in get_impl

get_impl() returns its local by value. A test added by the merged PRs instantiates it with a type Infer misreads, so ci_infer reported the 2021 code for the first time.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation M 🚀 ready to merge Ready to merge - just waiting for CI to complete. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legacy discarded comparison in C++20: scalar <= discarded and scalar >= discarded yield false

2 participants