From cd430b47a54841ec45d64d2377d7cabaf0eba610 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 23 Apr 2025 13:29:20 -0700 Subject: [PATCH] AllOf, AnyOf, Optional: Avoid generating unnecessary match explanations Previously, those matchers always invoked the child matchers with an IsInterested MatchResultListener, resulting in unnecessary work formatting match results that would be discarded. PiperOrigin-RevId: 750704295 Change-Id: I1639a3a15d269459f26b3aebc3a6cbdced6896a3 --- googlemock/include/gmock/gmock-matchers.h | 22 ++++++ .../test/gmock-matchers-comparisons_test.cc | 69 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 402dd6993..aa3a5eb16 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -1312,6 +1312,15 @@ class AllOfMatcherImpl : public MatcherInterface { bool MatchAndExplain(const T& x, MatchResultListener* listener) const override { + if (!listener->IsInterested()) { + // Fast path to avoid unnecessary formatting. + for (const Matcher& matcher : matchers_) { + if (!matcher.Matches(x)) { + return false; + } + } + return true; + } // This method uses matcher's explanation when explaining the result. // However, if matcher doesn't provide one, this method uses matcher's // description. @@ -1431,6 +1440,15 @@ class AnyOfMatcherImpl : public MatcherInterface { bool MatchAndExplain(const T& x, MatchResultListener* listener) const override { + if (!listener->IsInterested()) { + // Fast path to avoid unnecessary formatting of match explanations. + for (const Matcher& matcher : matchers_) { + if (matcher.Matches(x)) { + return true; + } + } + return false; + } // This method uses matcher's explanation when explaining the result. // However, if matcher doesn't provide one, this method uses matcher's // description. @@ -4118,6 +4136,10 @@ class OptionalMatcher { return false; } const ValueType& value = *optional; + if (!listener->IsInterested()) { + // Fast path to avoid unnecessary generation of match explanation. + return value_matcher_.Matches(value); + } StringMatchResultListener value_listener; const bool match = value_matcher_.MatchAndExplain(value, &value_listener); *listener << "whose value " << PrintToString(value) diff --git a/googlemock/test/gmock-matchers-comparisons_test.cc b/googlemock/test/gmock-matchers-comparisons_test.cc index a331aeca9..87eca08f5 100644 --- a/googlemock/test/gmock-matchers-comparisons_test.cc +++ b/googlemock/test/gmock-matchers-comparisons_test.cc @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -2396,6 +2397,74 @@ TEST(ExplainMatchResultTest, AllOf_True_True_2) { EXPECT_EQ("is >= 2, and is <= 3", Explain(m, 2)); } +// A matcher that records whether the listener was interested. +template +class CountingMatcher : public MatcherInterface { + public: + explicit CountingMatcher(const Matcher& base_matcher, + std::vector* listener_interested) + : base_matcher_(base_matcher), + listener_interested_(listener_interested) {} + + bool MatchAndExplain(T x, MatchResultListener* listener) const override { + listener_interested_->push_back(listener->IsInterested()); + return base_matcher_.MatchAndExplain(x, listener); + } + + void DescribeTo(ostream* os) const override { base_matcher_.DescribeTo(os); } + + private: + Matcher base_matcher_; + std::vector* listener_interested_; +}; + +TEST(AllOfTest, DoesNotFormatChildMatchersWhenNotInterested) { + std::vector listener_interested; + Matcher matcher = + MakeMatcher(new CountingMatcher(Eq(1), &listener_interested)); + EXPECT_TRUE(matcher.Matches(1)); + EXPECT_THAT(listener_interested, ElementsAre(false)); + listener_interested.clear(); + Matcher all_of_matcher = AllOf(matcher, matcher); + EXPECT_TRUE(all_of_matcher.Matches(1)); + EXPECT_THAT(listener_interested, ElementsAre(false, false)); + listener_interested.clear(); + EXPECT_FALSE(all_of_matcher.Matches(0)); + EXPECT_THAT(listener_interested, ElementsAre(false)); +} + +TEST(AnyOfTest, DoesNotFormatChildMatchersWhenNotInterested) { + std::vector listener_interested; + Matcher matcher = + MakeMatcher(new CountingMatcher(Eq(1), &listener_interested)); + EXPECT_TRUE(matcher.Matches(1)); + EXPECT_THAT(listener_interested, ElementsAre(false)); + listener_interested.clear(); + Matcher any_of_matcher = AnyOf(matcher, matcher); + EXPECT_TRUE(any_of_matcher.Matches(1)); + EXPECT_THAT(listener_interested, ElementsAre(false)); + listener_interested.clear(); + EXPECT_FALSE(any_of_matcher.Matches(0)); + EXPECT_THAT(listener_interested, ElementsAre(false, false)); +} + +TEST(OptionalTest, DoesNotFormatChildMatcherWhenNotInterested) { + std::vector listener_interested; + Matcher matcher = + MakeMatcher(new CountingMatcher(Eq(1), &listener_interested)); + EXPECT_TRUE(matcher.Matches(1)); + EXPECT_THAT(listener_interested, ElementsAre(false)); + listener_interested.clear(); + Matcher> optional_matcher = Optional(matcher); + EXPECT_FALSE(optional_matcher.Matches(std::nullopt)); + EXPECT_THAT(listener_interested, ElementsAre()); + EXPECT_TRUE(optional_matcher.Matches(1)); + EXPECT_THAT(listener_interested, ElementsAre(false)); + listener_interested.clear(); + EXPECT_FALSE(matcher.Matches(0)); + EXPECT_THAT(listener_interested, ElementsAre(false)); +} + INSTANTIATE_GTEST_MATCHER_TEST_P(ExplainmatcherResultTest); TEST_P(ExplainmatcherResultTestP, MonomorphicMatcher) {