Skip to content

Commit 2835677

Browse files
jacobsacopybara-github
authored andcommitted
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](llvm/llvm-project#55346) fixed by [this commit](llvm/llvm-project@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
1 parent 56246cd commit 2835677

File tree

2 files changed

+79
-4
lines changed

2 files changed

+79
-4
lines changed

googlemock/include/gmock/gmock-actions.h

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,53 @@ struct disjunction<P1, Ps...>
298298
template <typename...>
299299
using void_t = void;
300300

301+
// Detects whether an expression of type `From` can be implicitly converted to
302+
// `To` according to [conv]. In C++17, [conv]/3 defines this as follows:
303+
//
304+
// An expression e can be implicitly converted to a type T if and only if
305+
// the declaration T t=e; is well-formed, for some invented temporary
306+
// variable t ([dcl.init]).
307+
//
308+
// [conv]/2 implies we can use function argument passing to detect whether this
309+
// initialization is valid.
310+
//
311+
// Note that this is distinct from is_convertible, which requires this be valid:
312+
//
313+
// To test() {
314+
// return declval<From>();
315+
// }
316+
//
317+
// In particular, is_convertible doesn't give the correct answer when `To` and
318+
// `From` are the same non-moveable type since `declval<From>` will be an rvalue
319+
// reference, defeating the guaranteed copy elision that would otherwise make
320+
// this function work.
321+
//
322+
// REQUIRES: `From` is not cv void.
323+
template <typename From, typename To>
324+
struct is_implicitly_convertible {
325+
private:
326+
// A function that accepts a parameter of type T. This can be called with type
327+
// U successfully only if U is implicitly convertible to T.
328+
template <typename T>
329+
static void Accept(T);
330+
331+
// A function that creates a value of type T.
332+
template <typename T>
333+
static T Make();
334+
335+
// An overload be selected when implicit conversion from T to To is possible.
336+
template <typename T, typename = decltype(Accept<To>(Make<T>()))>
337+
static std::true_type TestImplicitConversion(int);
338+
339+
// A fallback overload selected in all other cases.
340+
template <typename T>
341+
static std::false_type TestImplicitConversion(...);
342+
343+
public:
344+
using type = decltype(TestImplicitConversion<From>(0));
345+
static constexpr bool value = type::value;
346+
};
347+
301348
// Like std::invoke_result_t from C++17, but works only for objects with call
302349
// operators (not e.g. member function pointers, which we don't need specific
303350
// support for in OnceAction because std::function deals with them).
@@ -313,9 +360,9 @@ struct is_callable_r_impl : std::false_type {};
313360
template <typename R, typename F, typename... Args>
314361
struct is_callable_r_impl<void_t<call_result_t<F, Args...>>, R, F, Args...>
315362
: std::conditional<
316-
std::is_same<R, void>::value, //
317-
std::true_type, //
318-
std::is_convertible<call_result_t<F, Args...>, R>>::type {};
363+
std::is_void<R>::value, //
364+
std::true_type, //
365+
is_implicitly_convertible<call_result_t<F, Args...>, R>>::type {};
319366

320367
// Like std::is_invocable_r from C++17, but works only for objects with call
321368
// operators. See the note on call_result_t.

googlemock/test/gmock-actions_test.cc

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,15 @@ TEST(TypeTraits, IsInvocableRV) {
192192
};
193193

194194
// The first overload is callable for const and non-const rvalues and lvalues.
195-
// It can be used to obtain an int, void, or anything int is convertible too.
195+
// It can be used to obtain an int, cv void, or anything int is convertible
196+
// to.
196197
static_assert(internal::is_callable_r<int, C>::value, "");
197198
static_assert(internal::is_callable_r<int, C&>::value, "");
198199
static_assert(internal::is_callable_r<int, const C>::value, "");
199200
static_assert(internal::is_callable_r<int, const C&>::value, "");
200201

201202
static_assert(internal::is_callable_r<void, C>::value, "");
203+
static_assert(internal::is_callable_r<const volatile void, C>::value, "");
202204
static_assert(internal::is_callable_r<char, C>::value, "");
203205

204206
// It's possible to provide an int. If it's given to an lvalue, the result is
@@ -217,6 +219,32 @@ TEST(TypeTraits, IsInvocableRV) {
217219
static_assert(!internal::is_callable_r<void, C, std::string>::value, "");
218220
static_assert(!internal::is_callable_r<void, C, int, int>::value, "");
219221

222+
// In C++17 and above, where it's guaranteed that functions can return
223+
// non-moveable objects, everything should work fine for non-moveable rsult
224+
// types too.
225+
#if defined(__cplusplus) && __cplusplus >= 201703L
226+
{
227+
struct NonMoveable {
228+
NonMoveable() = default;
229+
NonMoveable(NonMoveable&&) = delete;
230+
};
231+
232+
static_assert(!std::is_move_constructible_v<NonMoveable>);
233+
234+
struct Callable {
235+
NonMoveable operator()() { return NonMoveable(); }
236+
};
237+
238+
static_assert(internal::is_callable_r<NonMoveable, Callable>::value);
239+
static_assert(internal::is_callable_r<void, Callable>::value);
240+
static_assert(
241+
internal::is_callable_r<const volatile void, Callable>::value);
242+
243+
static_assert(!internal::is_callable_r<int, Callable>::value);
244+
static_assert(!internal::is_callable_r<NonMoveable, Callable, int>::value);
245+
}
246+
#endif // C++17 and above
247+
220248
// Nothing should choke when we try to call other arguments besides directly
221249
// callable objects, but they should not show up as callable.
222250
static_assert(!internal::is_callable_r<void, int>::value, "");

0 commit comments

Comments
 (0)