From 830fb567285c63ab5b5873e2e8b02f2249864916 Mon Sep 17 00:00:00 2001
From: Aaron Jacobs <jacobsa@google.com>
Date: Wed, 27 Apr 2022 08:48:35 -0700
Subject: [PATCH] gmock-actions: improve comments and tests for the implicit
 cast in Return.

Commit a070cbd91c added an implicit cast to this path but didn't leave a very
clear explanation for why it was desirable, a clear example, or even test
coverage. Add a better comment and a test that fails when the implicit cast is
removed.

PiperOrigin-RevId: 444871311
Change-Id: I127982fa8d5bce9b6d1b68177c12dc0709449164
---
 googlemock/include/gmock/gmock-actions.h | 15 ++++++------
 googlemock/test/gmock-actions_test.cc    | 31 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/googlemock/include/gmock/gmock-actions.h b/googlemock/include/gmock/gmock-actions.h
index 5836f179..f9fee550 100644
--- a/googlemock/include/gmock/gmock-actions.h
+++ b/googlemock/include/gmock/gmock-actions.h
@@ -935,15 +935,16 @@ class ReturnAction {
     typedef typename Function<F>::Result Result;
     typedef typename Function<F>::ArgumentTuple ArgumentTuple;
 
-    // The implicit cast is necessary when Result has more than one
-    // single-argument constructor (e.g. Result is std::vector<int>) and R
-    // has a type conversion operator template.  In that case, value_(value)
-    // won't compile as the compiler doesn't known which constructor of
-    // Result to call.  ImplicitCast_ forces the compiler to convert R to
-    // Result without considering explicit constructors, thus resolving the
-    // ambiguity. value_ is then initialized using its copy constructor.
     explicit Impl(const std::shared_ptr<R>& value)
         : value_before_cast_(*value),
+          // Make an implicit conversion to Result before initializing the
+          // Result object we store, avoiding calling any explicit constructor
+          // of Result from R.
+          //
+          // This simulates the language rules: a function with return type
+          // Result that does `return R()` requires R to be implicitly
+          // convertible to Result, and uses that path for the conversion, even
+          // if Result has an explicit constructor from R.
           value_(ImplicitCast_<Result>(value_before_cast_)) {}
 
     Result Perform(const ArgumentTuple&) override { return value_; }
diff --git a/googlemock/test/gmock-actions_test.cc b/googlemock/test/gmock-actions_test.cc
index a33358a7..db112f12 100644
--- a/googlemock/test/gmock-actions_test.cc
+++ b/googlemock/test/gmock-actions_test.cc
@@ -667,6 +667,37 @@ TEST(ReturnTest, SupportsWrapperReturnType) {
   EXPECT_THAT(result, ::testing::ElementsAre(0, 1, 2, 3, 4));
 }
 
+TEST(ReturnTest, PrefersConversionOperator) {
+  // Define types In and Out such that:
+  //
+  //  *  In is implicitly convertible to Out.
+  //  *  Out also has an explicit constructor from In.
+  //
+  struct In;
+  struct Out {
+    int x;
+
+    explicit Out(const int x) : x(x) {}
+    explicit Out(const In&) : x(0) {}
+  };
+
+  struct In {
+    operator Out() const { return Out{19}; }  // NOLINT
+  };
+
+  // Assumption check: the C++ language rules are such that a function that
+  // returns Out which uses In a return statement will use the implicit
+  // conversion path rather than the explicit constructor.
+  EXPECT_THAT([]() -> Out { return In(); }(), Field(&Out::x, 19));
+
+  // Return should work the same way: if the mock function's return type is Out
+  // and we feed Return an In value, then the Out should be created through the
+  // implicit conversion path rather than the explicit constructor.
+  MockFunction<Out()> mock;
+  EXPECT_CALL(mock, Call).WillOnce(Return(In()));
+  EXPECT_THAT(mock.AsStdFunction()(), Field(&Out::x, 19));
+}
+
 // Tests that Return(v) is covaraint.
 
 struct Base {