From c57b5cdf432b1983c6f1b1fd9fb1f7ff11397391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sat, 6 May 2023 14:31:29 +0200 Subject: [PATCH] Move-enable Catch::optional This avoids copies in couple places through Catch2, e.g. reporter spec handling, and moving around `AssertionResult` in `RunContext`. --- src/catch2/internal/catch_optional.hpp | 53 ++++++++++++------- .../IntrospectiveTests/Details.tests.cpp | 41 ++++++++++++++ 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/catch2/internal/catch_optional.hpp b/src/catch2/internal/catch_optional.hpp index ac3714ee..d1e953ad 100644 --- a/src/catch2/internal/catch_optional.hpp +++ b/src/catch2/internal/catch_optional.hpp @@ -8,6 +8,8 @@ #ifndef CATCH_OPTIONAL_HPP_INCLUDED #define CATCH_OPTIONAL_HPP_INCLUDED +#include + #include namespace Catch { @@ -16,35 +18,50 @@ namespace Catch { template class Optional { public: - Optional() : nullableValue( nullptr ) {} - Optional( T const& _value ) - : nullableValue( new( storage ) T( _value ) ) - {} - Optional( Optional const& _other ) - : nullableValue( _other ? new( storage ) T( *_other ) : nullptr ) - {} + Optional(): nullableValue( nullptr ) {} + ~Optional() { reset(); } - ~Optional() { + Optional( T const& _value ): + nullableValue( new ( storage ) T( _value ) ) {} + Optional( T&& _value ): + nullableValue( new ( storage ) T( CATCH_MOVE( _value ) ) ) {} + + Optional& operator=( T const& _value ) { reset(); + nullableValue = new ( storage ) T( _value ); + return *this; + } + Optional& operator=( T&& _value ) { + reset(); + nullableValue = new ( storage ) T( CATCH_MOVE( _value ) ); + return *this; } - Optional& operator= ( Optional const& _other ) { - if( &_other != this ) { + Optional( Optional const& _other ): + nullableValue( _other ? new ( storage ) T( *_other ) : nullptr ) {} + Optional( Optional&& _other ): + nullableValue( _other ? new ( storage ) T( CATCH_MOVE( *_other ) ) + : nullptr ) {} + + Optional& operator=( Optional const& _other ) { + if ( &_other != this ) { reset(); - if( _other ) - nullableValue = new( storage ) T( *_other ); + if ( _other ) { nullableValue = new ( storage ) T( *_other ); } } return *this; } - Optional& operator = ( T const& _value ) { - reset(); - nullableValue = new( storage ) T( _value ); + Optional& operator=( Optional&& _other ) { + if ( &_other != this ) { + reset(); + if ( _other ) { + nullableValue = new ( storage ) T( CATCH_MOVE( *_other ) ); + } + } return *this; } void reset() { - if( nullableValue ) - nullableValue->~T(); + if ( nullableValue ) { nullableValue->~T(); } nullableValue = nullptr; } @@ -91,7 +108,7 @@ namespace Catch { } private: - T *nullableValue; + T* nullableValue; alignas(alignof(T)) char storage[sizeof(T)]; }; diff --git a/tests/SelfTest/IntrospectiveTests/Details.tests.cpp b/tests/SelfTest/IntrospectiveTests/Details.tests.cpp index a5a43926..80eb647b 100644 --- a/tests/SelfTest/IntrospectiveTests/Details.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/Details.tests.cpp @@ -89,6 +89,47 @@ TEST_CASE("Optional comparison ops", "[optional][approvals]") { } } +namespace { + struct MoveChecker { + bool has_moved = false; + MoveChecker() = default; + MoveChecker( MoveChecker const& rhs ) = default; + MoveChecker& operator=( MoveChecker const& rhs ) = default; + MoveChecker( MoveChecker&& rhs ) { rhs.has_moved = true; } + MoveChecker& operator=( MoveChecker&& rhs ) { + rhs.has_moved = true; + return *this; + } + }; +} + +TEST_CASE( "Optional supports move ops", "[optional][approvals]" ) { + using Catch::Optional; + MoveChecker a; + Optional opt_A( a ); + REQUIRE_FALSE( a.has_moved ); + REQUIRE_FALSE( opt_A->has_moved ); + + SECTION( "Move construction from element" ) { + Optional opt_B( CATCH_MOVE( a ) ); + REQUIRE( a.has_moved ); + } + SECTION( "Move assignment from element" ) { + opt_A = CATCH_MOVE( a ); + REQUIRE( a.has_moved ); + } + SECTION( "Move construction from optional" ) { + Optional opt_B( CATCH_MOVE( opt_A ) ); + REQUIRE( opt_A->has_moved ); + } + SECTION( "Move assignment from optional" ) { + Optional opt_B( opt_A ); + REQUIRE_FALSE( opt_A->has_moved ); + opt_B = CATCH_MOVE( opt_A ); + REQUIRE( opt_A->has_moved ); + } +} + TEST_CASE( "Decomposer checks that the argument is 0 when handling " "only-0-comparable types", "[decomposition][approvals]" ) {