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/3] [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/3] 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/3] 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() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits