Compilation error with REQUIRE(std::strong_ordering < 0).

[vsaulue]

Bug description

The statement REQUIRE(x < 0); fails to compile when x is of type std::strong_ordering from C++20.

Reproduction steps

Compile the following code with C++20 flags:

#include <compare>

#include <catch2/catch_test_macros.hpp>

TEST_CASE("std::strong_ordering < 0") {
	auto cmp = 2 <=> 3;
	REQUIRE(cmp < 0);
}

Expected behavior

This test should compile and run successfully.

Platform information:

Catch version: v3.1.0

OS: Windows 10 x64
Compiler: MSVC 19.32.31328 (Visual studio 2022)
First error:
error C2678: binary '<': no operator found which takes a left-hand operand of type 'RhsT' (or there is no acceptable conversion) with [ RhsT=int ]
Full compiler output

OS: Ubuntu 20.04 (virtual machine)
Compiler: g++ 10.3.0
First error:
error: no match for ‘operator<’ (operand types are ‘std::strong_ordering’ and ‘int’)
Full compiler output

Additional context

The std::strong_ordering type is a possible return type of C++20's new three-way comparison operator <=> (a.k.a spaceship operator). It should be possible to compare it to literal 0, but comparing it to something else is undefined behaviour. Catch2 coerces the 0 into int and looks for an overload of the form operator<(std::strong_ordering, int), but standard libraries don't have to provide one.

Workaround

Double parentheses in the REQUIRE() statement:

auto x = 2 <=> 3;
REQUIRE((x < 0)); // compiles and pass

[horenmar]

I looked into this a bit -> it should be fixable, but the fix is going to be annoying because it will require adding bunch of code conditional on C++20.

[vsaulue]

From what I've seen, compilers & standard libraries abuse the fact that literal 0 can be interpreted as a pointer value during overload resolution, to prevent users from comparing std::strong_ordering to any other int value.

void f(std::nullptr_t);

int main(int argc, char** argv) {
  f(0); // correct
  f(1); // compiler error
  f(argc); // compiler error
}

I'm not familiar with Catch2's internals (I just had a quick look at it today), but it might be possible to also abuse this feature to enable the decomposer to detect a literal 0, and generate an appropriate comparison . Here's a proof of concept on godbolt.

It would be much more generic, in case people use this "0 to pointer" trick outside the standard library. It doesn't require conditionals on C++20 nor an exhaustive list of classes having such unconventional comparison operators. This trick seems to work on Clang, g++ and MSVC for now at least.

Of course it's assuming that the literal zero detector works flawlessly (I can't really tell. This is dark magic for me, I just took the implementation from Clang's source code). If it can match other expressions, it could cause quite a few bugs.

[horenmar]

This actually came up in the lib-ext mailing list. There is no 100% fool-proof way to detect literal 0, which is why comparing orderings with anything that is not literal 0 is UB (ill-formed cannot be standardized without new language features).

I poked around a bit, and different stdlibs currently perform the check with different strictness -> e.g. MSVC's strong_ordering can be compared with nullptr, while libstdc++'s cannot be compared with NULL, because it is defined as 0L, which is not compatible with int. The difference mainly seems to come down to how costly are they willing to make the cost - MSVC's trick is just using nullptr_t as the unspecified type, without further TMP, because 0 converts to nullptr, while other integer constants do not. And if the user passes in nullptr, well that's where library-UB comes in and it is the user's fault.

I also poked around with the implementation, and I am considering using the zero detection trick, but I have misgivings about it's compilation costs, as it will add a bunch of overloads and TMP to all assertions. It is possible that it will be hidden behind C++20 switch anyway, to avoid imposing the costs on everyone.

[horenmar]

Also I don't see a way to extend the detector hack to handle 0 < (1 <=> 2), because when the LhsExpr is being instantiated, we have no idea what is going to be on the right hand side, and can't easily distinguish between needing to try implicit conversion, or capturing proper int, and I don't see a way to craft an overload set that handles this properly.