Googletest export

Give each of Naggy/Nice/StrictMock a base class whose constructor runs before
the mocked class's constructor, and a destructor that runs after the mocked
class's destructor, so that any mock methods run in either the constructor or
destructor use the same strictness as other calls.

PiperOrigin-RevId: 348511612

4 files changed, 121 insertions, 54 deletions

diff --git a/googlemock/docs/cook_book.md b/googlemock/docs/cook_book.md
index cfe5858..cd64150 100644
--- a/googlemock/docs/cook_book.md
+++ b/googlemock/docs/cook_book.md
@@ -429,16 +429,6 @@ limitations):
 2.  `NiceMock<MockFoo>` and `StrictMock<MockFoo>` may not work correctly if the
     destructor of `MockFoo` is not virtual. We would like to fix this, but it
     requires cleaning up existing tests.
-3.  During the constructor or destructor of `MockFoo`, the mock object is *not*
-    nice or strict. This may cause surprises if the constructor or destructor
-    calls a mock method on `this` object. (This behavior, however, is consistent
-    with C++'s general rule: if a constructor or destructor calls a virtual
-    method of `this` object, that method is treated as non-virtual. In other
-    words, to the base class's constructor or destructor, `this` object behaves
-    like an instance of the base class, not the derived class. This rule is
-    required for safety. Otherwise a base constructor may use members of a
-    derived class before they are initialized, or a base destructor may use
-    members of a derived class after they have been destroyed.)
 
 Finally, you should be **very cautious** about when to use naggy or strict
 mocks, as they tend to make tests more brittle and harder to maintain. When you
diff --git a/googlemock/include/gmock/gmock-nice-strict.h b/googlemock/include/gmock/gmock-nice-strict.h
index 69afa1b..8230058 100644
--- a/googlemock/include/gmock/gmock-nice-strict.h
+++ b/googlemock/include/gmock/gmock-nice-strict.h
@@ -90,10 +90,51 @@ constexpr bool HasStrictnessModifier() {
   return decltype(StrictnessModifierProbe(std::declval<const T&>()))::value;
 }
 
+// Base classes that register and deregister with testing::Mock to alter the
+// default behavior around uninteresting calls. Inheriting from one of these
+// classes first and then MockClass ensures the MockClass constructor is run
+// after registration, and that the MockClass destructor runs before
+// deregistration. This guarantees that MockClass's constructor and destructor
+// run with the same level of strictness as its instance methods.
+
+#if GTEST_OS_WINDOWS && (defined(_MSC_VER) || defined(__clang__))
+// We need to mark these classes with this declspec to ensure that
+// the empty base class optimization is performed.
+#define GTEST_INTERNAL_EMPTY_BASE_CLASS __declspec(empty_bases)
+#else
+#define GTEST_INTERNAL_EMPTY_BASE_CLASS
+#endif
+
+template <typename Base>
+class NiceMockImpl {
+ public:
+  NiceMockImpl() { ::testing::Mock::AllowUninterestingCalls(this); }
+
+  ~NiceMockImpl() { ::testing::Mock::UnregisterCallReaction(this); }
+};
+
+template <typename Base>
+class NaggyMockImpl {
+ public:
+  NaggyMockImpl() { ::testing::Mock::WarnUninterestingCalls(this); }
+
+  ~NaggyMockImpl() { ::testing::Mock::UnregisterCallReaction(this); }
+};
+
+template <typename Base>
+class StrictMockImpl {
+ public:
+  StrictMockImpl() { ::testing::Mock::FailUninterestingCalls(this); }
+
+  ~StrictMockImpl() { ::testing::Mock::UnregisterCallReaction(this); }
+};
+
 }  // namespace internal
 
 template <class MockClass>
-class NiceMock : public MockClass {
+class GTEST_INTERNAL_EMPTY_BASE_CLASS NiceMock
+    : private internal::NiceMockImpl<MockClass>,
+      public MockClass {
  public:
   static_assert(
       !internal::HasStrictnessModifier<MockClass>(),
@@ -102,8 +143,8 @@ class NiceMock : public MockClass {
       "https://github.com/google/googletest/blob/master/googlemock/docs/"
       "cook_book.md#the-nice-the-strict-and-the-naggy-nicestrictnaggy");
   NiceMock() : MockClass() {
-    ::testing::Mock::AllowUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
   // Ideally, we would inherit base class's constructors through a using
@@ -115,21 +156,16 @@ class NiceMock : public MockClass {
   // made explicit.
   template <typename A>
   explicit NiceMock(A&& arg) : MockClass(std::forward<A>(arg)) {
-    ::testing::Mock::AllowUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
   template <typename TArg1, typename TArg2, typename... An>
   NiceMock(TArg1&& arg1, TArg2&& arg2, An&&... args)
       : MockClass(std::forward<TArg1>(arg1), std::forward<TArg2>(arg2),
                   std::forward<An>(args)...) {
-    ::testing::Mock::AllowUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
-  }
-
-  ~NiceMock() {  // NOLINT
-    ::testing::Mock::UnregisterCallReaction(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
  private:
@@ -137,7 +173,9 @@ class NiceMock : public MockClass {
 };
 
 template <class MockClass>
-class NaggyMock : public MockClass {
+class GTEST_INTERNAL_EMPTY_BASE_CLASS NaggyMock
+    : private internal::NaggyMockImpl<MockClass>,
+      public MockClass {
   static_assert(
       !internal::HasStrictnessModifier<MockClass>(),
       "Can't apply NaggyMock to a class hierarchy that already has a "
@@ -147,8 +185,8 @@ class NaggyMock : public MockClass {
 
  public:
   NaggyMock() : MockClass() {
-    ::testing::Mock::WarnUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
   // Ideally, we would inherit base class's constructors through a using
@@ -160,21 +198,16 @@ class NaggyMock : public MockClass {
   // made explicit.
   template <typename A>
   explicit NaggyMock(A&& arg) : MockClass(std::forward<A>(arg)) {
-    ::testing::Mock::WarnUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
   template <typename TArg1, typename TArg2, typename... An>
   NaggyMock(TArg1&& arg1, TArg2&& arg2, An&&... args)
       : MockClass(std::forward<TArg1>(arg1), std::forward<TArg2>(arg2),
                   std::forward<An>(args)...) {
-    ::testing::Mock::WarnUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
-  }
-
-  ~NaggyMock() {  // NOLINT
-    ::testing::Mock::UnregisterCallReaction(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
  private:
@@ -182,7 +215,9 @@ class NaggyMock : public MockClass {
 };
 
 template <class MockClass>
-class StrictMock : public MockClass {
+class GTEST_INTERNAL_EMPTY_BASE_CLASS StrictMock
+    : private internal::StrictMockImpl<MockClass>,
+      public MockClass {
  public:
   static_assert(
       !internal::HasStrictnessModifier<MockClass>(),
@@ -191,8 +226,8 @@ class StrictMock : public MockClass {
       "https://github.com/google/googletest/blob/master/googlemock/docs/"
       "cook_book.md#the-nice-the-strict-and-the-naggy-nicestrictnaggy");
   StrictMock() : MockClass() {
-    ::testing::Mock::FailUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
   // Ideally, we would inherit base class's constructors through a using
@@ -204,27 +239,24 @@ class StrictMock : public MockClass {
   // made explicit.
   template <typename A>
   explicit StrictMock(A&& arg) : MockClass(std::forward<A>(arg)) {
-    ::testing::Mock::FailUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
   template <typename TArg1, typename TArg2, typename... An>
   StrictMock(TArg1&& arg1, TArg2&& arg2, An&&... args)
       : MockClass(std::forward<TArg1>(arg1), std::forward<TArg2>(arg2),
                   std::forward<An>(args)...) {
-    ::testing::Mock::FailUninterestingCalls(
-        internal::ImplicitCast_<MockClass*>(this));
-  }
-
-  ~StrictMock() {  // NOLINT
-    ::testing::Mock::UnregisterCallReaction(
-        internal::ImplicitCast_<MockClass*>(this));
+    static_assert(sizeof(*this) == sizeof(MockClass),
+                  "The impl subclass shouldn't introduce any padding");
   }
 
  private:
   GTEST_DISALLOW_COPY_AND_ASSIGN_(StrictMock);
 };
 
+#undef GTEST_INTERNAL_EMPTY_BASE_CLASS
+
 }  // namespace testing
 
 #endif  // GMOCK_INCLUDE_GMOCK_GMOCK_NICE_STRICT_H_
diff --git a/googlemock/include/gmock/gmock-spec-builders.h b/googlemock/include/gmock/gmock-spec-builders.h
index ac21550..d42d4cb 100644
--- a/googlemock/include/gmock/gmock-spec-builders.h
+++ b/googlemock/include/gmock/gmock-spec-builders.h
@@ -108,6 +108,14 @@ template <typename F> class TypedExpectation;
 // Helper class for testing the Expectation class template.
 class ExpectationTester;
 
+// Helper classes for implementing NiceMock, StrictMock, and NaggyMock.
+template <typename MockClass>
+class NiceMockImpl;
+template <typename MockClass>
+class StrictMockImpl;
+template <typename MockClass>
+class NaggyMockImpl;
+
 // Protects the mock object registry (in class Mock), all function
 // mockers, and all expectations.
 //
@@ -413,14 +421,12 @@ class GTEST_API_ Mock {
   template <typename F>
   friend class internal::FunctionMocker;
 
-  template <typename M>
-  friend class NiceMock;
-
-  template <typename M>
-  friend class NaggyMock;
-
-  template <typename M>
-  friend class StrictMock;
+  template <typename MockClass>
+  friend class internal::NiceMockImpl;
+  template <typename MockClass>
+  friend class internal::NaggyMockImpl;
+  template <typename MockClass>
+  friend class internal::StrictMockImpl;
 
   // Tells Google Mock to allow uninteresting calls on the given mock
   // object.
diff --git a/googlemock/test/gmock-nice-strict_test.cc b/googlemock/test/gmock-nice-strict_test.cc
index 0a201ed..25558eb 100644
--- a/googlemock/test/gmock-nice-strict_test.cc
+++ b/googlemock/test/gmock-nice-strict_test.cc
@@ -67,6 +67,12 @@ class NotDefaultConstructible {
   explicit NotDefaultConstructible(int) {}
 };
 
+class CallsMockMethodInDestructor {
+ public:
+  ~CallsMockMethodInDestructor() { OnDestroy(); }
+  MOCK_METHOD(void, OnDestroy, ());
+};
+
 // Defines some mock classes needed by the tests.
 
 class Foo {
@@ -302,6 +308,13 @@ TEST(NiceMockTest, AcceptsClassNamedMock) {
   nice.DoThis();
 }
 
+TEST(NiceMockTest, IsNiceInDestructor) {
+  {
+    NiceMock<CallsMockMethodInDestructor> nice_on_destroy;
+    // Don't add an expectation for the call before the mock goes out of scope.
+  }
+}
+
 TEST(NiceMockTest, IsNaggy_IsNice_IsStrict) {
   NiceMock<MockFoo> nice_foo;
   EXPECT_FALSE(Mock::IsNaggy(&nice_foo));
@@ -405,6 +418,22 @@ TEST(NaggyMockTest, AcceptsClassNamedMock) {
   naggy.DoThis();
 }
 
+TEST(NaggyMockTest, IsNaggyInDestructor) {
+  const std::string saved_flag = GMOCK_FLAG(verbose);
+  GMOCK_FLAG(verbose) = "warning";
+  CaptureStdout();
+
+  {
+    NaggyMock<CallsMockMethodInDestructor> naggy_on_destroy;
+    // Don't add an expectation for the call before the mock goes out of scope.
+  }
+
+  EXPECT_THAT(GetCapturedStdout(),
+              HasSubstr("Uninteresting mock function call"));
+
+  GMOCK_FLAG(verbose) = saved_flag;
+}
+
 TEST(NaggyMockTest, IsNaggy_IsNice_IsStrict) {
   NaggyMock<MockFoo> naggy_foo;
   EXPECT_TRUE(Mock::IsNaggy(&naggy_foo));
@@ -489,6 +518,16 @@ TEST(StrictMockTest, AcceptsClassNamedMock) {
   strict.DoThis();
 }
 
+TEST(StrictMockTest, IsStrictInDestructor) {
+  EXPECT_NONFATAL_FAILURE(
+      {
+        StrictMock<CallsMockMethodInDestructor> strict_on_destroy;
+        // Don't add an expectation for the call before the mock goes out of
+        // scope.
+      },
+      "Uninteresting mock function call");
+}
+
 TEST(StrictMockTest, IsNaggy_IsNice_IsStrict) {
   StrictMock<MockFoo> strict_foo;
   EXPECT_FALSE(Mock::IsNaggy(&strict_foo));