[clang][sema] Fix collectConjunctionTerms()

[tbaeder]

Summary

Consider:

A == 5 && A != 5

IfA is 5, the old collectConjunctionTerms() would call itself again for
the LHS (which it ignores), then the RHS (which it also ignores) and
then just return without ever adding anything to the Terms array.

For example, there's a test case in clang/test/SemaCXX/recovery-expr-type.cpp:

namespace test13 {
enum Circular {             // expected-note {{not complete until the closing '}'}}
  Circular_A = Circular(1), // expected-error {{'Circular' is an incomplete type}}
};
// Enumerators can be evaluated (they evaluate as zero, but we don't care).
static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
}

which currently prints:

test2.cpp:6:1: error: static assertion failed due to requirement 'Circular_A == 0 && Circular_A != 0':
static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

(other diagnostics omitted)

but after this change prints:

test2.cpp:6:1: error: static assertion failed due to requirement 'Circular_A != 0':
static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
^                                ~~~~~~~~~~~~~~~
2 errors generated.

The patch depends on https://reviews.llvm.org/D130894 because of the note it adds, but that's not necessary. It's just easier because they are both in my local tree.

I wanted to add Douglas Gregor as reviewer as well but seems like he isn't around anymore.

[aaron.ballman]

It looks like precommit CI caught a relevant issue

[tbaeder]

That's just because of the note that https://reviews.llvm.org/D130894 adds, which the patch expects.

[aaron.ballman]

Oh shoot, I saw that precommit CI made it past patch application failing (last time) and I thought the precommit CI bug was fixed and so it was testing the whole stack for once. Drat.

LGTM once we finish up with what this builds on.

[tbaeder]

FWIW I switched it around so _this_ patch is now good as-is while https://reviews.llvm.org/D130894 now depends on this one.

clang/docs/ReleaseNotes.rst

 - Fix `#56772 <https://github.com/llvm/llvm-project/issues/56772>`_ - invalid
   destructor names were incorrectly accepted on template classes.
 - Improve compile-times with large dynamic array allocations with trivial
   constructors. This fixes
   `Issue 56774 <https://github.com/llvm/llvm-project/issues/56774>`_.
 - No longer assert/miscompile when trying to make a vectorized ``_BitInt`` type
   using the ``ext_vector_type`` attribute (the ``vector_size`` attribute was
   already properly diagnosing this case).
+- Fix clang not properly diagnosing the failing subexpression when chained
+  binary operators are used in a ``static_assert`` expression.
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Clang will now correctly diagnose as ill-formed a constant expression where an
   enum without a fixed underlying type is set to a value outside the range of
   the enumeration's values. Fixes
   `Issue 50055: <https://github.com/llvm/llvm-project/issues/50055>`_.
 - Clang will now check compile-time determinable string literals as format strings.

clang/lib/Sema/SemaTemplate.cpp

 /// disjunctive normal form, so we can find the first failed term
 /// within each possible clause.
 static void collectConjunctionTerms(Expr *Clause,
                                     SmallVectorImpl<Expr *> &Terms) {
   if (auto BinOp = dyn_cast<BinaryOperator>(Clause->IgnoreParenImpCasts())) {
     if (BinOp->getOpcode() == BO_LAnd) {
       collectConjunctionTerms(BinOp->getLHS(), Terms);
       collectConjunctionTerms(BinOp->getRHS(), Terms);
-    }
-
       return;
     }
+  }
 
   Terms.push_back(Clause);
 }
 
 // The ranges-v3 library uses an odd pattern of a top-level "||" with
 // a left-hand side that is value-dependent but never true. Identify
 // the idiom and ignore that term.
 static Expr *lookThroughRangesV3Condition(Preprocessor &PP, Expr *Cond) {