Skip to content

Commit 6230d31

Browse files
Abseil Teamcopybara-github
authored andcommitted
In MatcherCast, store the input value as its own type rather than as the Matcher type, to avoid dangling references
PiperOrigin-RevId: 769240838 Change-Id: I7b1ac23a0a88ba90b5d1ae6e20b97f679f381f31
1 parent 28e9d1f commit 6230d31

File tree

3 files changed

+72
-18
lines changed

3 files changed

+72
-18
lines changed

googlemock/include/gmock/gmock-matchers.h

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,16 @@ class MatcherCastImpl {
376376

377377
// M can't be implicitly converted to Matcher<T>, so M isn't a polymorphic
378378
// matcher. It's a value of a type implicitly convertible to T. Use direct
379-
// initialization to create a matcher.
379+
// initialization or `ImplicitCastEqMatcher` to create a matcher.
380380
static Matcher<T> CastImpl(const M& value,
381381
std::false_type /* convertible_to_matcher */,
382382
std::true_type /* convertible_to_T */) {
383-
return Matcher<T>(ImplicitCast_<T>(value));
383+
using NoRefT = std::remove_cv_t<std::remove_reference_t<T>>;
384+
if constexpr (std::is_same_v<M, NoRefT>) {
385+
return Matcher<T>(value);
386+
} else {
387+
return ImplicitCastEqMatcher<NoRefT, std::decay_t<const M&>>(value);
388+
}
384389
}
385390

386391
// M can't be implicitly converted to either Matcher<T> or T. Attempt to use
@@ -391,11 +396,11 @@ class MatcherCastImpl {
391396
// latter calls bool operator==(const Lhs& lhs, const Rhs& rhs) in the end
392397
// which might be undefined even when Rhs is implicitly convertible to Lhs
393398
// (e.g. std::pair<const int, int> vs. std::pair<int, int>).
394-
//
395-
// We don't define this method inline as we need the declaration of Eq().
396399
static Matcher<T> CastImpl(const M& value,
397400
std::false_type /* convertible_to_matcher */,
398-
std::false_type /* convertible_to_T */);
401+
std::false_type /* convertible_to_T */) {
402+
return Eq(value);
403+
}
399404
};
400405

401406
// This more specialized version is used when MatcherCast()'s argument
@@ -4483,13 +4488,6 @@ inline Matcher<T> An() {
44834488
return _;
44844489
}
44854490

4486-
template <typename T, typename M>
4487-
Matcher<T> internal::MatcherCastImpl<T, M>::CastImpl(
4488-
const M& value, std::false_type /* convertible_to_matcher */,
4489-
std::false_type /* convertible_to_T */) {
4490-
return Eq(value);
4491-
}
4492-
44934491
// Creates a polymorphic matcher that matches any NULL pointer.
44944492
inline PolymorphicMatcher<internal::IsNullMatcher> IsNull() {
44954493
return MakePolymorphicMatcher(internal::IsNullMatcher());

googlemock/test/gmock-matchers-comparisons_test.cc

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -622,15 +622,42 @@ struct IntReferenceWrapper {
622622
const int* value;
623623
};
624624

625+
// Compared the contained values
625626
bool operator==(const IntReferenceWrapper& a, const IntReferenceWrapper& b) {
626-
return a.value == b.value;
627+
return *a.value == *b.value;
627628
}
628629

629-
TEST(MatcherCastTest, ValueIsNotCopied) {
630-
int n = 42;
631-
Matcher<IntReferenceWrapper> m = MatcherCast<IntReferenceWrapper>(n);
632-
// Verify that the matcher holds a reference to n, not to its temporary copy.
633-
EXPECT_TRUE(m.Matches(n));
630+
TEST(MatcherCastTest, ValueIsCopied) {
631+
{
632+
// When an IntReferenceWrapper is passed.
633+
int n = 42;
634+
Matcher<IntReferenceWrapper> m =
635+
MatcherCast<IntReferenceWrapper>(IntReferenceWrapper(n));
636+
{
637+
int value = 42;
638+
EXPECT_TRUE(m.Matches(value));
639+
value = 10;
640+
EXPECT_FALSE(m.Matches(value));
641+
// This changes the stored reference.
642+
n = 10;
643+
EXPECT_TRUE(m.Matches(value));
644+
}
645+
}
646+
647+
{
648+
// When an int is passed.
649+
int n = 42;
650+
Matcher<IntReferenceWrapper> m = MatcherCast<IntReferenceWrapper>(n);
651+
{
652+
int value = 42;
653+
EXPECT_TRUE(m.Matches(value));
654+
value = 10;
655+
EXPECT_FALSE(m.Matches(value));
656+
// This does not change the stored int.
657+
n = 10;
658+
EXPECT_FALSE(m.Matches(value));
659+
}
660+
}
634661
}
635662

636663
class Base {

googletest/include/gtest/gtest-matchers.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,35 @@ class GeMatcher
773773
static const char* NegatedDesc() { return "isn't >="; }
774774
};
775775

776+
// Same as `EqMatcher<Rhs>`, except that the `rhs` is stored as `StoredRhs` and
777+
// must be implicitly convertible to `Rhs`.
778+
template <typename Rhs, typename StoredRhs>
779+
class ImplicitCastEqMatcher {
780+
public:
781+
explicit ImplicitCastEqMatcher(const StoredRhs& rhs) : stored_rhs_(rhs) {}
782+
783+
using is_gtest_matcher = void;
784+
785+
template <typename Lhs>
786+
bool MatchAndExplain(const Lhs& lhs, std::ostream*) const {
787+
return lhs == rhs();
788+
}
789+
790+
void DescribeTo(std::ostream* os) const {
791+
*os << "is equal to ";
792+
UniversalPrint(rhs(), os);
793+
}
794+
void DescribeNegationTo(std::ostream* os) const {
795+
*os << "isn't equal to ";
796+
UniversalPrint(rhs(), os);
797+
}
798+
799+
private:
800+
Rhs rhs() const { return ImplicitCast_<Rhs>(stored_rhs_); }
801+
802+
StoredRhs stored_rhs_;
803+
};
804+
776805
template <typename T, typename = typename std::enable_if<
777806
std::is_constructible<std::string, T>::value>::type>
778807
using StringLike = T;

0 commit comments

Comments
 (0)