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 <corentinja...@gmail.com>
Date: Wed, 23 Apr 2025 11:50:10 +0200
Subject: [PATCH 1/5] [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@-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@-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);
+ }
+
+};
>From 7e53392a9c7cee9c09b107c1447fd904e63fdaf2 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Wed, 23 Apr 2025 12:30:11 +0200
Subject: [PATCH 2/5] 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
>From 7c61d91b5d6cef29bac947abd14270e8232c8461 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Wed, 23 Apr 2025 14:18:19 +0200
Subject: [PATCH 3/5] Address aaron's feedback
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 10 ++++------
clang/test/Parser/cxx1z-fold-expressions.cpp | 2 +-
clang/test/SemaTemplate/cxx1z-fold-expressions.cpp | 4 ++--
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5f636a713e34c..361ee3e4a737d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7138,12 +7138,10 @@ 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 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/test/Parser/cxx1z-fold-expressions.cpp
b/clang/test/Parser/cxx1z-fold-expressions.cpp
index 3ba9a0932ccf1..4a329646b799f 100644
--- a/clang/test/Parser/cxx1z-fold-expressions.cpp
+++ b/clang/test/Parser/cxx1z-fold-expressions.cpp
@@ -52,7 +52,7 @@ 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}}
+// expected-error@-1 {{comparison in fold expression}}
template <int... N> constexpr int rightShift() { return (N >> ...); }
diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
index 7dc67fe8d6bf7..61cf710aeba52 100644
--- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -143,9 +143,9 @@ namespace comparison_warning {
template <typename...T>
void f(T... ts) {
(void)(ts == ...);
- // expected-error@-1{{comparison in a fold expression would evaluate to
'(X == Y) == Z'}}
+ // expected-error@-1{{comparison in fold expression would evaluate to '(X
== Y) == Z'}}
(void)(ts < ...);
- // expected-error@-1{{comparison in a fold expression would evaluate to
'(X < Y) < Z'}}
+ // expected-error@-1{{comparison in fold expression would evaluate to '(X
< Y) < Z'}}
}
void test() {
>From 5a2297bbce7be13f5eeacc15b462bb3d5f7f7d98 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Wed, 23 Apr 2025 17:02:02 +0200
Subject: [PATCH 4/5] add tests
---
clang/test/SemaTemplate/cxx1z-fold-expressions.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
index 61cf710aeba52..34987758abfd4 100644
--- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -143,13 +143,14 @@ namespace comparison_warning {
template <typename...T>
void f(T... ts) {
(void)(ts == ...);
- // expected-error@-1{{comparison in fold expression would evaluate to '(X
== Y) == Z'}}
+ // expected-error@-1 2{{comparison in fold expression would evaluate to
'(X == Y) == Z'}}
(void)(ts < ...);
- // expected-error@-1{{comparison in fold expression would evaluate to '(X
< Y) < Z'}}
+ // expected-error@-1 2{{comparison in fold expression would evaluate to
'(X < Y) < Z'}}
}
void test() {
f(0, 1, 2); // expected-note{{in instantiation}}
+ f(0, 1); // expected-note{{in instantiation}}
f(S{}, S{});
f(0);
}
>From 2b8e3b0843d73f67175e1d5a9c38f377165a4b10 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Thu, 1 May 2025 11:12:26 +0200
Subject: [PATCH 5/5] add dpplicasted wordings for right folds
---
clang/include/clang/Sema/Sema.h | 6 ++++--
clang/lib/Sema/SemaExpr.cpp | 15 ++++++++-------
clang/lib/Sema/TreeTransform.h | 12 +++++++-----
.../test/SemaTemplate/cxx1z-fold-expressions.cpp | 3 +++
4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a757f4c6430ae..9c69ac0b09fd8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7182,13 +7182,15 @@ class Sema final : public SemaBase {
ExprResult ActOnBinOp(Scope *S, SourceLocation TokLoc, tok::TokenKind Kind,
Expr *LHSExpr, Expr *RHSExpr);
ExprResult BuildBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
- Expr *LHSExpr, Expr *RHSExpr);
+ Expr *LHSExpr, Expr *RHSExpr,
+ bool ForFoldExpression = false);
/// CreateBuiltinBinOp - Creates a new built-in binary operation with
/// operator @p Opc at location @c TokLoc. This routine only supports
/// built-in operations; ActOnBinOp handles overloaded operators.
ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
- Expr *LHSExpr, Expr *RHSExpr);
+ Expr *LHSExpr, Expr *RHSExpr,
+ bool ForFoldExpression = false);
void LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
UnresolvedSetImpl &Functions);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 9cd1a7e45fb64..7b62281712cf6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14918,8 +14918,8 @@ static bool needsConversionOfHalfVec(bool
OpRequiresConversion, ASTContext &Ctx,
}
ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
- BinaryOperatorKind Opc,
- Expr *LHSExpr, Expr *RHSExpr) {
+ BinaryOperatorKind Opc, Expr *LHSExpr,
+ Expr *RHSExpr, bool ForFoldExpression) {
if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) {
// The syntax only allows initializer lists on the RHS of assignment,
// so we don't need to worry about accepting invalid code for
@@ -15050,7 +15050,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation
OpLoc,
ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr);
- BI && BI->isComparisonOp())
+ !ForFoldExpression && BI && BI->isComparisonOp())
Diag(OpLoc, diag::warn_consecutive_comparison)
<< BI->getOpcodeStr() << BinaryOperator::getOpcodeStr(Opc);
@@ -15460,8 +15460,8 @@ static ExprResult BuildOverloadedBinOp(Sema &S, Scope
*Sc, SourceLocation OpLoc,
}
ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
- BinaryOperatorKind Opc,
- Expr *LHSExpr, Expr *RHSExpr) {
+ BinaryOperatorKind Opc, Expr *LHSExpr,
+ Expr *RHSExpr, bool ForFoldExpression) {
ExprResult LHS, RHS;
std::tie(LHS, RHS) = CorrectDelayedTyposInBinOp(*this, Opc, LHSExpr,
RHSExpr);
if (!LHS.isUsable() || !RHS.isUsable())
@@ -15535,7 +15535,8 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation
OpLoc,
LHSExpr->getType()->isOverloadableType()))
return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
- return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
+ return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr,
+ ForFoldExpression);
}
// Don't resolve overloads if the other type is overloadable.
@@ -15599,7 +15600,7 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation
OpLoc,
}
// Build a built-in binary operation.
- return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
+ return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr, ForFoldExpression);
}
static bool isOverflowingIntegerType(ASTContext &Ctx, QualType T) {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7ecf30dd481ce..f83bd1707be82 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2967,10 +2967,11 @@ class TreeTransform {
///
/// By default, performs semantic analysis to build the new expression.
/// Subclasses may override this routine to provide different behavior.
- ExprResult RebuildBinaryOperator(SourceLocation OpLoc,
- BinaryOperatorKind Opc,
- Expr *LHS, Expr *RHS) {
- return getSema().BuildBinOp(/*Scope=*/nullptr, OpLoc, Opc, LHS, RHS);
+ ExprResult RebuildBinaryOperator(SourceLocation OpLoc, BinaryOperatorKind
Opc,
+ Expr *LHS, Expr *RHS,
+ bool ForFoldExpression = false) {
+ return getSema().BuildBinOp(/*Scope=*/nullptr, OpLoc, Opc, LHS, RHS,
+ ForFoldExpression);
}
/// Build a new rewritten operator expression.
@@ -16439,7 +16440,8 @@
TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) {
Functions, LHS, RHS);
} else {
Result = getDerived().RebuildBinaryOperator(E->getEllipsisLoc(),
- E->getOperator(), LHS,
RHS);
+ E->getOperator(), LHS, RHS,
+ /*ForFoldExpresion=*/true);
if (!WarnedOnComparison && Result.isUsable()) {
if (auto *BO = dyn_cast<BinaryOperator>(Result.get());
BO && BO->isComparisonOp()) {
diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
index 34987758abfd4..1ac821002f95c 100644
--- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -137,6 +137,7 @@ void g(bool = (f<T>() || ...));
namespace comparison_warning {
struct S {
bool operator<(const S&) const;
+ bool operator<(int) const;
bool operator==(const S&) const;
};
@@ -146,6 +147,8 @@ namespace comparison_warning {
// expected-error@-1 2{{comparison in fold expression would evaluate to
'(X == Y) == Z'}}
(void)(ts < ...);
// expected-error@-1 2{{comparison in fold expression would evaluate to
'(X < Y) < Z'}}
+ (void)(... < ts);
+ // expected-error@-1 2{{comparison in fold expression would evaluate to
'(X < Y) < Z'}}
}
void test() {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
