[clang] [Clang] Warning as error for fold expressions over comparison operators (PR #136836)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 23 03:30:28 PDT 2025
https://github.com/cor3ntin updated https://github.com/llvm/llvm-project/pull/136836
>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 1/2] [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);
+ }
+
+};
>From 7e53392a9c7cee9c09b107c1447fd904e63fdaf2 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Wed, 23 Apr 2025 12:30:11 +0200
Subject: [PATCH 2/2] format
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 10 ++++++----
clang/lib/Sema/TreeTransform.h | 14 ++++++++------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b1d261193763a..5f636a713e34c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7138,10 +7138,12 @@ 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_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">,
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ba107da82d683..7ecf30dd481ce 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -16440,12 +16440,14 @@ 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();
- }
+ 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
More information about the cfe-commits
mailing list