Support decomposing types that only compare with literal 0

This is primarily done to support new `std::*_ordering` types,
but the refactoring also supports any other type with this
property.

The compilation overhead is surprisingly low. Testing it with
clang on a Linux machine, compiling our SelfTest project takes
only 2-3% longer with these changes than it takes otherwise.

Closes #2555

src/catch2/internal/catch_compare_traits.hpp

@@ -15,6 +15,19 @@
 namespace Catch {
     namespace Detail {
 
+#if defined( __GNUC__ ) && !defined( __clang__ )
+#    pragma GCC diagnostic push
+    // GCC likes to complain about comparing bool with 0, in the decltype()
+    // that defines the comparable traits below.
+#    pragma GCC diagnostic ignored "-Wbool-compare"
+    // "ordered comparison of pointer with integer zero" same as above,
+    // but it does not have a separate warning flag to suppress
+#    pragma GCC diagnostic ignored "-Wextra"
+    // Did you know that comparing floats with `0` directly
+    // is super-duper dangerous in unevaluated context?
+#    pragma GCC diagnostic ignored "-Wfloat-equal"
+#endif
+
 #define CATCH_DEFINE_COMPARABLE_TRAIT( id, op )                               \
     template <typename, typename, typename = void>                            \
     struct is_##id##_comparable : std::false_type {};                         \
@@ -41,6 +54,11 @@ namespace Catch {
 
 #undef CATCH_DEFINE_COMPARABLE_TRAIT
 
+#if defined( __GNUC__ ) && !defined( __clang__ )
+#    pragma GCC diagnostic pop
+#endif
+
+
     } // namespace Detail
 } // namespace Catch
 

src/catch2/internal/catch_decomposer.hpp

@@ -11,7 +11,9 @@
 #include <catch2/catch_tostring.hpp>
 #include <catch2/internal/catch_stringref.hpp>
 #include <catch2/internal/catch_meta.hpp>
+#include <catch2/internal/catch_compare_traits.hpp>
 
+#include <type_traits>
 #include <iosfwd>
 
 #ifdef _MSC_VER
@@ -155,53 +157,119 @@ namespace Catch {
     };
 
 
-    // Specialised comparison functions to handle equality comparisons between ints and pointers (NULL deduces as an int)
-    template<typename LhsT, typename RhsT>
-    auto compareEqual( LhsT const& lhs, RhsT const& rhs ) -> bool { return static_cast<bool>(lhs == rhs); }
-    template<typename T>
-    auto compareEqual( T* const& lhs, int rhs ) -> bool { return lhs == reinterpret_cast<void const*>( rhs ); }
-    template<typename T>
-    auto compareEqual( T* const& lhs, long rhs ) -> bool { return lhs == reinterpret_cast<void const*>( rhs ); }
-    template<typename T>
-    auto compareEqual( int lhs, T* const& rhs ) -> bool { return reinterpret_cast<void const*>( lhs ) == rhs; }
-    template<typename T>
-    auto compareEqual( long lhs, T* const& rhs ) -> bool { return reinterpret_cast<void const*>( lhs ) == rhs; }
-
-    template<typename LhsT, typename RhsT>
-    auto compareNotEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return static_cast<bool>(lhs != rhs); }
-    template<typename T>
-    auto compareNotEqual( T* const& lhs, int rhs ) -> bool { return lhs != reinterpret_cast<void const*>( rhs ); }
-    template<typename T>
-    auto compareNotEqual( T* const& lhs, long rhs ) -> bool { return lhs != reinterpret_cast<void const*>( rhs ); }
-    template<typename T>
-    auto compareNotEqual( int lhs, T* const& rhs ) -> bool { return reinterpret_cast<void const*>( lhs ) != rhs; }
-    template<typename T>
-    auto compareNotEqual( long lhs, T* const& rhs ) -> bool { return reinterpret_cast<void const*>( lhs ) != rhs; }
-
-
     template<typename LhsT>
     class ExprLhs {
         LhsT m_lhs;
     public:
         explicit ExprLhs( LhsT lhs ) : m_lhs( lhs ) {}
 
-        template<typename RhsT, std::enable_if_t<!std::is_arithmetic<std::remove_reference_t<RhsT>>::value, int> = 0>
+#define CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR( id, op )           \
-        friend auto operator == ( ExprLhs && lhs, RhsT && rhs ) -> BinaryExpr<LhsT, RhsT const&> {
+    template <                                                                 \
-            return { compareEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "=="_sr, rhs };
+        typename RhsT,                                                         \
+        std::enable_if_t<                                                      \
+            Detail::is_##id##_comparable<LhsT, RhsT>::value &&                 \
+                !std::is_arithmetic<std::remove_reference_t<RhsT>>::value,     \
+            int> = 0>                                                          \
+    friend auto operator op( ExprLhs&& lhs, RhsT&& rhs )                       \
+        ->BinaryExpr<LhsT, RhsT const&> {                                      \
+        return {                                                               \
+            static_cast<bool>( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \
+    }                                                                          \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<Detail::is_##id##_comparable<LhsT, RhsT>::value &&    \
+                             std::is_arithmetic<RhsT>::value,                  \
+                         int> = 0>                                             \
+    friend auto operator op( ExprLhs&& lhs, RhsT rhs )                         \
+        ->BinaryExpr<LhsT, RhsT> {                                             \
+        return {                                                               \
+            static_cast<bool>( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \
+    }                                                                          \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<!Detail::is_##id##_comparable<LhsT, RhsT>::value &&   \
+                             Detail::is_eq_0_comparable<LhsT>::                \
+                                 value && /* We allow long because we want     \
+                                             `ptr op NULL to be accepted */    \
+                             ( std::is_same<RhsT, int>::value ||               \
+                               std::is_same<RhsT, long>::value ),              \
+                         int> = 0>                                             \
+    friend auto operator op( ExprLhs&& lhs, RhsT rhs )                         \
+        ->BinaryExpr<LhsT, RhsT> {                                             \
+        return {                                                               \
+            static_cast<bool>( lhs.m_lhs op 0 ), lhs.m_lhs, #op##_sr, rhs };   \
+    }                                                                          \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<!Detail::is_##id##_comparable<LhsT, RhsT>::value &&   \
+                             Detail::is_eq_0_comparable<RhsT>::                \
+                                 value && /* We allow long because we want     \
+                                             `ptr op NULL` to be accepted */   \
+                             ( std::is_same<LhsT, int>::value ||               \
+                               std::is_same<LhsT, long>::value ),              \
+                         int> = 0>                                             \
+    friend auto operator op( ExprLhs&& lhs, RhsT rhs )                         \
+        ->BinaryExpr<LhsT, RhsT> {                                             \
+        return { static_cast<bool>( 0 op rhs ), lhs.m_lhs, #op##_sr, rhs };    \
     }
-        template<typename RhsT, std::enable_if_t<std::is_arithmetic<RhsT>::value, int> = 0>
+        CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR( eq, == )
-        friend auto operator == ( ExprLhs && lhs, RhsT rhs ) -> BinaryExpr<LhsT, RhsT> {
+        CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR( ne, != )
-            return { compareEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "=="_sr, rhs };
+
+    #undef CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR
+
+    #define CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( id, op )     \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<                                                      \
+            Detail::is_##id##_comparable<LhsT, RhsT>::value &&                 \
+                !std::is_arithmetic<std::remove_reference_t<RhsT>>::value,     \
+            int> = 0>                                                          \
+    friend auto operator op( ExprLhs&& lhs, RhsT&& rhs )                       \
+        ->BinaryExpr<LhsT, RhsT const&> {                                      \
+        return {                                                               \
+            static_cast<bool>( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \
+    }                                                                          \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<Detail::is_##id##_comparable<LhsT, RhsT>::value &&    \
+                             std::is_arithmetic<RhsT>::value,                  \
+                         int> = 0>                                             \
+    friend auto operator op( ExprLhs&& lhs, RhsT rhs )                         \
+        ->BinaryExpr<LhsT, RhsT> {                                             \
+        return {                                                               \
+            static_cast<bool>( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \
+    }                                                                          \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<!Detail::is_##id##_comparable<LhsT, RhsT>::value &&   \
+                             Detail::is_##id##_0_comparable<LhsT>::value &&    \
+                             std::is_same<RhsT, int>::value,                   \
+                         int> = 0>                                             \
+    friend auto operator op( ExprLhs&& lhs, RhsT rhs )                         \
+        ->BinaryExpr<LhsT, RhsT> {                                             \
+        /* TODO: do we want to assert that Rhs is 0? */                        \
+        return {                                                               \
+            static_cast<bool>( lhs.m_lhs op 0 ), lhs.m_lhs, #op##_sr, rhs };   \
+    }                                                                          \
+    template <                                                                 \
+        typename RhsT,                                                         \
+        std::enable_if_t<!Detail::is_##id##_comparable<LhsT, RhsT>::value &&   \
+                             Detail::is_##id##_0_comparable<RhsT>::value &&    \
+                             std::is_same<LhsT, int>::value,                   \
+                         int> = 0>                                             \
+    friend auto operator op( ExprLhs&& lhs, RhsT rhs )                         \
+        ->BinaryExpr<LhsT, RhsT> {                                             \
+        /* TODO: do we want to assert that lhs is 0? */                        \
+        return { static_cast<bool>( 0 op rhs ), lhs.m_lhs, #op##_sr, rhs };    \
     }
 
-        template<typename RhsT, std::enable_if_t<!std::is_arithmetic<std::remove_reference_t<RhsT>>::value, int> = 0>
+        CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( lt, < )
-        friend auto operator != ( ExprLhs && lhs, RhsT && rhs ) -> BinaryExpr<LhsT, RhsT const&> {
+        CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( le, <= )
-            return { compareNotEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "!="_sr, rhs };
+        CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( gt, > )
-        }
+        CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( ge, >= )
-        template<typename RhsT, std::enable_if_t<std::is_arithmetic<RhsT>::value, int> = 0>
+
-        friend auto operator != ( ExprLhs && lhs, RhsT rhs ) -> BinaryExpr<LhsT, RhsT> {
+    #undef CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR
-            return { compareNotEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "!="_sr, rhs };
+
-        }
 
     #define CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(op) \
         template<typename RhsT, std::enable_if_t<!std::is_arithmetic<std::remove_reference_t<RhsT>>::value, int> = 0> \
@@ -213,10 +281,6 @@ namespace Catch {
             return { static_cast<bool>(lhs.m_lhs op rhs), lhs.m_lhs, #op##_sr, rhs }; \
         }
 
-        CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(<)
-        CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(>)
-        CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(<=)
-        CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(>=)
         CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(|)
         CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(&)
         CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(^)

tests/CMakeLists.txt

@@ -133,6 +133,7 @@ set(TEST_SOURCES
 
 set(TEST_HEADERS
   ${SELF_TEST_DIR}/helpers/parse_test_spec.hpp
+  ${SELF_TEST_DIR}/helpers/type_with_lit_0_comparisons.hpp
 )
 
 

tests/SelfTest/IntrospectiveTests/Traits.tests.cpp

@@ -8,36 +8,7 @@
 
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/internal/catch_compare_traits.hpp>
-
+#include <helpers/type_with_lit_0_comparisons.hpp>
-// Should only be constructible from literal 0.
-// Used by `TypeWithLit0Comparisons` for testing comparison
-// ops that only work with literal zero, the way std::*orderings do
-struct ZeroLiteralDetector {
-    constexpr ZeroLiteralDetector( ZeroLiteralDetector* ) noexcept {}
-
-    template <typename T,
-              typename = std::enable_if_t<!std::is_same<T, int>::value>>
-    constexpr ZeroLiteralDetector( T ) = delete;
-};
-
-struct TypeWithLit0Comparisons {
-#define DEFINE_COMP_OP( op )                                                  \
-    friend bool operator op( TypeWithLit0Comparisons, ZeroLiteralDetector ) { \
-        return true;                                                          \
-    }                                                                         \
-    friend bool operator op( ZeroLiteralDetector, TypeWithLit0Comparisons ) { \
-        return false;                                                          \
-    }
-
-    DEFINE_COMP_OP( < )
-    DEFINE_COMP_OP( <= )
-    DEFINE_COMP_OP( > )
-    DEFINE_COMP_OP( >= )
-    DEFINE_COMP_OP( == )
-    DEFINE_COMP_OP( != )
-
-#undef DEFINE_COMP_OP
-};
 
 
 #define ADD_TRAIT_TEST_CASE( op )                                         \

tests/SelfTest/UsageTests/Compilation.tests.cpp

@@ -6,6 +6,8 @@
 
 // SPDX-License-Identifier: BSL-1.0
 
+#include <helpers/type_with_lit_0_comparisons.hpp>
+
 #include <type_traits>
 
 // Setup for #1403 -- look for global overloads of operator << for classes
@@ -310,3 +312,20 @@ TEST_CASE("ADL universal operators don't hijack expression deconstruction", "[co
     REQUIRE(0 & adl::always_true{});
     REQUIRE(0 ^ adl::always_true{});
 }
+
+TEST_CASE( "#2555 - types that can only be compared with 0 literal (not int/long) are supported", "[compilation][approvals]" ) {
+    REQUIRE( TypeWithLit0Comparisons{} < 0 );
+    REQUIRE_FALSE( 0 < TypeWithLit0Comparisons{} );
+    REQUIRE( TypeWithLit0Comparisons{} <= 0 );
+    REQUIRE_FALSE( 0 > TypeWithLit0Comparisons{} );
+
+    REQUIRE( TypeWithLit0Comparisons{} > 0 );
+    REQUIRE_FALSE( 0 > TypeWithLit0Comparisons{} );
+    REQUIRE( TypeWithLit0Comparisons{} >= 0 );
+    REQUIRE_FALSE( 0 >= TypeWithLit0Comparisons{} );
+
+    REQUIRE( TypeWithLit0Comparisons{} == 0 );
+    REQUIRE_FALSE( 0 == TypeWithLit0Comparisons{} );
+    REQUIRE( TypeWithLit0Comparisons{} != 0 );
+    REQUIRE_FALSE( 0 != TypeWithLit0Comparisons{} );
+}

tests/SelfTest/helpers/type_with_lit_0_comparisons.hpp

@@ -0,0 +1,44 @@
+
+//              Copyright Catch2 Authors
+// Distributed under the Boost Software License, Version 1.0.
+//   (See accompanying file LICENSE.txt or copy at
+//        https://www.boost.org/LICENSE_1_0.txt)
+
+// SPDX-License-Identifier: BSL-1.0
+
+#ifndef CATCH_TEST_HELPERS_TYPE_WITH_LIT_0_COMPARISONS_HPP_INCLUDED
+#define CATCH_TEST_HELPERS_TYPE_WITH_LIT_0_COMPARISONS_HPP_INCLUDED
+
+#include <type_traits>
+
+// Should only be constructible from literal 0.
+// Used by `TypeWithLit0Comparisons` for testing comparison
+// ops that only work with literal zero, the way std::*orderings do
+struct ZeroLiteralDetector {
+    constexpr ZeroLiteralDetector( ZeroLiteralDetector* ) noexcept {}
+
+    template <typename T,
+              typename = std::enable_if_t<!std::is_same<T, int>::value>>
+    constexpr ZeroLiteralDetector( T ) = delete;
+};
+
+struct TypeWithLit0Comparisons {
+#define DEFINE_COMP_OP( op )                                                  \
+    friend bool operator op( TypeWithLit0Comparisons, ZeroLiteralDetector ) { \
+        return true;                                                          \
+    }                                                                         \
+    friend bool operator op( ZeroLiteralDetector, TypeWithLit0Comparisons ) { \
+        return false;                                                         \
+    }
+
+    DEFINE_COMP_OP( < )
+    DEFINE_COMP_OP( <= )
+    DEFINE_COMP_OP( > )
+    DEFINE_COMP_OP( >= )
+    DEFINE_COMP_OP( == )
+    DEFINE_COMP_OP( != )
+
+#undef DEFINE_COMP_OP
+};
+
+#endif // CATCH_TEST_HELPERS_TYPE_WITH_LIT_0_COMPARISONS_HPP_INCLUDED