From 28356773cbc429eb9d62ab0bf311c9979fa301fd Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 27 May 2022 02:23:19 -0700 Subject: gmock-actions: properly support non-moveable results in `is_callable_r`. Previously this excluded callables that return non-moveable types. This is the same as the [libc++ std::is_invocable_r bug](https://github.com/llvm/llvm-project/issues/55346) fixed by [this commit](https://github.com/llvm/llvm-project/commit/c3a24882903d): it's wrong to use std::is_convertible for checking the return type, since (despite its name) that doesn't check the standard-defined notion of "implicitly convertible". Instead we must base the check on whether the source type can be used as an argument to a function that accepts the destination type. PiperOrigin-RevId: 451341205 Change-Id: I2530051312a0361ea7a2ce26993ae973c9242089 --- googlemock/include/gmock/gmock-actions.h | 53 ++++++++++++++++++++++++++++++-- googlemock/test/gmock-actions_test.cc | 30 +++++++++++++++++- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/googlemock/include/gmock/gmock-actions.h b/googlemock/include/gmock/gmock-actions.h index e5981f96..c785ad8a 100644 --- a/googlemock/include/gmock/gmock-actions.h +++ b/googlemock/include/gmock/gmock-actions.h @@ -298,6 +298,53 @@ struct disjunction template using void_t = void; +// Detects whether an expression of type `From` can be implicitly converted to +// `To` according to [conv]. In C++17, [conv]/3 defines this as follows: +// +// An expression e can be implicitly converted to a type T if and only if +// the declaration T t=e; is well-formed, for some invented temporary +// variable t ([dcl.init]). +// +// [conv]/2 implies we can use function argument passing to detect whether this +// initialization is valid. +// +// Note that this is distinct from is_convertible, which requires this be valid: +// +// To test() { +// return declval(); +// } +// +// In particular, is_convertible doesn't give the correct answer when `To` and +// `From` are the same non-moveable type since `declval` will be an rvalue +// reference, defeating the guaranteed copy elision that would otherwise make +// this function work. +// +// REQUIRES: `From` is not cv void. +template +struct is_implicitly_convertible { + private: + // A function that accepts a parameter of type T. This can be called with type + // U successfully only if U is implicitly convertible to T. + template + static void Accept(T); + + // A function that creates a value of type T. + template + static T Make(); + + // An overload be selected when implicit conversion from T to To is possible. + template (Make()))> + static std::true_type TestImplicitConversion(int); + + // A fallback overload selected in all other cases. + template + static std::false_type TestImplicitConversion(...); + + public: + using type = decltype(TestImplicitConversion(0)); + static constexpr bool value = type::value; +}; + // 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). @@ -313,9 +360,9 @@ struct is_callable_r_impl : 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 {}; + std::is_void::value, // + std::true_type, // + is_implicitly_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. diff --git a/googlemock/test/gmock-actions_test.cc b/googlemock/test/gmock-actions_test.cc index 9aa9f810..215495ed 100644 --- a/googlemock/test/gmock-actions_test.cc +++ b/googlemock/test/gmock-actions_test.cc @@ -192,13 +192,15 @@ TEST(TypeTraits, IsInvocableRV) { }; // 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. + // It can be used to obtain an int, cv void, or anything int is convertible + // to. 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 possible to provide an int. If it's given to an lvalue, the result is @@ -217,6 +219,32 @@ TEST(TypeTraits, IsInvocableRV) { static_assert(!internal::is_callable_r::value, ""); static_assert(!internal::is_callable_r::value, ""); + // In C++17 and above, where it's guaranteed that functions can return + // non-moveable objects, everything should work fine for non-moveable rsult + // types too. +#if defined(__cplusplus) && __cplusplus >= 201703L + { + struct NonMoveable { + NonMoveable() = default; + NonMoveable(NonMoveable&&) = delete; + }; + + static_assert(!std::is_move_constructible_v); + + struct Callable { + NonMoveable operator()() { return NonMoveable(); } + }; + + 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); + } +#endif // C++17 and above + // 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, ""); -- cgit v1.2.1