From a1cc8c55195661a58ad60c3bb062a0b9c302710d Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Fri, 8 Apr 2022 18:39:39 -0700 Subject: [PATCH] Add support for move-only and &&-qualified actions in WillOnce. This provides a type-safe way for an action to express that it wants to be called only once, or to capture move-only objects. It is a generalization of the type system-evading hack in ByMove, with the improvement that it works for _any_ action (including user-defined ones), and correctly expresses that the action can only be used with WillOnce. I'll make existing actions benefit in a future commit. PiperOrigin-RevId: 440496139 Change-Id: I4145d191cca5655995ef41360bb126c123cb41d3 --- docs/reference/mocking.md | 4 +- googlemock/include/gmock/gmock-actions.h | 269 +++++++++++++- .../include/gmock/gmock-spec-builders.h | 8 +- googlemock/test/gmock-actions_test.cc | 338 ++++++++++++++++-- 4 files changed, 584 insertions(+), 35 deletions(-) diff --git a/docs/reference/mocking.md b/docs/reference/mocking.md index c29f7160..e414ffbd 100644 --- a/docs/reference/mocking.md +++ b/docs/reference/mocking.md @@ -248,7 +248,9 @@ EXPECT_CALL(my_mock, GetNumber()) .WillOnce(Return(3)); ``` -The `WillOnce` clause can be used any number of times on an expectation. +The `WillOnce` clause can be used any number of times on an expectation. Unlike +`WillRepeatedly`, the action fed to each `WillOnce` call will be called at most +once, so may be a move-only type and/or have an `&&`-qualified call operator. #### WillRepeatedly {#EXPECT_CALL.WillRepeatedly} diff --git a/googlemock/include/gmock/gmock-actions.h b/googlemock/include/gmock/gmock-actions.h index 2c4ce363..4ae18dac 100644 --- a/googlemock/include/gmock/gmock-actions.h +++ b/googlemock/include/gmock/gmock-actions.h @@ -262,9 +262,65 @@ GMOCK_DEFINE_DEFAULT_ACTION_FOR_RETURN_TYPE_(double, 0); #undef GMOCK_DEFINE_DEFAULT_ACTION_FOR_RETURN_TYPE_ -// Simple two-arg form of std::disjunction. -template -using disjunction = typename ::std::conditional::type; +// Partial implementations of metaprogramming types from the standard library +// not available in C++11. + +template +struct negation + // NOLINTNEXTLINE + : std::integral_constant {}; + +// Base case: with zero predicates the answer is always true. +template +struct conjunction : std::true_type {}; + +// With a single predicate, the answer is that predicate. +template +struct conjunction : P1 {}; + +// With multiple predicates the answer is the first predicate if that is false, +// and we recurse otherwise. +template +struct conjunction + : std::conditional, P1>::type {}; + +template +struct disjunction : std::false_type {}; + +template +struct disjunction : P1 {}; + +template +struct disjunction + // NOLINTNEXTLINE + : std::conditional, P1>::type {}; + +template +using void_t = void; + +// Like std::invoke_result_t from C++17, but works only for objects with call +// operators (not e.g. member function pointers, which we don't need specific +// support for in OnceAction because std::function deals with them). +template +using call_result_t = decltype(std::declval()(std::declval()...)); + +template +struct is_callable_r_impl : std::false_type {}; + +// Specialize the struct for those template arguments where call_result_t is +// well-formed. When it's not, the generic template above is chosen, resulting +// in std::false_type. +template +struct is_callable_r_impl>, R, F, Args...> + : std::conditional< + std::is_same::value, // + std::true_type, // + std::is_convertible, R>>::type {}; + +// Like std::is_invocable_r from C++17, but works only for objects with call +// operators. See the note on call_result_t. +template +using is_callable_r = is_callable_r_impl; } // namespace internal @@ -596,6 +652,213 @@ inline PolymorphicAction MakePolymorphicAction(const Impl& impl) { namespace internal { +template +class TypedExpectation; + +// Specialized for function types below. +template +class OnceAction; + +// An action that can only be used once. +// +// This is what is accepted by WillOnce, which doesn't require the underlying +// action to be copy-constructible (only move-constructible), and promises to +// invoke it as an rvalue reference. This allows the action to work with +// move-only types like std::move_only_function in a type-safe manner. +// +// For example: +// +// // Assume we have some API that needs to accept a unique pointer to some +// // non-copyable object Foo. +// void AcceptUniquePointer(std::unique_ptr foo); +// +// // We can define an action that provides a Foo to that API. Because It +// // has to give away its unique pointer, it must not be called more than +// // once, so its call operator is &&-qualified. +// struct ProvideFoo { +// std::unique_ptr foo; +// +// void operator()() && { +// AcceptUniquePointer(std::move(Foo)); +// } +// }; +// +// // This action can be used with WillOnce. +// EXPECT_CALL(mock, Call) +// .WillOnce(ProvideFoo{std::make_unique(...)}); +// +// // But a call to WillRepeatedly will fail to compile. This is correct, +// // since the action cannot correctly be used repeatedly. +// EXPECT_CALL(mock, Call) +// .WillRepeatedly(ProvideFoo{std::make_unique(...)}); +// +// A less-contrived example would be an action that returns an arbitrary type, +// whose &&-qualified call operator is capable of dealing with move-only types. +template +class OnceAction final { + private: + // True iff we can use the given callable type (or lvalue reference) directly + // via ActionAdaptor. + template + using IsDirectlyCompatible = internal::conjunction< + // It must be possible to capture the callable in ActionAdaptor. + std::is_constructible::type, Callable>, + // The callable must be compatible with our signature. + internal::is_callable_r::type, + Args...>>; + + // True iff we can use the given callable type via ActionAdaptor once we + // ignore incoming arguments. + template + using IsCompatibleAfterIgnoringArguments = internal::conjunction< + // It must be possible to capture the callable in a lambda. + std::is_constructible::type, Callable>, + // The callable must be invocable with zero arguments, returning something + // convertible to Result. + internal::is_callable_r::type>>; + + public: + // Construct from a callable that is directly compatible with our mocked + // signature: it accepts our function type's arguments and returns something + // convertible to our result type. + template ::type>>, + IsDirectlyCompatible> // + ::value, + int>::type = 0> + OnceAction(Callable&& callable) // NOLINT + : action_(ActionAdaptor::type>( + {}, std::forward(callable))) {} + + // As above, but for a callable that ignores the mocked function's arguments. + template ::type>>, + // Exclude callables for which the overload above works. + // We'd rather provide the arguments if possible. + internal::negation>, + IsCompatibleAfterIgnoringArguments>::value, + int>::type = 0> + OnceAction(Callable&& callable) // NOLINT + // Call the constructor above with a callable + // that ignores the input arguments. + : OnceAction(IgnoreIncomingArguments::type>{ + std::forward(callable)}) {} + + // A fallback constructor for anything that is convertible to Action, for use + // with legacy actions that uses older styles like implementing + // ActionInterface or a conversion operator to Action. Modern code should + // implement a call operator with appropriate restrictions. + template ::type>>, + // Exclude the overloads above, which we want to take + // precedence. + internal::negation>, + internal::negation>, + // It must be possible to turn the object into an action of + // the appropriate type. + std::is_convertible> // + >::value, + int>::type = 0> + OnceAction(T&& action) : action_(std::forward(action)) {} // NOLINT + + // We are naturally copyable because we store only an Action, but semantically + // we should not be copyable. + OnceAction(const OnceAction&) = delete; + OnceAction& operator=(const OnceAction&) = delete; + OnceAction(OnceAction&&) = default; + + private: + // Allow TypedExpectation::WillOnce to use our type-unsafe API below. + friend class TypedExpectation; + + // An adaptor that wraps a callable that is compatible with our signature and + // being invoked as an rvalue reference so that it can be used as an + // Action. This throws away type safety, but that's fine because this is only + // used by WillOnce, which we know calls at most once. + template + class ActionAdaptor final { + public: + // A tag indicating that the (otherwise universal) constructor is accepting + // the callable itself, instead of e.g. stealing calls for the move + // constructor. + struct CallableTag final {}; + + template + explicit ActionAdaptor(CallableTag, F&& callable) + : callable_(std::make_shared(std::forward(callable))) {} + + // Rather than explicitly returning Result, we return whatever the wrapped + // callable returns. This allows for compatibility with existing uses like + // the following, when the mocked function returns void: + // + // EXPECT_CALL(mock_fn_, Call) + // .WillOnce([&] { + // [...] + // return 0; + // }); + // + // This works with Action since such a callable can be turned into + // std::function. If we use an explicit return type of Result here + // then it *doesn't* work with OnceAction, because we'll get a "void + // function should not return a value" error. + // + // We need not worry about incompatible result types because the SFINAE on + // OnceAction already checks this for us. std::is_invocable_r_v itself makes + // the same allowance for void result types. + template + internal::call_result_t operator()( + ArgRefs&&... args) const { + return std::move(*callable_)(std::forward(args)...); + } + + private: + // We must put the callable on the heap so that we are copyable, which + // Action needs. + std::shared_ptr callable_; + }; + + // An adaptor that makes a callable that accepts zero arguments callable with + // our mocked arguments. + template + struct IgnoreIncomingArguments { + internal::call_result_t operator()(Args&&...) { + return std::move(callable)(); + } + + Callable callable; + }; + + // Return an Action that calls the underlying callable in a type-safe manner. + // The action's Perform method must be called at most once. + // + // This is the transition from a type-safe API to a type-unsafe one, since + // "must be called at most once" is no longer reflecting in the type system. + Action ReleaseAction() && { return std::move(action_); } + + Action action_; +}; + // Helper struct to specialize ReturnAction to execute a move instead of a copy // on return. Useful for move-only types, but could be used on any type. template diff --git a/googlemock/include/gmock/gmock-spec-builders.h b/googlemock/include/gmock/gmock-spec-builders.h index 8e4c5c62..917fa9ae 100644 --- a/googlemock/include/gmock/gmock-spec-builders.h +++ b/googlemock/include/gmock/gmock-spec-builders.h @@ -992,14 +992,16 @@ class TypedExpectation : public ExpectationBase { return After(s1, s2, s3, s4).After(s5); } - // Implements the .WillOnce() clause. - TypedExpectation& WillOnce(const Action& action) { + // Implements the .WillOnce() clause for copyable actions. + TypedExpectation& WillOnce(OnceAction once_action) { ExpectSpecProperty(last_clause_ <= kWillOnce, ".WillOnce() cannot appear after " ".WillRepeatedly() or .RetiresOnSaturation()."); last_clause_ = kWillOnce; - untyped_actions_.push_back(new Action(action)); + untyped_actions_.push_back( + new Action(std::move(once_action).ReleaseAction())); + if (!cardinality_specified()) { set_cardinality(Exactly(static_cast(untyped_actions_.size()))); } diff --git a/googlemock/test/gmock-actions_test.cc b/googlemock/test/gmock-actions_test.cc index 233b60cb..7ff5780c 100644 --- a/googlemock/test/gmock-actions_test.cc +++ b/googlemock/test/gmock-actions_test.cc @@ -55,37 +55,170 @@ #include "gtest/gtest-spi.h" #include "gtest/gtest.h" +namespace testing { namespace { -using ::testing::_; -using ::testing::Action; -using ::testing::ActionInterface; -using ::testing::Assign; -using ::testing::ByMove; -using ::testing::ByRef; -using ::testing::DefaultValue; -using ::testing::DoAll; -using ::testing::DoDefault; -using ::testing::IgnoreResult; -using ::testing::Invoke; -using ::testing::InvokeWithoutArgs; -using ::testing::MakePolymorphicAction; -using ::testing::PolymorphicAction; -using ::testing::Return; -using ::testing::ReturnNew; -using ::testing::ReturnNull; -using ::testing::ReturnRef; -using ::testing::ReturnRefOfCopy; -using ::testing::ReturnRoundRobin; -using ::testing::SetArgPointee; -using ::testing::SetArgumentPointee; -using ::testing::Unused; -using ::testing::WithArgs; using ::testing::internal::BuiltInDefaultValue; -#if !GTEST_OS_WINDOWS_MOBILE -using ::testing::SetErrnoAndReturn; -#endif +TEST(TypeTraits, Negation) { + // Direct use with std types. + static_assert(std::is_base_of>::value, + ""); + + static_assert(std::is_base_of>::value, + ""); + + // With other types that fit the requirement of a value member that is + // convertible to bool. + static_assert(std::is_base_of< + std::true_type, + internal::negation>>::value, + ""); + + static_assert(std::is_base_of< + std::false_type, + internal::negation>>::value, + ""); + + static_assert(std::is_base_of< + std::false_type, + internal::negation>>::value, + ""); +} + +// Weird false/true types that aren't actually bool constants (but should still +// be legal according to [meta.logical] because `bool(T::value)` is valid), are +// distinct from std::false_type and std::true_type, and are distinct from other +// instantiations of the same template. +// +// These let us check finicky details mandated by the standard like +// "std::conjunction should evaluate to a type that inherits from the first +// false-y input". +template +struct MyFalse : std::integral_constant {}; + +template +struct MyTrue : std::integral_constant {}; + +TEST(TypeTraits, Conjunction) { + // Base case: always true. + static_assert(std::is_base_of>::value, + ""); + + // One predicate: inherits from that predicate, regardless of value. + static_assert( + std::is_base_of, internal::conjunction>>::value, + ""); + + static_assert( + std::is_base_of, internal::conjunction>>::value, ""); + + // Multiple predicates, with at least one false: inherits from that one. + static_assert( + std::is_base_of, internal::conjunction, MyFalse<1>, + MyTrue<2>>>::value, + ""); + + static_assert( + std::is_base_of, internal::conjunction, MyFalse<1>, + MyFalse<2>>>::value, + ""); + + // Short circuiting: in the case above, additional predicates need not even + // define a value member. + struct Empty {}; + static_assert( + std::is_base_of, internal::conjunction, MyFalse<1>, + Empty>>::value, + ""); + + // All predicates true: inherits from the last. + static_assert( + std::is_base_of, internal::conjunction, MyTrue<1>, + MyTrue<2>>>::value, + ""); +} + +TEST(TypeTraits, Disjunction) { + // Base case: always false. + static_assert( + std::is_base_of>::value, ""); + + // One predicate: inherits from that predicate, regardless of value. + static_assert( + std::is_base_of, internal::disjunction>>::value, + ""); + + static_assert( + std::is_base_of, internal::disjunction>>::value, ""); + + // Multiple predicates, with at least one true: inherits from that one. + static_assert( + std::is_base_of, internal::disjunction, MyTrue<1>, + MyFalse<2>>>::value, + ""); + + static_assert( + std::is_base_of, internal::disjunction, MyTrue<1>, + MyTrue<2>>>::value, + ""); + + // Short circuiting: in the case above, additional predicates need not even + // define a value member. + struct Empty {}; + static_assert( + std::is_base_of, internal::disjunction, MyTrue<1>, + Empty>>::value, + ""); + + // All predicates false: inherits from the last. + static_assert( + std::is_base_of, internal::disjunction, MyFalse<1>, + MyFalse<2>>>::value, + ""); +} + +TEST(TypeTraits, IsInvocableRV) { + struct C { + int operator()() const { return 0; } + void operator()(int) & {} + std::string operator()(int) && { return ""; }; + }; + + // The first overload is callable for const and non-const rvalues and lvalues. + // It can be used to obtain an int, void, or anything int is convertible too. + static_assert(internal::is_callable_r::value, ""); + static_assert(internal::is_callable_r::value, ""); + static_assert(internal::is_callable_r::value, ""); + static_assert(internal::is_callable_r::value, ""); + + static_assert(internal::is_callable_r::value, ""); + static_assert(internal::is_callable_r::value, ""); + + // It's possible to provide an int. If it's given to an lvalue, the result is + // void. Otherwise it is std::string (which is also treated as allowed for a + // void result type). + static_assert(internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); + + static_assert(internal::is_callable_r::value, ""); + static_assert(internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); + + // It's not possible to provide other arguments. + static_assert(!internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); + + // Nothing should choke when we try to call other arguments besides directly + // callable objects, but they should not show up as callable. + static_assert(!internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); + static_assert(!internal::is_callable_r::value, ""); +} // Tests that BuiltInDefaultValue::Get() returns NULL. TEST(BuiltInDefaultValueTest, IsNullForPointerTypes) { @@ -1428,6 +1561,154 @@ TEST(MockMethodTest, CanTakeMoveOnlyValue) { EXPECT_EQ(42, *saved); } +// It should be possible to use callables with an &&-qualified call operator +// with WillOnce, since they will be called only once. This allows actions to +// contain and manipulate move-only types. +TEST(MockMethodTest, ActionHasRvalueRefQualifiedCallOperator) { + struct Return17 { + int operator()() && { return 17; } + }; + + // Action is directly compatible with mocked function type. + { + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(Return17()); + + EXPECT_EQ(17, mock.AsStdFunction()()); + } + + // Action doesn't want mocked function arguments. + { + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(Return17()); + + EXPECT_EQ(17, mock.AsStdFunction()(0)); + } +} + +// Edge case: if an action has both a const-qualified and an &&-qualified call +// operator, there should be no "ambiguous call" errors. The &&-qualified +// operator should be used by WillOnce (since it doesn't need to retain the +// action beyond one call), and the const-qualified one by WillRepeatedly. +TEST(MockMethodTest, ActionHasMultipleCallOperators) { + struct ReturnInt { + int operator()() && { return 17; } + int operator()() const& { return 19; } + }; + + // Directly compatible with mocked function type. + { + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(ReturnInt()).WillRepeatedly(ReturnInt()); + + EXPECT_EQ(17, mock.AsStdFunction()()); + EXPECT_EQ(19, mock.AsStdFunction()()); + EXPECT_EQ(19, mock.AsStdFunction()()); + } + + // Ignores function arguments. + { + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(ReturnInt()).WillRepeatedly(ReturnInt()); + + EXPECT_EQ(17, mock.AsStdFunction()(0)); + EXPECT_EQ(19, mock.AsStdFunction()(0)); + EXPECT_EQ(19, mock.AsStdFunction()(0)); + } +} + +// WillOnce should have no problem coping with a move-only action, whether it is +// &&-qualified or not. +TEST(MockMethodTest, MoveOnlyAction) { + // &&-qualified + { + struct Return17 { + Return17() = default; + Return17(Return17&&) = default; + + Return17(const Return17&) = delete; + Return17 operator=(const Return17&) = delete; + + int operator()() && { return 17; } + }; + + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(Return17()); + EXPECT_EQ(17, mock.AsStdFunction()()); + } + + // Not &&-qualified + { + struct Return17 { + Return17() = default; + Return17(Return17&&) = default; + + Return17(const Return17&) = delete; + Return17 operator=(const Return17&) = delete; + + int operator()() const { return 17; } + }; + + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(Return17()); + EXPECT_EQ(17, mock.AsStdFunction()()); + } +} + +// It should be possible to use an action that returns a value with a mock +// function that doesn't, both through WillOnce and WillRepeatedly. +TEST(MockMethodTest, ActionReturnsIgnoredValue) { + struct ReturnInt { + int operator()() const { return 0; } + }; + + MockFunction mock; + EXPECT_CALL(mock, Call).WillOnce(ReturnInt()).WillRepeatedly(ReturnInt()); + + mock.AsStdFunction()(); + mock.AsStdFunction()(); +} + +// Despite the fanciness around move-only actions and so on, it should still be +// possible to hand an lvalue reference to a copyable action to WillOnce. +TEST(MockMethodTest, WillOnceCanAcceptLvalueReference) { + MockFunction mock; + + const auto action = [] { return 17; }; + EXPECT_CALL(mock, Call).WillOnce(action); + + EXPECT_EQ(17, mock.AsStdFunction()()); +} + +// A callable that doesn't use SFINAE to restrict its call operator's overload +// set, but is still picky about which arguments it will accept. +struct StaticAssertSingleArgument { + template + static constexpr bool CheckArgs() { + static_assert(sizeof...(Args) == 1, ""); + return true; + } + + template ()> + int operator()(Args...) const { + return 17; + } +}; + +// WillOnce and WillRepeatedly should both work fine with naïve implementations +// of actions that don't use SFINAE to limit the overload set for their call +// operator. If they are compatible with the actual mocked signature, we +// shouldn't probe them with no arguments and trip a static_assert. +TEST(MockMethodTest, ActionSwallowsAllArguments) { + MockFunction mock; + EXPECT_CALL(mock, Call) + .WillOnce(StaticAssertSingleArgument{}) + .WillRepeatedly(StaticAssertSingleArgument{}); + + EXPECT_EQ(17, mock.AsStdFunction()(0)); + EXPECT_EQ(17, mock.AsStdFunction()(0)); +} + // Tests for std::function based action. int Add(int val, int& ref, int* ptr) { // NOLINT @@ -1552,7 +1833,8 @@ TEST(ActionMacro, LargeArity) { 14, 15, 16, 17, 18, 19))); } -} // Unnamed namespace +} // namespace +} // namespace testing #ifdef _MSC_VER #if _MSC_VER == 1900