From 28a96d1834dd4e973c9b95c07a1598a8aa9ac234 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 5 Feb 2019 12:18:47 -0500 Subject: Googletest export Fix matcher comparisons for std::reference_wrapper. The googletest docs indicate that std::reference_wrapper should be used to for objects that should not be copied by the matcher (in fact, the ByRef() function is basically the same as a call to std::cref). However, for many types (such as std::string), the overloaded operator== will not resolve correctly. Specifically, this is problematic if operator== depends on template argument deduction, where the same type is named on LHS and RHS. Because template argument deduction happens before any implict conversions for purposes of overload resolution, attempting to compare T with std::reference_wrapper simply looks like a comparison of unlike types. For exapmle, std::reference_wrapper is implicitly convertible to 'const std::string&', which would be able to choose an overload specialization of operator==. However, the implicit conversion can only happen after template argument deduction for operator==, so a specialization that would other be an applicable overload is never considered. Note also that this change only affects matchers. There are good reasons that matchers may need to transparently hold a std::reference_wrapper. Other comparisons (like EXPECT_EQ, et. al.) don't need to capture a reference: they don't need to defer evaluation (as in googlemock), and they don't need to avoid copies (as the call chain of matchers does). PiperOrigin-RevId: 232499175 --- googlemock/test/gmock-matchers_test.cc | 14 ++++++++++++++ googletest/include/gtest/gtest-matchers.h | 13 +++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index 1f14a056..c2f1d0ad 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -452,6 +452,20 @@ TEST(StringViewMatcherTest, CanBeImplicitlyConstructedFromStringView) { } #endif // GTEST_HAS_ABSL +// Tests that a std::reference_wrapper object can be implicitly +// converted to a Matcher or Matcher via Eq(). +TEST(StringMatcherTest, + CanBeImplicitlyConstructedFromEqReferenceWrapperString) { + std::string value = "cats"; + Matcher m1 = Eq(std::ref(value)); + EXPECT_TRUE(m1.Matches("cats")); + EXPECT_FALSE(m1.Matches("dogs")); + + Matcher m2 = Eq(std::ref(value)); + EXPECT_TRUE(m2.Matches("cats")); + EXPECT_FALSE(m2.Matches("dogs")); +} + // Tests that MakeMatcher() constructs a Matcher from a // MatcherInterface* without requiring the user to explicitly // write the type. diff --git a/googletest/include/gtest/gtest-matchers.h b/googletest/include/gtest/gtest-matchers.h index 9a8841bb..846b9455 100644 --- a/googletest/include/gtest/gtest-matchers.h +++ b/googletest/include/gtest/gtest-matchers.h @@ -599,21 +599,26 @@ class ComparisonBase { } private: - template + template + static const T& Unwrap(const T& v) { return v; } + template + static const T& Unwrap(std::reference_wrapper v) { return v; } + + template class Impl : public MatcherInterface { public: explicit Impl(const Rhs& rhs) : rhs_(rhs) {} bool MatchAndExplain(Lhs lhs, MatchResultListener* /* listener */) const override { - return Op()(lhs, rhs_); + return Op()(lhs, Unwrap(rhs_)); } void DescribeTo(::std::ostream* os) const override { *os << D::Desc() << " "; - UniversalPrint(rhs_, os); + UniversalPrint(Unwrap(rhs_), os); } void DescribeNegationTo(::std::ostream* os) const override { *os << D::NegatedDesc() << " "; - UniversalPrint(rhs_, os); + UniversalPrint(Unwrap(rhs_), os); } private: -- cgit v1.2.1