From dbc4039905647f139a5376f9d44e86d14c0fe359 Mon Sep 17 00:00:00 2001 From: FranckRJ Date: Mon, 15 Apr 2024 01:30:27 +0200 Subject: [PATCH] Objects passed by rval ref to Return are now always moved and never copied. --- include/fakeit/StubbingProgress.hpp | 79 +++++++++++++++++------------ tests/move_only_return_tests.cpp | 58 +++++++++++---------- tests/stubbing_tests.cpp | 3 +- 3 files changed, 79 insertions(+), 61 deletions(-) diff --git a/include/fakeit/StubbingProgress.hpp b/include/fakeit/StubbingProgress.hpp index a93252e1..d6554788 100644 --- a/include/fakeit/StubbingProgress.hpp +++ b/include/fakeit/StubbingProgress.hpp @@ -38,35 +38,59 @@ namespace fakeit { template struct ParamWalker; - } // namespace helper + template + struct BasicDoImplHelper { + virtual ~BasicDoImplHelper() FAKEIT_THROWS = default; + virtual Self& Do(std::function::type...)> method) { + return DoImpl(new Repeat(method)); + } - template - struct MethodStubbingProgress { + protected: + virtual Self& DoImpl(Action *action) = 0; + }; - virtual ~MethodStubbingProgress() FAKEIT_THROWS { - } + template + struct BasicReturnImplHelper {}; - template - typename std::enable_if::value, MethodStubbingProgress &>::type - Return(const R &r) { - return Do([r](const typename fakeit::test_arg::type...) -> R { return r; }); - } + // If R is a reference. + template + struct BasicReturnImplHelper : public BasicDoImplHelper { + using BasicDoImplHelper::Do; - template - typename std::enable_if::value, MethodStubbingProgress &>::type - Return(const R &r) { - return Do([&r](const typename fakeit::test_arg::type...) -> R { return r; }); - } + Self& Return(const R& r) { + return Do([&r](const typename fakeit::test_arg::type...) -> R { return r; }); + } + }; - template - typename std::enable_if::value, MethodStubbingProgress&>::type - Return(R&& r) { - auto store = std::make_shared(std::move(r)); // work around for lack of move_only_funciton( C++23) - move into a shared_ptr which we can copy. - return Do([store](const typename fakeit::test_arg::type...) mutable -> R { - return std::move(*store); - }); - } + // If R is not a reference. + template + struct BasicReturnImplHelper : public BasicDoImplHelper { + using BasicDoImplHelper::Do; + + Self& Return(const R& r) { + return Do([r](const typename fakeit::test_arg::type...) -> R { return r; }); + } + + Self& Return(R&& r) { + auto store = std::make_shared(std::move(r)); // work around for lack of move_only_funciton( C++23) - move into a shared_ptr which we can copy. + return Do([store](const typename fakeit::test_arg::type...) mutable -> R { + return std::move(*store); + }); + } + }; + } // namespace helper + + + template + struct MethodStubbingProgress : public helper::BasicReturnImplHelper, R, std::is_reference::value, arglist...> { + + protected: + using helper::BasicReturnImplHelper, R, std::is_reference::value, arglist...>::DoImpl; + + public: + using helper::BasicReturnImplHelper, R, std::is_reference::value, arglist...>::Do; + using helper::BasicReturnImplHelper, R, std::is_reference::value, arglist...>::Return; MethodStubbingProgress & Return(const Quantifier &q) { @@ -142,11 +166,6 @@ namespace fakeit { std::forward(arg_vals)...)); } - virtual MethodStubbingProgress & - Do(std::function::type...)> method) { - return DoImpl(new Repeat(method)); - } - template MethodStubbingProgress & Do(const Quantifier &q) { @@ -164,10 +183,6 @@ namespace fakeit { DoImpl(new RepeatForever(method)); } - protected: - - virtual MethodStubbingProgress &DoImpl(Action *action) = 0; - private: MethodStubbingProgress &operator=(const MethodStubbingProgress &other) = delete; diff --git a/tests/move_only_return_tests.cpp b/tests/move_only_return_tests.cpp index dbc4461b..76e7ac5d 100644 --- a/tests/move_only_return_tests.cpp +++ b/tests/move_only_return_tests.cpp @@ -16,35 +16,33 @@ using namespace fakeit; struct MoveOnlyReturnTests: tpunit::TestFixture { - class AbstractType { - public: - virtual ~AbstractType() = default; - virtual void foo() = 0; - }; - - class ConcreteType : public AbstractType { + class MoveOnlyType { public: int state; - ConcreteType(int value) : + MoveOnlyType(int value) : state(value) { } - ConcreteType(const ConcreteType&) = delete; - ConcreteType(ConcreteType&&) = default; - - void foo() override { - } + MoveOnlyType(const MoveOnlyType&) = delete; + MoveOnlyType(MoveOnlyType&&) = default; - bool operator==(const ConcreteType& other) const { + bool operator==(const MoveOnlyType& other) const { return (other.state == this->state); } }; - struct ReferenceInterface { + struct MoveOnlyInterface { virtual std::unique_ptr returnMoveOnlyUniqueString() = 0; - virtual ConcreteType returnMoveOnlyConcreteTypeByRef() = 0; + virtual MoveOnlyType returnMoveOnlyConcreteType() = 0; + virtual std::vector returnVectorOfMoveOnly() = 0; }; + static std::vector constructVectorOfMoveOnly(int i) { + std::vector vectorOfMoveOnly; + vectorOfMoveOnly.emplace_back(i); + return vectorOfMoveOnly; + } + MoveOnlyReturnTests() : tpunit::TestFixture( // @@ -55,33 +53,37 @@ struct MoveOnlyReturnTests: tpunit::TestFixture { } void explicitStubbingReturnValuesFromTemporary() { - Mock mock; + Mock mock; When(Method(mock, returnMoveOnlyUniqueString)).Return(std::unique_ptr(new std::string("value"))); - When(Method(mock, returnMoveOnlyConcreteTypeByRef)).Return(ConcreteType(10)); + When(Method(mock, returnMoveOnlyConcreteType)).Return(MoveOnlyType(10)); + When(Method(mock, returnVectorOfMoveOnly)).Return(constructVectorOfMoveOnly(5)); - ReferenceInterface & i = mock.get(); + MoveOnlyInterface & i = mock.get(); ASSERT_EQUAL("value", *i.returnMoveOnlyUniqueString()); - - ASSERT_EQUAL(ConcreteType(10), i.returnMoveOnlyConcreteTypeByRef()); + ASSERT_EQUAL(MoveOnlyType(10), i.returnMoveOnlyConcreteType()); + ASSERT_EQUAL(constructVectorOfMoveOnly(5), i.returnVectorOfMoveOnly()); } void explicitStubbingReturnValuesFromMove() { - Mock mock; + Mock mock; - ConcreteType c(10); + MoveOnlyType c(10); std::unique_ptr string(new std::string("value")); + std::vector vectorOfC = constructVectorOfMoveOnly(5); When(Method(mock, returnMoveOnlyUniqueString)).Return(std::move(string)); - When(Method(mock, returnMoveOnlyConcreteTypeByRef)).Return(std::move(c)); + When(Method(mock, returnMoveOnlyConcreteType)).Return(std::move(c)); + When(Method(mock, returnVectorOfMoveOnly)).Return(std::move(vectorOfC)); - ASSERT_FALSE(string); // check move did happen + ASSERT_EQUAL(string, nullptr); // check move did happen + ASSERT_TRUE(vectorOfC.empty()); // check move did happen - ReferenceInterface& i = mock.get(); + MoveOnlyInterface& i = mock.get(); ASSERT_EQUAL(std::string("value"), *i.returnMoveOnlyUniqueString()); - - ASSERT_EQUAL(ConcreteType(10), i.returnMoveOnlyConcreteTypeByRef()); + ASSERT_EQUAL(MoveOnlyType(10), i.returnMoveOnlyConcreteType()); + ASSERT_EQUAL(constructVectorOfMoveOnly(5), i.returnVectorOfMoveOnly()); } } __MoveOnlyReturnTests; diff --git a/tests/stubbing_tests.cpp b/tests/stubbing_tests.cpp index bff76c72..ada64205 100644 --- a/tests/stubbing_tests.cpp +++ b/tests/stubbing_tests.cpp @@ -412,7 +412,8 @@ struct BasicStubbing : tpunit::TestFixture { void stub_a_method_with_mutable_lambda_delegate_always() { Mock mock; - When(Method(mock, funcNoArgs)).AlwaysDo([mutableVar = 0]() mutable { + int mutableVar = 0; + When(Method(mock, funcNoArgs)).AlwaysDo([mutableVar]() mutable { return ++mutableVar; });