From d9f9fce6fc5c8bfcea2f882a39533adad10b0045 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 14 Jul 2020 21:17:14 +0200 Subject: [PATCH] Make action classes non-relocatable This fixes an issue with auto_ptr. As shown by missing coverage the actual copy/move ctors are not required as C++11 emplace functions can be used. Furthermore some places using shared_ptr could be replaced by unique_ptr leading to better performance. --- include/turtle/detail/action.hpp | 51 +++++++------------ .../turtle/detail/expectation_template.hpp | 20 ++++---- .../turtle/detail/function_impl_template.hpp | 16 +++--- include/turtle/detail/type_name.hpp | 9 ++-- test/detail/test_function.cpp | 13 +++++ 5 files changed, 54 insertions(+), 55 deletions(-) diff --git a/include/turtle/detail/action.hpp b/include/turtle/detail/action.hpp index 204b4f2..1f3a1b8 100644 --- a/include/turtle/detail/action.hpp +++ b/include/turtle/detail/action.hpp @@ -25,7 +25,16 @@ namespace detail typedef std::function< Signature > functor_type; typedef std::function< Result() > action_type; + protected: + // Meant to be subclassed and not be directly used + // Non-relocatable (contained functions may wrap references/pointers which could be invalidated) + action_base() = default; + action_base(const action_base&) = delete; + action_base(action_base&&) = delete; + action_base& operator=(const action_base&) = delete; + action_base& operator=(action_base&&) = delete; public: + const functor_type& functor() const { return f_; @@ -91,11 +100,6 @@ namespace detail } private: - template< typename Value > - static Value&& move( Value& t ) - { - return std::move( t ); - } struct value { value() = default; @@ -106,41 +110,28 @@ namespace detail template< typename T > struct value_imp : value { - typedef std::remove_const_t> value_type; + typedef std::remove_const_t> type; - value_imp( value_type&& t ) - : t_( std::move( t ) ) + template< typename U > + value_imp( U&& t ) : t_( std::forward( t ) ) {} - value_imp( const value_type& t ) - : t_( t ) - {} - template< typename Y > - value_imp( Y* y ) - : t_( y ) - {} - value_type t_; + type t_; }; template< typename T > - T& store( T&& t ) + typename value_imp::type& store( T&& t ) { - v_ = std::make_shared< value_imp >( std::move( t ) ); - return static_cast< value_imp< T >& >( *v_ ).t_; - } - template< typename T > - T& store( const T& t ) - { - v_ = std::make_shared< value_imp >( t ); + v_ = std::make_unique< value_imp >( std::forward( t ) ); return static_cast< value_imp< T >& >( *v_ ).t_; } template< typename T > std::remove_reference_t< Result >& store( T* t ) { - v_ = std::make_shared< value_imp >( t ); + v_ = std::make_unique< value_imp >( t ); return static_cast< value_imp< Result >& >( *v_ ).t_; } - std::shared_ptr< value > v_; + std::unique_ptr< value > v_; }; template< typename Signature > @@ -159,14 +150,6 @@ namespace detail : public action_base< std::auto_ptr< Result >, Signature > { public: - action() = default; - action( const action& rhs ) - : v_( rhs.v_.release() ) - { - if( v_.get() ) - returns( std::ref( v_ ) ); - } - template< typename Y > void returns( Y* r ) { diff --git a/include/turtle/detail/expectation_template.hpp b/include/turtle/detail/expectation_template.hpp index ea2926a..d3e74bd 100644 --- a/include/turtle/detail/expectation_template.hpp +++ b/include/turtle/detail/expectation_template.hpp @@ -133,9 +133,9 @@ namespace detail { public: expectation() - : invocation_( std::make_shared< unlimited >() ) + : invocation_( std::make_unique< unlimited >() ) , matcher_( - std::make_shared< + std::make_unique< default_matcher< void( BOOST_PP_ENUM_PARAMS(MOCK_NUM_ARGS, T) ) > @@ -144,9 +144,9 @@ namespace detail , line_( 0 ) {} expectation( const char* file, int line ) - : invocation_( std::make_shared< unlimited >() ) + : invocation_( std::make_unique< unlimited >() ) , matcher_( - std::make_shared< + std::make_unique< default_matcher< void( BOOST_PP_ENUM_PARAMS(MOCK_NUM_ARGS, T) ) > @@ -166,9 +166,9 @@ namespace detail sequence->remove( this ); } - void invoke( const std::shared_ptr< invocation >& i ) + void invoke( std::unique_ptr< invocation > i ) { - invocation_ = i; + invocation_ = std::move(i); } #ifndef MOCK_NUM_ARGS_0 @@ -178,7 +178,7 @@ namespace detail expectation& with( BOOST_PP_ENUM_BINARY_PARAMS(MOCK_NUM_ARGS, Constraint_, c) ) { - matcher_ = std::make_shared< + matcher_ = std::make_unique< single_matcher< void( BOOST_PP_ENUM_PARAMS(MOCK_NUM_ARGS, Constraint_) ), void( BOOST_PP_ENUM_PARAMS(MOCK_NUM_ARGS, T) ) @@ -189,7 +189,7 @@ namespace detail template< typename Constraint > expectation& with( const Constraint& c ) { - matcher_ = std::make_shared< + matcher_ = std::make_unique< multi_matcher< Constraint, void( BOOST_PP_ENUM_PARAMS(MOCK_NUM_ARGS, T) ) @@ -250,8 +250,8 @@ namespace detail } private: - std::shared_ptr< invocation > invocation_; - std::shared_ptr< + std::unique_ptr< invocation > invocation_; + std::unique_ptr< matcher_base< void( BOOST_PP_ENUM_PARAMS(MOCK_NUM_ARGS, T) ) > diff --git a/include/turtle/detail/function_impl_template.hpp b/include/turtle/detail/function_impl_template.hpp index 727632d..c5a212d 100644 --- a/include/turtle/detail/function_impl_template.hpp +++ b/include/turtle/detail/function_impl_template.hpp @@ -125,32 +125,32 @@ namespace detail } wrapper& once() { - this->e_->invoke( std::make_shared< detail::once >() ); + this->e_->invoke( std::make_unique< detail::once >() ); return *this; } wrapper& never() { - this->e_->invoke( std::make_shared< detail::never >() ); + this->e_->invoke( std::make_unique< detail::never >() ); return *this; } wrapper& exactly( std::size_t count ) { - this->e_->invoke( std::make_shared< detail::exactly >( count ) ); + this->e_->invoke( std::make_unique< detail::exactly >( count ) ); return *this; } wrapper& at_least( std::size_t min ) { - this->e_->invoke( std::make_shared< detail::at_least >( min ) ); + this->e_->invoke( std::make_unique< detail::at_least >( min ) ); return *this; } wrapper& at_most( std::size_t max ) { - this->e_->invoke( std::make_shared< detail::at_most >( max ) ); + this->e_->invoke( std::make_unique< detail::at_most >( max ) ); return *this; } wrapper& between( std::size_t min, std::size_t max ) { - this->e_->invoke( std::make_shared< detail::between >( min, max ) ); + this->e_->invoke( std::make_unique< detail::between >( min, max ) ); return *this; } @@ -217,14 +217,14 @@ namespace detail wrapper expect( const char* file, int line ) { lock _( mutex_ ); - expectations_.push_back( expectation_type( file, line ) ); + expectations_.emplace_back( file, line ); valid_ = true; return wrapper( mutex_, expectations_.back() ); } wrapper expect() { lock _( mutex_ ); - expectations_.push_back( expectation_type() ); + expectations_.emplace_back( ); valid_ = true; return wrapper( mutex_, expectations_.back() ); } diff --git a/include/turtle/detail/type_name.hpp b/include/turtle/detail/type_name.hpp index 72e3025..7f84193 100644 --- a/include/turtle/detail/type_name.hpp +++ b/include/turtle/detail/type_name.hpp @@ -45,9 +45,12 @@ namespace detail const char* name = info.name(); #ifdef __GNUC__ int status = 0; - std::shared_ptr< char > demangled( - abi::__cxa_demangle( name, 0, 0, &status ), - &std::free ); + struct Deleter + { + void operator()(const void* p) { std::free(const_cast(p)); } + }; + std::unique_ptr< const char, Deleter > demangled( + abi::__cxa_demangle( name, 0, 0, &status )); if( ! status && demangled ) serialize( s, demangled.get() ); else diff --git a/test/detail/test_function.cpp b/test/detail/test_function.cpp index 46e5d5c..110bf1b 100644 --- a/test/detail/test_function.cpp +++ b/test/detail/test_function.cpp @@ -535,6 +535,19 @@ BOOST_FIXTURE_TEST_CASE( triggering_an_expectation_returns_the_set_auto_ptr_valu BOOST_CHECK( ! f().get() ); CHECK_CALLS( 2 ); } + { + mock::detail::function< std::auto_ptr< int >() > f; + f.expect().once().returns( new int( 1 ) ); + f.expect().once().returns( new int( 2 ) ); + f.expect().once().returns( new int( 3 ) ); + f.expect().returns( new int( 4 ) ); + BOOST_CHECK_EQUAL( 1, *f() ); + BOOST_CHECK_EQUAL( 2, *f() ); + BOOST_CHECK_EQUAL( 3, *f() ); + BOOST_CHECK_EQUAL( 4, *f() ); + BOOST_CHECK( ! f().get() ); + CHECK_CALLS( 5 ); + } { mock::detail::function< std::auto_ptr< int >() > f; f.expect().returns( std::auto_ptr< int >( new int( 3 ) ) );