Stop accepting non-const comparison operators

A) non-const comparison operators should not exist and should not be
encouraged

B) The logic breaks comparing function pointers certain way

C) It was inconsistent anyway, as it only applied to `==` and `!=`

Closes #925
This commit is contained in:
Martin Hořeňovský 2017-09-06 14:42:44 +02:00
parent aef2e4d9e7
commit b000411434
6 changed files with 56 additions and 58 deletions

View File

@ -76,7 +76,7 @@ namespace Catch {
// Specialised comparison functions to handle equality comparisons between ints and pointers (NULL deduces as an int) // Specialised comparison functions to handle equality comparisons between ints and pointers (NULL deduces as an int)
template<typename LhsT, typename RhsT> template<typename LhsT, typename RhsT>
auto compareEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return const_cast<LhsT&>( lhs ) == rhs; }; auto compareEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return lhs == rhs; };
template<typename T> template<typename T>
auto compareEqual( T* const& lhs, int rhs ) -> bool { return lhs == reinterpret_cast<void const*>( rhs ); }; auto compareEqual( T* const& lhs, int rhs ) -> bool { return lhs == reinterpret_cast<void const*>( rhs ); };
template<typename T> template<typename T>
@ -87,7 +87,7 @@ namespace Catch {
auto compareEqual( long lhs, T* const& rhs ) -> bool { return reinterpret_cast<void const*>( lhs ) == rhs; }; auto compareEqual( long lhs, T* const& rhs ) -> bool { return reinterpret_cast<void const*>( lhs ) == rhs; };
template<typename LhsT, typename RhsT> template<typename LhsT, typename RhsT>
auto compareNotEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return const_cast<LhsT&>( lhs ) != rhs; }; auto compareNotEqual( LhsT const& lhs, RhsT&& rhs ) -> bool { return lhs != rhs; };
template<typename T> template<typename T>
auto compareNotEqual( T* const& lhs, int rhs ) -> bool { return lhs != reinterpret_cast<void const*>( rhs ); }; auto compareNotEqual( T* const& lhs, int rhs ) -> bool { return lhs != reinterpret_cast<void const*>( rhs ); };
template<typename T> template<typename T>
@ -150,10 +150,6 @@ namespace Catch {
} }
struct Decomposer { struct Decomposer {
template<typename T>
auto operator <= ( T& lhs ) -> ExprLhs<T&> {
return ExprLhs<T&>( lhs );
}
template<typename T> template<typename T>
auto operator <= ( T const& lhs ) -> ExprLhs<T const&> { auto operator <= ( T const& lhs ) -> ExprLhs<T const&> {
return ExprLhs<T const&>( lhs ); return ExprLhs<T const&>( lhs );

View File

@ -156,6 +156,18 @@ with expansion:
with message: with message:
dummy := 0 dummy := 0
-------------------------------------------------------------------------------
#925: comparing function pointer to function address failed to compile
-------------------------------------------------------------------------------
TrickyTests.cpp:<line number>
...............................................................................
TrickyTests.cpp:<line number>:
PASSED:
REQUIRE( utility::synchronizing_callback != test.testMethod_uponComplete_arg )
with expansion:
1 != 0
------------------------------------------------------------------------------- -------------------------------------------------------------------------------
#961 -- Dynamically created sections should all be reported #961 -- Dynamically created sections should all be reported
Looped section 0 Looped section 0
@ -1215,18 +1227,6 @@ ExceptionTests.cpp:<line number>: FAILED:
due to unexpected exception with message: due to unexpected exception with message:
custom std exception custom std exception
-------------------------------------------------------------------------------
Demonstrate that a non-const == is not used
-------------------------------------------------------------------------------
TrickyTests.cpp:<line number>
...............................................................................
TrickyTests.cpp:<line number>:
PASSED:
REQUIRE( t == 1u )
with expansion:
{?} == 1
------------------------------------------------------------------------------- -------------------------------------------------------------------------------
EndsWith string matcher EndsWith string matcher
------------------------------------------------------------------------------- -------------------------------------------------------------------------------

View File

@ -156,6 +156,18 @@ with expansion:
with message: with message:
dummy := 0 dummy := 0
-------------------------------------------------------------------------------
#925: comparing function pointer to function address failed to compile
-------------------------------------------------------------------------------
TrickyTests.cpp:<line number>
...............................................................................
TrickyTests.cpp:<line number>:
PASSED:
REQUIRE( utility::synchronizing_callback != test.testMethod_uponComplete_arg )
with expansion:
1 != 0
------------------------------------------------------------------------------- -------------------------------------------------------------------------------
#961 -- Dynamically created sections should all be reported #961 -- Dynamically created sections should all be reported
Looped section 0 Looped section 0
@ -237,6 +249,6 @@ ConditionTests.cpp:<line number>: FAILED:
CHECK_FALSE( true ) CHECK_FALSE( true )
=============================================================================== ===============================================================================
test cases: 9 | 6 passed | 1 failed | 2 failed as expected test cases: 10 | 7 passed | 1 failed | 2 failed as expected
assertions: 26 | 19 passed | 4 failed | 3 failed as expected assertions: 27 | 20 passed | 4 failed | 3 failed as expected

View File

@ -27,6 +27,7 @@ MiscTests.cpp:<line number>
</failure> </failure>
</testcase> </testcase>
<testcase classname="<exe-name>.global" name="#872" time="{duration}"/> <testcase classname="<exe-name>.global" name="#872" time="{duration}"/>
<testcase classname="<exe-name>.global" name="#925: comparing function pointer to function address failed to compile" time="{duration}"/>
<testcase classname="<exe-name>.global" name="#961 -- Dynamically created sections should all be reported/Looped section 0" time="{duration}"/> <testcase classname="<exe-name>.global" name="#961 -- Dynamically created sections should all be reported/Looped section 0" time="{duration}"/>
<testcase classname="<exe-name>.global" name="#961 -- Dynamically created sections should all be reported/Looped section 1" time="{duration}"/> <testcase classname="<exe-name>.global" name="#961 -- Dynamically created sections should all be reported/Looped section 1" time="{duration}"/>
<testcase classname="<exe-name>.global" name="#961 -- Dynamically created sections should all be reported/Looped section 2" time="{duration}"/> <testcase classname="<exe-name>.global" name="#961 -- Dynamically created sections should all be reported/Looped section 2" time="{duration}"/>
@ -143,7 +144,6 @@ custom std exception
ExceptionTests.cpp:<line number> ExceptionTests.cpp:<line number>
</error> </error>
</testcase> </testcase>
<testcase classname="<exe-name>.global" name="Demonstrate that a non-const == is not used" time="{duration}"/>
<testcase classname="<exe-name>.global" name="EndsWith string matcher" time="{duration}"> <testcase classname="<exe-name>.global" name="EndsWith string matcher" time="{duration}">
<failure message="&quot;this string contains 'abc' as a substring&quot; ends with: &quot;this&quot;" type="CHECK_THAT"> <failure message="&quot;this string contains 'abc' as a substring&quot; ends with: &quot;this&quot;" type="CHECK_THAT">
MatchersTests.cpp:<line number> MatchersTests.cpp:<line number>

View File

@ -169,6 +169,17 @@
</Expression> </Expression>
<OverallResult success="true"/> <OverallResult success="true"/>
</TestCase> </TestCase>
<TestCase name="#925: comparing function pointer to function address failed to compile" filename="projects/<exe-name>/TrickyTests.cpp" >
<Expression success="true" type="REQUIRE" filename="projects/<exe-name>/TrickyTests.cpp" >
<Original>
utility::synchronizing_callback != test.testMethod_uponComplete_arg
</Original>
<Expanded>
1 != 0
</Expanded>
</Expression>
<OverallResult success="true"/>
</TestCase>
<TestCase name="#961 -- Dynamically created sections should all be reported" tags="[.]" filename="projects/<exe-name>/MiscTests.cpp" > <TestCase name="#961 -- Dynamically created sections should all be reported" tags="[.]" filename="projects/<exe-name>/MiscTests.cpp" >
<Section name="Looped section 0" filename="projects/<exe-name>/MiscTests.cpp" > <Section name="Looped section 0" filename="projects/<exe-name>/MiscTests.cpp" >
<OverallResults successes="1" failures="0" expectedFailures="0"/> <OverallResults successes="1" failures="0" expectedFailures="0"/>
@ -1336,17 +1347,6 @@
</Exception> </Exception>
<OverallResult success="false"/> <OverallResult success="false"/>
</TestCase> </TestCase>
<TestCase name="Demonstrate that a non-const == is not used" tags="[Tricky]" filename="projects/<exe-name>/TrickyTests.cpp" >
<Expression success="true" type="REQUIRE" filename="projects/<exe-name>/TrickyTests.cpp" >
<Original>
t == 1u
</Original>
<Expanded>
{?} == 1
</Expanded>
</Expression>
<OverallResult success="true"/>
</TestCase>
<TestCase name="EndsWith string matcher" tags="[.][failing][matchers]" filename="projects/<exe-name>/MatchersTests.cpp" > <TestCase name="EndsWith string matcher" tags="[.][failing][matchers]" filename="projects/<exe-name>/MatchersTests.cpp" >
<Expression success="false" type="CHECK_THAT" filename="projects/<exe-name>/MatchersTests.cpp" > <Expression success="false" type="CHECK_THAT" filename="projects/<exe-name>/MatchersTests.cpp" >
<Original> <Original>

View File

@ -183,32 +183,6 @@ namespace ObjectWithConversions
} }
} }
namespace ObjectWithNonConstEqualityOperator
{
struct Test
{
Test( unsigned int v )
: m_value(v)
{}
bool operator==( const Test&rhs )
{
return (m_value == rhs.m_value);
}
bool operator==( const Test&rhs ) const
{
return (m_value != rhs.m_value);
}
unsigned int m_value;
};
TEST_CASE("Demonstrate that a non-const == is not used", "[Tricky]" )
{
Test t( 1 );
REQUIRE( t == 1u );
}
}
namespace EnumBitFieldTests namespace EnumBitFieldTests
{ {
enum Bits : uint32_t { enum Bits : uint32_t {
@ -448,3 +422,19 @@ TEST_CASE( "non-copyable objects", "[.][failing]" ) {
std::type_info const& ti = typeid(int); std::type_info const& ti = typeid(int);
CHECK( ti == typeid(int) ); CHECK( ti == typeid(int) );
} }
// #925
using signal_t = void (*) (void*);
struct TestClass {
signal_t testMethod_uponComplete_arg = nullptr;
};
namespace utility {
inline static void synchronizing_callback( void * ) { }
}
TEST_CASE("#925: comparing function pointer to function address failed to compile") {
TestClass test;
REQUIRE(utility::synchronizing_callback != test.testMethod_uponComplete_arg);
}