[clang] [Clang] Warning as error for fold expressions over comparison operators (PR #136836)

[via cfe-commits]

https://github.com/cor3ntin created https://github.com/llvm/llvm-project/pull/136836

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

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

Partially addresses #129570

>From 8d70f4bb100ad0dec57947ce0999a7f837d2646e Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Wed, 23 Apr 2025 11:50:10 +0200
Subject: [PATCH] [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
---
 clang/docs/ReleaseNotes.rst                   |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td        |  5 ++++
 clang/lib/Sema/TreeTransform.h                |  8 +++++++
 clang/test/Parser/cxx1z-fold-expressions.cpp  |  4 +++-
 .../SemaTemplate/cxx1z-fold-expressions.cpp   | 23 +++++++++++++++++++
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5cd1fbeabcfe..3905aef111394 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c29a3422acd26..b1d261193763a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2469991bf2ce8..ba107da82d683 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/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;
diff --git a/clang/test/Parser/cxx1z-fold-expressions.cpp b/clang/test/Parser/cxx1z-fold-expressions.cpp
index ac27111697737..3ba9a0932ccf1 100644
--- a/clang/test/Parser/cxx1z-fold-expressions.cpp
+++ b/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 at -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;
diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
index 47a252eb335f6..7dc67fe8d6bf7 100644
--- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ b/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 at -1{{comparison in a fold expression would evaluate to '(X == Y) == Z'}}
+    (void)(ts < ...);
+    // expected-error at -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);
+  }
+
+};