[Clang] Warning as error for fold expressions over comparison operators

We made chained comparisons an error.
Fold exprerssions over a comparison operators produce chained
comparison, so we should be consistent there too.

We only emit the warning when instantiating the fold expression
as to not warn on types with user-defined comparisons.

Partially addresses #129570

Author: cor3ntin

clang/docs/ReleaseNotes.rst

@@ -318,6 +318,7 @@ Improvements to Clang's diagnostics
   under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
 - Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
   ``-Wno-error=parentheses``.
+- Similarly, fold expressions over a comparison operator are now an error by default.
 - Clang now better preserves the sugared types of pointers to member.
 - Clang now better preserves the presence of the template keyword with dependent
   prefixes.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7138,6 +7138,11 @@ def warn_consecutive_comparison : Warning<
   "chained comparison 'X %0 Y %1 Z' does not behave the same as a mathematical expression">,
   InGroup<Parentheses>, DefaultError;
 
+def warn_comparison_in_fold_expression : Warning<
+  "comparison in a fold expression would evaluate to '(X %0 Y) %0 Z' "
+  "which does not behave the same as a mathematical expression">,
+  InGroup<Parentheses>, DefaultError;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup<IntInBoolContext>, DefaultIgnore;

clang/lib/Sema/TreeTransform.h

@@ -16411,6 +16411,7 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) {
       return true;
   }
 
+  bool WarnedOnComparison = false;
   for (unsigned I = 0; I != *NumExpansions; ++I) {
     Sema::ArgPackSubstIndexRAII SubstIndex(
         getSema(), LeftFold ? I : *NumExpansions - I - 1);
@@ -16439,6 +16440,13 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) {
       } else {
         Result = getDerived().RebuildBinaryOperator(E->getEllipsisLoc(),
                                                     E->getOperator(), LHS, RHS);
+        if(!WarnedOnComparison && Result.isUsable()) {
+            if(auto * BO = dyn_cast<BinaryOperator>(Result.get()); BO && BO->isComparisonOp()) {
+                WarnedOnComparison = true;
+                SemaRef.Diag(BO->getBeginLoc(), diag::warn_comparison_in_fold_expression)
+                        << BO->getOpcodeStr();
+            }
+        }
       }
     } else
       Result = Out;

clang/test/Parser/cxx1z-fold-expressions.cpp

@@ -52,9 +52,11 @@ template<typename ...T> void as_operand_of_cast(int a, T ...t) {
 
 // fold-operator can be '>' or '>>'.
 template <int... N> constexpr bool greaterThan() { return (N > ...); }
+// expected-error@-1 {{comparison in a fold expression}}
+
 template <int... N> constexpr int rightShift() { return (N >> ...); }
 
-static_assert(greaterThan<2, 1>());
+static_assert(greaterThan<2, 1>()); // expected-note {{in instantiation}}
 static_assert(rightShift<10, 1>() == 5);
 
 template <auto V> constexpr auto Identity = V;

clang/test/SemaTemplate/cxx1z-fold-expressions.cpp

@@ -132,3 +132,26 @@ bool f();
 template <typename... T>
 void g(bool = (f<T>() || ...));
 }
+
+
+namespace comparison_warning {
+  struct S {
+    bool operator<(const S&) const;
+    bool operator==(const S&) const;
+  };
+
+  template <typename...T>
+  void f(T... ts) {
+    (void)(ts == ...);
+    // expected-error@-1{{comparison in a fold expression would evaluate to '(X == Y) == Z'}}
+    (void)(ts < ...);
+    // expected-error@-1{{comparison in a fold expression would evaluate to '(X < Y) < Z'}}
+  }
+
+  void test() {
+    f(0, 1, 2); // expected-note{{in instantiation}}
+    f(S{}, S{});
+    f(0);
+  }
+
+};