Remove short-circuiting from AllOf, for better failure messages

For `EXPECT_THAT` matcher usage, showing only the first failure meant
that users would sometimes have to make a fix and run the test again
only to notice that there's another failure. It's better to show more
failures so that the user can fix several issues in one go.

In practice, very little code actually wants the short circuiting here,
only a handful of cases with custom matchers used like
`AllOf(BoundsCheck(), UncheckedAccess())`. These cases are fixable by
refactoring `UncheckedAccess()` to instead also apply a bounds check to
fail the matcher rather than crash. Notably, this change doesn't affect
`AnyOf`, so another workaround is to change `AllOf(m1, m2, ...)` into
`Not(AnyOf(Not(m1), Not(m2), ...))`.

PiperOrigin-RevId: 826316273
Change-Id: Ie8186f75c10443d8da35b5d07b6a8cd9ae85b451

Author: Quincunx271

googlemock/include/gmock/gmock-matchers.h

@@ -1329,17 +1329,23 @@ class AllOfMatcherImpl : public MatcherInterface<const T&> {
     // However, if matcher doesn't provide one, this method uses matcher's
     // description.
     std::string all_match_result;
+    bool success = true;
     for (const Matcher<T>& matcher : matchers_) {
       StringMatchResultListener slistener;
       // Return explanation for first failed matcher.
       if (!matcher.MatchAndExplain(x, &slistener)) {
         const std::string explanation = slistener.str();
+        if (!success) {
+          // Already had a failure.
+          *listener << ", and ";
+        }
         if (!explanation.empty()) {
           *listener << explanation;
         } else {
           *listener << "which doesn't match (" << Describe(matcher) << ")";
         }
-        return false;
+        success = false;
+        continue;
       }
       // Keep track of explanations in case all matchers succeed.
       std::string explanation = slistener.str();
@@ -1356,8 +1362,10 @@ class AllOfMatcherImpl : public MatcherInterface<const T&> {
       }
     }
 
-    *listener << all_match_result;
-    return true;
+    if (success) {
+      *listener << all_match_result;
+    }
+    return success;
   }
 
  private:

googlemock/test/gmock-matchers-arithmetic_test.cc

@@ -770,7 +770,8 @@ TEST_P(AllOfTestP, ExplainsResult) {
   // Failed match.  The first matcher, which failed, needs to
   // explain.
   m = AllOf(GreaterThan(10), GreaterThan(20));
-  EXPECT_EQ("which is 5 less than 10", Explain(m, 5));
+  EXPECT_EQ("which is 5 less than 10, and which is 15 less than 20",
+            Explain(m, 5));
 
   // Failed match.  The second matcher, which failed, needs to
   // explain.  Since it doesn't given an explanation, the matcher text is

googlemock/test/gmock-matchers-comparisons_test.cc

@@ -2389,22 +2389,19 @@ PolymorphicMatcher<DivisibleByImpl> DivisibleBy(int n) {
   return MakePolymorphicMatcher(DivisibleByImpl(n));
 }
 
-// Tests that when AllOf() fails, only the first failing matcher is
-// asked to explain why.
+// Tests that when AllOf() fails, all failing matchers are asked to explain why.
 TEST(ExplainMatchResultTest, AllOf_False_False) {
   const Matcher<int> m = AllOf(DivisibleBy(4), DivisibleBy(3));
-  EXPECT_EQ("which is 1 modulo 4", Explain(m, 5));
+  EXPECT_EQ("which is 1 modulo 4, and which is 2 modulo 3", Explain(m, 5));
 }
 
-// Tests that when AllOf() fails, only the first failing matcher is
-// asked to explain why.
+// Tests that when AllOf() fails, all failing matchers are asked to explain why.
 TEST(ExplainMatchResultTest, AllOf_False_True) {
   const Matcher<int> m = AllOf(DivisibleBy(4), DivisibleBy(3));
   EXPECT_EQ("which is 2 modulo 4", Explain(m, 6));
 }
 
-// Tests that when AllOf() fails, only the first failing matcher is
-// asked to explain why.
+// Tests that when AllOf() fails, all failing matchers are asked to explain why.
 TEST(ExplainMatchResultTest, AllOf_True_False) {
   const Matcher<int> m = AllOf(Ge(1), DivisibleBy(3));
   EXPECT_EQ("which is 2 modulo 3", Explain(m, 5));