diff --git a/src/catch2/internal/catch_compare_traits.hpp b/src/catch2/internal/catch_compare_traits.hpp index 381dba89..2debeced 100644 --- a/src/catch2/internal/catch_compare_traits.hpp +++ b/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 \ 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 diff --git a/src/catch2/internal/catch_decomposer.hpp b/src/catch2/internal/catch_decomposer.hpp index 90922c81..ab563701 100644 --- a/src/catch2/internal/catch_decomposer.hpp +++ b/src/catch2/internal/catch_decomposer.hpp @@ -11,7 +11,9 @@ #include #include #include +#include +#include #include #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 - auto compareEqual( LhsT const& lhs, RhsT const& rhs ) -> bool { return static_cast(lhs == rhs); } - template - auto compareEqual( T* const& lhs, int rhs ) -> bool { return lhs == reinterpret_cast( rhs ); } - template - auto compareEqual( T* const& lhs, long rhs ) -> bool { return lhs == reinterpret_cast( rhs ); } - template - auto compareEqual( int lhs, T* const& rhs ) -> bool { return reinterpret_cast( lhs ) == rhs; } - template - auto compareEqual( long lhs, T* const& rhs ) -> bool { return reinterpret_cast( lhs ) == rhs; } - - template - auto compareNotEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return static_cast(lhs != rhs); } - template - auto compareNotEqual( T* const& lhs, int rhs ) -> bool { return lhs != reinterpret_cast( rhs ); } - template - auto compareNotEqual( T* const& lhs, long rhs ) -> bool { return lhs != reinterpret_cast( rhs ); } - template - auto compareNotEqual( int lhs, T* const& rhs ) -> bool { return reinterpret_cast( lhs ) != rhs; } - template - auto compareNotEqual( long lhs, T* const& rhs ) -> bool { return reinterpret_cast( lhs ) != rhs; } - - template class ExprLhs { LhsT m_lhs; public: explicit ExprLhs( LhsT lhs ) : m_lhs( lhs ) {} - template>::value, int> = 0> - friend auto operator == ( ExprLhs && lhs, RhsT && rhs ) -> BinaryExpr { - return { compareEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "=="_sr, rhs }; - } - template::value, int> = 0> - friend auto operator == ( ExprLhs && lhs, RhsT rhs ) -> BinaryExpr { - return { compareEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "=="_sr, rhs }; - } +#define CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR( id, op ) \ + template < \ + typename RhsT, \ + std::enable_if_t< \ + Detail::is_##id##_comparable::value && \ + !std::is_arithmetic>::value, \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT&& rhs ) \ + ->BinaryExpr { \ + return { \ + static_cast( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \ + } \ + template < \ + typename RhsT, \ + std::enable_if_t::value && \ + std::is_arithmetic::value, \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT rhs ) \ + ->BinaryExpr { \ + return { \ + static_cast( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \ + } \ + template < \ + typename RhsT, \ + std::enable_if_t::value && \ + Detail::is_eq_0_comparable:: \ + value && /* We allow long because we want \ + `ptr op NULL to be accepted */ \ + ( std::is_same::value || \ + std::is_same::value ), \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT rhs ) \ + ->BinaryExpr { \ + return { \ + static_cast( lhs.m_lhs op 0 ), lhs.m_lhs, #op##_sr, rhs }; \ + } \ + template < \ + typename RhsT, \ + std::enable_if_t::value && \ + Detail::is_eq_0_comparable:: \ + value && /* We allow long because we want \ + `ptr op NULL` to be accepted */ \ + ( std::is_same::value || \ + std::is_same::value ), \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT rhs ) \ + ->BinaryExpr { \ + return { static_cast( 0 op rhs ), lhs.m_lhs, #op##_sr, rhs }; \ + } + CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR( eq, == ) + CATCH_INTERNAL_DEFINE_EXPRESSION_EQUALITY_OPERATOR( ne, != ) + + #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::value && \ + !std::is_arithmetic>::value, \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT&& rhs ) \ + ->BinaryExpr { \ + return { \ + static_cast( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \ + } \ + template < \ + typename RhsT, \ + std::enable_if_t::value && \ + std::is_arithmetic::value, \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT rhs ) \ + ->BinaryExpr { \ + return { \ + static_cast( lhs.m_lhs op rhs ), lhs.m_lhs, #op##_sr, rhs }; \ + } \ + template < \ + typename RhsT, \ + std::enable_if_t::value && \ + Detail::is_##id##_0_comparable::value && \ + std::is_same::value, \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT rhs ) \ + ->BinaryExpr { \ + /* TODO: do we want to assert that Rhs is 0? */ \ + return { \ + static_cast( lhs.m_lhs op 0 ), lhs.m_lhs, #op##_sr, rhs }; \ + } \ + template < \ + typename RhsT, \ + std::enable_if_t::value && \ + Detail::is_##id##_0_comparable::value && \ + std::is_same::value, \ + int> = 0> \ + friend auto operator op( ExprLhs&& lhs, RhsT rhs ) \ + ->BinaryExpr { \ + /* TODO: do we want to assert that lhs is 0? */ \ + return { static_cast( 0 op rhs ), lhs.m_lhs, #op##_sr, rhs }; \ + } + + CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( lt, < ) + CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( le, <= ) + CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( gt, > ) + CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR( ge, >= ) + + #undef CATCH_INTERNAL_DEFINE_EXPRESSION_COMPARISON_OPERATOR - template>::value, int> = 0> - friend auto operator != ( ExprLhs && lhs, RhsT && rhs ) -> BinaryExpr { - return { compareNotEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "!="_sr, rhs }; - } - template::value, int> = 0> - friend auto operator != ( ExprLhs && lhs, RhsT rhs ) -> BinaryExpr { - return { compareNotEqual( lhs.m_lhs, rhs ), lhs.m_lhs, "!="_sr, rhs }; - } #define CATCH_INTERNAL_DEFINE_EXPRESSION_OPERATOR(op) \ template>::value, int> = 0> \ @@ -213,10 +281,6 @@ namespace Catch { return { static_cast(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(^) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c6f93d96..b64ecd08 100644 --- a/tests/CMakeLists.txt +++ b/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 ) diff --git a/tests/SelfTest/IntrospectiveTests/Traits.tests.cpp b/tests/SelfTest/IntrospectiveTests/Traits.tests.cpp index 35d30c76..459e0d48 100644 --- a/tests/SelfTest/IntrospectiveTests/Traits.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/Traits.tests.cpp @@ -8,36 +8,7 @@ #include #include - -// 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 ::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 -}; +#include #define ADD_TRAIT_TEST_CASE( op ) \ diff --git a/tests/SelfTest/UsageTests/Compilation.tests.cpp b/tests/SelfTest/UsageTests/Compilation.tests.cpp index 5998ce62..a61a9674 100644 --- a/tests/SelfTest/UsageTests/Compilation.tests.cpp +++ b/tests/SelfTest/UsageTests/Compilation.tests.cpp @@ -6,6 +6,8 @@ // SPDX-License-Identifier: BSL-1.0 +#include + #include // 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{} ); +} diff --git a/tests/SelfTest/helpers/type_with_lit_0_comparisons.hpp b/tests/SelfTest/helpers/type_with_lit_0_comparisons.hpp new file mode 100644 index 00000000..202c3af4 --- /dev/null +++ b/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 + +// 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 ::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