Fix C++20/gcc-12 issues (Part 2)

[falbrechtskirchinger]

This is part 2 of #3379.

Add the spaceship operator

Part 1 had to disable some comparisons in unit tests because of incorrect results due to standard library code using 3-way comparison which, in the absence of a user-defined operator, compares the result of implicit conversions.
In part 2 we implement the spaceship operator and fix some inconsistencies in the old comparison code:

• NaN values always compare false/unordered when compared to themselves, other NaNs, or other numbers.
• Discarded values will always compare false/unordered to themselves or any other value, unless the legacy comparison behavior is explicitly enabled by defining the macro JSON_LEGACY_COMPARISON=1.
  When the legacy behavior is selected, operations that were computed as an odd number of inverses of other operations always return true. Example:
  Given two values lhs and rhs and at least one of them being a discarded value.
  1. (lhs < rhs) == false
  2. (lhs >= rhs) == true, because it is computed as !(lhs < rhs) (odd)
  3. (lhs > rhs) == false, because it is computed as !(lhs <= rhs) which is computed as !(rhs < lhs) (even)

GCC ignores user-defined operator<=> for enumerated types when rewriting operators. Clang, MSVC, and ICC do not. (ICC manages to produce an incorrect result anyway o.O.)
I've filed a bug with GCC but do agree now that GCC is following the letter of the standard. If anyone knows where to best raise concerns over a language defect, let me know.
it's easy enough to work around by defining operator< and explicitly delegating to operator<=>. Currently only enabled for GCC but almost guaranteed to be required for (some versions of) ICC.

The unit tests were updated in several ways:

• Defined equal-length constants for bool and std::partial::ordering.
  The goal was to find visually distinguishable names for true and false. T/F, t/f, tr/fa were all ruled out because they look too similar (in my editor anyway). f_ and _t aren't perfect but the best I've come up with so far.
• Replaced constants, aligned all tables, and numbered rows and columns.
• Added value_t::discarded to the list of types and updated tables.
• Added NaN and json(value_t::discarded) to the list of values and updated tables.
• Added tables for the results of 3-way comparisons.
• Cross-check comparisons with their 3-way comparison result and perform range check between both tables on traversal.
• Added test-comparison_legacy based on unit-comparison.cpp but with JSON_LEGACY_COMPARISON=1.
• Added a regression test to verify a unit test from unit-class_parser.cpp, which breaks with the new comparison behavior, continues to work in legacy mode.

Fix iterators to meet (more) std::ranges requirements

Unlike the unconstrained algorithms, which only declared named requirements, constrained algorithms and views use concepts to enforce requirements.
Some iterators did not meet enough of those requirements.
In this PR we ensure that iter_impl satisfies the std::bidirectional_iterator concept and iteration_proxy_value satisfies the std::input_iterator concept.

The apply*() functions included in an earlier revision were moved into a separate PR: #3450

---

To Do:

• Check iterator concepts in unit test.
• Add note about "magic keywords" to unit tests.
• Should JSON_LEGACY_COMPARISON=1 be added to CI?
• Should JSON_LEGACY_COMPARISON be renamed to JSON_LEGACY_DISCARDED_COMPARISON (or something else entirely) now that it only applies to discarded values?
• Document macro JSON_LEGACY_COMPARISON.
• Document macro JSON_HAS_THREE_WAY_COMPARISON.
• Document macro JSON_HAS_RANGES.
• Update operator documentation.
• Complete issue references in commit messages.

---

Fixes #3130.
Fixes #3409.

Related discussion: #3408

[coveralls]

Coverage remained the same at 100.0% when pulling 4f744f4 on falbrechtskirchinger:fix-c++20-issues-part2 into ede6667 on nlohmann:develop.

[falbrechtskirchinger]

Ignore the "Disable regression test for 3070 on GCC <8.4" commit for now. I rebased incorrectly against a local branch instead of upstream/develop. I'll fix it when I push the next set of changes.

[nlohmann]

Since I keep making comments to things you are then commenting yourself, I submit my first bunch of comments now... :)

[nlohmann]

@falbrechtskirchinger Is there anything I can do to help here?

[falbrechtskirchinger]

@falbrechtskirchinger Is there anything I can do to help here?

Yes. Please review and merge. :-)

I see a minor merge conflict in the documentation that I can resolve. Otherwise, this PR is done.

[gregmarr]

There's also a Codacy issue that needs to be ignored in the comparison tests.

[nlohmann]

There's also a Codacy issue that needs to be ignored in the comparison tests.

I marked it as ignored.

[nlohmann]

Looks good to me.

[nlohmann]

I would feel more comfortable if @gregmarr could have a second look at this before merging.

[falbrechtskirchinger]

I'd also like to have one last look at the tables in the unit test and spot-check them.

[gregmarr]

A couple minor questions/observations, otherwise LGTM.

[falbrechtskirchinger]

I've fixed a misaligned column numbers comment in the unit test, and added JSON_INLINE_VARIABLE.

I've also completed my spot-check of the unit test tables and added a CHECK() for std::is_gt() to the cross-check loops. I'm as certain that these tables are correct as I'm ever going to be.

@nlohmann Ready for merge pending all-green CI.
(There's an opportunity to clarify how different types compare on the individual operator pages. I spotted at least two very minor issues in the BJData documentation but missed my window to leave a comment. I'll submit a separate PR with various small doc fixes later.)

[nlohmann]

Thanks a lot!!!

[gregmarr]

@falbrechtskirchinger I wonder if this is the reason for the issues we had with differing compiler behaviors: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2468r2.html

[falbrechtskirchinger]

@falbrechtskirchinger I wonder if this is the reason for the issues we had with differing compiler behaviors: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2468r2.html

Interesting find. I just briefly scanned through it. Based on that, my initial thought is no. Our issue is specific to enums and how overload resolution is specified for them. I wish I knew what the proper process is to bring (perceived) language defects to the attention of the standard committee. Maybe I should just email the authors of that paper.

[gregmarr]

@falbrechtskirchinger Ah, that's right. I just remembered that there was surprising results in the comparisons, not what exactly they were.