From e4ece4881d1fefc1e67d21c7493835815cd13085 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 15 Jan 2025 09:16:21 -0800 Subject: [PATCH] Enable safe matcher casts from `Matcher` to `Matcher`. PiperOrigin-RevId: 715826130 Change-Id: Id962fd456f6da21ea2a909f331f92d814f1dad46 --- docs/gmock_cook_book.md | 4 +- googlemock/include/gmock/gmock-matchers.h | 30 ++++++---- .../test/gmock-matchers-comparisons_test.cc | 60 ++++++++++++++++++- googlemock/test/gmock-matchers-misc_test.cc | 19 ++++++ 4 files changed, 96 insertions(+), 17 deletions(-) diff --git a/docs/gmock_cook_book.md b/docs/gmock_cook_book.md index 948abe9b..633dbc37 100644 --- a/docs/gmock_cook_book.md +++ b/docs/gmock_cook_book.md @@ -936,8 +936,8 @@ casts a matcher `m` to type `Matcher`. To ensure safety, gMock checks that floating-point numbers), the conversion from `T` to `U` is not lossy (in other words, any value representable by `T` can also be represented by `U`); and -3. When `U` is a reference, `T` must also be a reference (as the underlying - matcher may be interested in the address of the `U` value). +3. When `U` is a non-const reference, `T` must also be a reference (as the + underlying matcher may be interested in the address of the `U` value). The code won't compile if any of these conditions isn't met. diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 8f82b273..18cbe160 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -408,13 +408,22 @@ class MatcherCastImpl> { } private: - class Impl : public MatcherInterface { + // If it's possible to implicitly convert a `const T&` to U, then `Impl` can + // take that as input to avoid a copy. Otherwise, such as when `T` is a + // non-const reference type or a type explicitly constructible only from a + // non-const reference, then `Impl` must use `T` as-is (potentially copying). + using ImplArgT = + typename std::conditional::value, + const T&, T>::type; + + class Impl : public MatcherInterface { public: explicit Impl(const Matcher& source_matcher) : source_matcher_(source_matcher) {} // We delegate the matching logic to the source matcher. - bool MatchAndExplain(T x, MatchResultListener* listener) const override { + bool MatchAndExplain(ImplArgT x, + MatchResultListener* listener) const override { using FromType = typename std::remove_cv::type>::type>::type; using ToType = typename std::remove_cv> { // Do the cast to `U` explicitly if necessary. // Otherwise, let implicit conversions do the trick. - using CastType = - typename std::conditional::value, - T&, U>::type; + using CastType = typename std::conditional< + std::is_convertible::value, ImplArgT&, U>::type; return source_matcher_.MatchAndExplain(static_cast(x), listener); @@ -528,18 +536,16 @@ inline Matcher SafeMatcherCast(const M& polymorphic_matcher_or_value) { // safely convert a Matcher to a Matcher (i.e. Matcher is // contravariant): just keep a copy of the original Matcher, convert the // argument from type T to U, and then pass it to the underlying Matcher. -// The only exception is when U is a reference and T is not, as the +// The only exception is when U is a non-const reference and T is not, as the // underlying Matcher may be interested in the argument's address, which -// is not preserved in the conversion from T to U. +// cannot be preserved in the conversion from T to U (since a copy of the input +// T argument would be required to provide a non-const reference U). template inline Matcher SafeMatcherCast(const Matcher& matcher) { // Enforce that T can be implicitly converted to U. static_assert(std::is_convertible::value, - "T must be implicitly convertible to U"); - // Enforce that we are not converting a non-reference type T to a reference - // type U. - static_assert(std::is_reference::value || !std::is_reference::value, - "cannot convert non reference arg to reference"); + "T must be implicitly convertible to U (and T must be a " + "non-const reference if U is a non-const reference)"); // In case both T and U are arithmetic types, enforce that the // conversion is not lossy. typedef GTEST_REMOVE_REFERENCE_AND_CONST_(T) RawT; diff --git a/googlemock/test/gmock-matchers-comparisons_test.cc b/googlemock/test/gmock-matchers-comparisons_test.cc index a324c4c7..a331aeca 100644 --- a/googlemock/test/gmock-matchers-comparisons_test.cc +++ b/googlemock/test/gmock-matchers-comparisons_test.cc @@ -411,9 +411,27 @@ class IntValue { int value_; }; +// For testing casting matchers between compatible types. This is similar to +// IntValue, but takes a non-const reference to the value, showing MatcherCast +// works with such types (and doesn't, for example, use a const ref internally). +class MutableIntView { + public: + // An int& can be statically (although not implicitly) cast to a + // MutableIntView. + explicit MutableIntView(int& a_value) : value_(a_value) {} + + int& value() const { return value_; } + + private: + int& value_; +}; + // For testing casting matchers between compatible types. bool IsPositiveIntValue(const IntValue& foo) { return foo.value() > 0; } +// For testing casting matchers between compatible types. +bool IsPositiveMutableIntView(MutableIntView foo) { return foo.value() > 0; } + // Tests that MatcherCast(m) works when m is a Matcher where T // can be statically converted to U. TEST(MatcherCastTest, FromCompatibleType) { @@ -429,14 +447,34 @@ TEST(MatcherCastTest, FromCompatibleType) { // predicate. EXPECT_TRUE(m4.Matches(1)); EXPECT_FALSE(m4.Matches(0)); + + Matcher m5 = Truly(IsPositiveMutableIntView); + Matcher m6 = MatcherCast(m5); + // In the following, the arguments 1 and 0 are statically converted to + // MutableIntView objects, and then tested by the IsPositiveMutableIntView() + // predicate. + EXPECT_TRUE(m6.Matches(1)); + EXPECT_FALSE(m6.Matches(0)); } // Tests that MatcherCast(m) works when m is a Matcher. TEST(MatcherCastTest, FromConstReferenceToNonReference) { - Matcher m1 = Eq(0); + int n = 0; + Matcher m1 = Ref(n); Matcher m2 = MatcherCast(m1); - EXPECT_TRUE(m2.Matches(0)); - EXPECT_FALSE(m2.Matches(1)); + int n1 = 0; + EXPECT_TRUE(m2.Matches(n)); + EXPECT_FALSE(m2.Matches(n1)); +} + +// Tests that MatcherCast(m) works when m is a Matcher. +TEST(MatcherCastTest, FromConstReferenceToReference) { + int n = 0; + Matcher m1 = Ref(n); + Matcher m2 = MatcherCast(m1); + int n1 = 0; + EXPECT_TRUE(m2.Matches(n)); + EXPECT_FALSE(m2.Matches(n1)); } // Tests that MatcherCast(m) works when m is a Matcher. @@ -445,6 +483,12 @@ TEST(MatcherCastTest, FromReferenceToNonReference) { Matcher m2 = MatcherCast(m1); EXPECT_TRUE(m2.Matches(0)); EXPECT_FALSE(m2.Matches(1)); + + // Of course, reference identity isn't preserved since a copy is required. + int n = 0; + Matcher m3 = Ref(n); + Matcher m4 = MatcherCast(m3); + EXPECT_FALSE(m4.Matches(n)); } // Tests that MatcherCast(m) works when m is a Matcher. @@ -649,6 +693,16 @@ TEST(SafeMatcherCastTest, FromBaseClass) { EXPECT_FALSE(m4.Matches(d2)); } +// Tests that SafeMatcherCast(m) works when m is a Matcher. +TEST(SafeMatcherCastTest, FromConstReferenceToNonReference) { + int n = 0; + Matcher m1 = Ref(n); + Matcher m2 = SafeMatcherCast(m1); + int n1 = 0; + EXPECT_TRUE(m2.Matches(n)); + EXPECT_FALSE(m2.Matches(n1)); +} + // Tests that SafeMatcherCast(m) works when m is a Matcher. TEST(SafeMatcherCastTest, FromConstReferenceToReference) { int n = 0; diff --git a/googlemock/test/gmock-matchers-misc_test.cc b/googlemock/test/gmock-matchers-misc_test.cc index ac976dd3..de8b76c6 100644 --- a/googlemock/test/gmock-matchers-misc_test.cc +++ b/googlemock/test/gmock-matchers-misc_test.cc @@ -32,6 +32,7 @@ // This file tests some commonly used argument matchers. #include +#include #include #include #include @@ -747,6 +748,24 @@ TYPED_TEST(OptionalTest, DoesNotMatchNullopt) { EXPECT_FALSE(m.Matches(empty)); } +TYPED_TEST(OptionalTest, ComposesWithMonomorphicMatchersTakingReferences) { + const Matcher eq1 = Eq(1); + const Matcher eq2 = Eq(2); + TypeParam opt(1); + EXPECT_THAT(opt, Optional(eq1)); + EXPECT_THAT(opt, Optional(Not(eq2))); + EXPECT_THAT(opt, Optional(AllOf(eq1, Not(eq2)))); +} + +TYPED_TEST(OptionalTest, ComposesWithMonomorphicMatchersRequiringConversion) { + const Matcher eq1 = Eq(1); + const Matcher eq2 = Eq(2); + TypeParam opt(1); + EXPECT_THAT(opt, Optional(eq1)); + EXPECT_THAT(opt, Optional(Not(eq2))); + EXPECT_THAT(opt, Optional(AllOf(eq1, Not(eq2)))); +} + template class MoveOnlyOptionalTest : public testing::Test {};