https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/110761
>From 9c69d6584d6b71554aec55f0de52abb4baa9435f Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Wed, 2 Oct 2024 02:13:51 +0300 Subject: [PATCH 1/6] [Clang] omit parentheses in fold expressions with a single expansion --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/TreeTransform.h | 3 +++ clang/test/SemaCXX/warn-assignment-condition.cpp | 12 +++++++++++- .../test/SemaTemplate/instantiate-requires-expr.cpp | 2 +- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index bf128e370b076e..254779dc734dea 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -460,6 +460,7 @@ Bug Fixes to C++ Support containing outer unexpanded parameters were not correctly expanded. (#GH101754) - Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly in certain friend declarations. (#GH93099) +- Fixed warnings for extra parentheses in fold expressions by eliminating them in single expansion cases. (#GH101863) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 6fdb18d51acef9..1a963858d0a3e4 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -15598,6 +15598,9 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { return getDerived().RebuildEmptyCXXFoldExpr(E->getEllipsisLoc(), E->getOperator()); + if (*NumExpansions == 1) + Result = Result.get()->IgnoreParens(); + return Result; } diff --git a/clang/test/SemaCXX/warn-assignment-condition.cpp b/clang/test/SemaCXX/warn-assignment-condition.cpp index 09084e36bb4916..1b644260aa61d5 100644 --- a/clang/test/SemaCXX/warn-assignment-condition.cpp +++ b/clang/test/SemaCXX/warn-assignment-condition.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -Wparentheses -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wparentheses -std=c++2a -verify %s struct A { int foo(); @@ -144,3 +144,13 @@ void test() { f(S()); // expected-note {{in instantiation}} } } + +namespace GH101863 { +void foo(auto... args) { + if (((args == 0) or ...)) {} // ok +} + +void bar() { + foo(3); +} +} diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index 20a19d731ae169..ce2c060a176045 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -36,7 +36,7 @@ using r1i2 = r1<char>; // expected-error {{constraints not satisfied for class t template<typename... Ts> requires false_v<requires (Ts... ts) {requires ((sizeof(ts) == 2) && ...);}> // expected-note@-1 {{because 'false_v<requires (short ts, unsigned short ts) { requires (sizeof (ts) == 2) && (sizeof (ts) == 2); }>'}} -// expected-note@-2 {{because 'false_v<requires (short ts) { requires (sizeof (ts) == 2); }>' evaluated to false}} +// expected-note@-2 {{because 'false_v<requires (short ts) { requires sizeof (ts) == 2; }>' evaluated to false}} struct r2 {}; using r2i1 = r2<short, unsigned short>; // expected-error {{constraints not satisfied for class template 'r2' [with Ts = <short, unsigned short>]}} >From 22f98718d499b60836cccb5cbfcfa2098cddeb10 Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Wed, 2 Oct 2024 15:59:26 +0300 Subject: [PATCH 2/6] move changelog message to diagnostics --- clang/docs/ReleaseNotes.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d71de13b093daf..e927b305ef1281 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -375,6 +375,8 @@ Improvements to Clang's diagnostics - Clang now diagnoses when a ``requires`` expression has a local parameter of void type, aligning with the function parameter (#GH109831). +- Clang now omits warnings for extra parentheses in fold expressions with single expansion. (#GH101863) + Improvements to Clang's time-trace ---------------------------------- @@ -471,7 +473,6 @@ Bug Fixes to C++ Support containing outer unexpanded parameters were not correctly expanded. (#GH101754) - Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly in certain friend declarations. (#GH93099) -- Fixed warnings for extra parentheses in fold expressions by eliminating them in single expansion cases. (#GH101863) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ >From b9305e649a5dbf2101d2528c1eaabc6fb039483f Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 3 Oct 2024 02:25:43 +0300 Subject: [PATCH 3/6] adjust pattern to omit parentheses --- clang/lib/Sema/TreeTransform.h | 12 ++++-- .../SemaCXX/warn-assignment-condition.cpp | 41 +++++++++---------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 1a963858d0a3e4..72b50245f16a7d 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -15540,6 +15540,14 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { return true; } + // When there's only one expansion, the parentheses can be safely eliminated + // to avoid any extra redundancy that may result in incorrect checks + // For example, transforming (((args == 0) || ...)) into (args == 0) + // allows the omission of parentheses while ensuring precise representation + // and avoiding warnings regarding redundant parentheses. + if (*NumExpansions == 1) + Pattern = Pattern->IgnoreParens(); + for (unsigned I = 0; I != *NumExpansions; ++I) { Sema::ArgumentPackSubstitutionIndexRAII SubstIndex( getSema(), LeftFold ? I : *NumExpansions - I - 1); @@ -15597,10 +15605,6 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { if (Result.isUnset()) return getDerived().RebuildEmptyCXXFoldExpr(E->getEllipsisLoc(), E->getOperator()); - - if (*NumExpansions == 1) - Result = Result.get()->IgnoreParens(); - return Result; } diff --git a/clang/test/SemaCXX/warn-assignment-condition.cpp b/clang/test/SemaCXX/warn-assignment-condition.cpp index 1b644260aa61d5..ea169e8f9fa49c 100644 --- a/clang/test/SemaCXX/warn-assignment-condition.cpp +++ b/clang/test/SemaCXX/warn-assignment-condition.cpp @@ -122,35 +122,34 @@ void test() { #undef EQ } -void (*fn)(); - -void test2() { - if ((fn == test2)) {} // expected-warning {{equality comparison with extraneous parentheses}} \ - // expected-note {{use '=' to turn this equality comparison into an assignment}} \ - // expected-note {{remove extraneous parentheses around the comparison to silence this warning}} - if ((test2 == fn)) {} +namespace GH101863 { +void t1(auto... args) { + if (((args == 0) or ...)) { } } -namespace rdar9027658 { -template <typename T> -void f(T t) { - if ((t.g == 3)) { } // expected-warning {{equality comparison with extraneous parentheses}} \ - // expected-note {{use '=' to turn this equality comparison into an assignment}} \ - // expected-note {{remove extraneous parentheses around the comparison to silence this warning}} +template <typename... Args> +void t2(Args... args) { + if (((args == 0) or ...)) { } } -struct S { int g; }; -void test() { - f(S()); // expected-note {{in instantiation}} +void t3(auto... args) { + if ((... && (args == 0))) { } } + +void t4(auto... a, auto... b) { + if (((a == 0) or ...) && ((b == 0) or ...)) { } } -namespace GH101863 { -void foo(auto... args) { - if (((args == 0) or ...)) {} // ok +void t5(auto... args) { + if ((((args == 0) or ...))) { } } -void bar() { - foo(3); +void test() { + t1(0, 1); + t2<>(); + t3(1, 2, 3); + t3(0, 1); + t4(0, 1); + t5(0, 1); } } >From 8baa1007a4c3e6148195409ec86707f0131bd3c0 Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 3 Oct 2024 18:18:02 +0300 Subject: [PATCH 4/6] revert missed test cases --- .../SemaCXX/warn-assignment-condition.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/clang/test/SemaCXX/warn-assignment-condition.cpp b/clang/test/SemaCXX/warn-assignment-condition.cpp index ea169e8f9fa49c..4f9d13be3c58d3 100644 --- a/clang/test/SemaCXX/warn-assignment-condition.cpp +++ b/clang/test/SemaCXX/warn-assignment-condition.cpp @@ -122,6 +122,29 @@ void test() { #undef EQ } +void (*fn)(); + +void test2() { + if ((fn == test2)) {} // expected-warning {{equality comparison with extraneous parentheses}} \ + // expected-note {{use '=' to turn this equality comparison into an assignment}} \ + // expected-note {{remove extraneous parentheses around the comparison to silence this warning}} + if ((test2 == fn)) {} +} + +namespace rdar9027658 { +template <typename T> +void f(T t) { + if ((t.g == 3)) { } // expected-warning {{equality comparison with extraneous parentheses}} \ + // expected-note {{use '=' to turn this equality comparison into an assignment}} \ + // expected-note {{remove extraneous parentheses around the comparison to silence this warning}} +} + +struct S { int g; }; +void test() { + f(S()); // expected-note {{in instantiation}} +} +} + namespace GH101863 { void t1(auto... args) { if (((args == 0) or ...)) { } >From 8f4e5b94fbe89d51cf5a19e213bd6166ee7c0268 Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 3 Oct 2024 18:31:24 +0300 Subject: [PATCH 5/6] update ReleaseNotes and comments --- clang/docs/ReleaseNotes.rst | 2 +- clang/lib/Sema/TreeTransform.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a86bb4839e49d2..76273ca6dbef96 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -378,7 +378,7 @@ Improvements to Clang's diagnostics - Clang now emits a diagnostic note at the class declaration when the method definition does not match any declaration (#GH110638). -- Clang now omits warnings for extra parentheses in fold expressions with single expansion. (#GH101863) +- Clang now omits warnings for extra parentheses in fold expressions with single expansion (#GH101863). Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 6a78ecc1229b3a..6ff451761c07fb 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -15567,7 +15567,7 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { } // When there's only one expansion, the parentheses can be safely eliminated - // to avoid any extra redundancy that may result in incorrect checks + // to avoid any extra redundancy that may result in incorrect checks. // For example, transforming (((args == 0) || ...)) into (args == 0) // allows the omission of parentheses while ensuring precise representation // and avoiding warnings regarding redundant parentheses. >From 7c65ec5a9eea035d826ae543bd4be4a2dfe3b88b Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Fri, 4 Oct 2024 23:37:06 +0300 Subject: [PATCH 6/6] add additional tests --- clang/lib/Sema/SemaExpr.cpp | 1 + clang/lib/Sema/TreeTransform.h | 2 +- clang/test/SemaCXX/warn-assignment-condition.cpp | 6 ++++++ clang/test/SemaTemplate/instantiate-requires-expr.cpp | 2 +- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index ae7bcedfb28c73..976152da39a408 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20219,6 +20219,7 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) { SourceLocation parenLoc = ParenE->getBeginLoc(); if (parenLoc.isInvalid() || parenLoc.isMacroID()) return; + // Don't warn for dependent expressions. if (ParenE->isTypeDependent()) return; diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 6ff451761c07fb..c1b7dcaaccd764 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -15571,7 +15571,7 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { // For example, transforming (((args == 0) || ...)) into (args == 0) // allows the omission of parentheses while ensuring precise representation // and avoiding warnings regarding redundant parentheses. - if (*NumExpansions == 1) + if (*NumExpansions == 1 && !E->isTypeDependent()) Pattern = Pattern->IgnoreParens(); for (unsigned I = 0; I != *NumExpansions; ++I) { diff --git a/clang/test/SemaCXX/warn-assignment-condition.cpp b/clang/test/SemaCXX/warn-assignment-condition.cpp index 4f9d13be3c58d3..07cf7a65d2a412 100644 --- a/clang/test/SemaCXX/warn-assignment-condition.cpp +++ b/clang/test/SemaCXX/warn-assignment-condition.cpp @@ -167,6 +167,11 @@ void t5(auto... args) { if ((((args == 0) or ...))) { } } +void t6(auto a, auto... b) { + static_assert(__is_same_as(decltype((a)), int&)); + static_assert(__is_same_as(decltype(((b), ...)), int&)); +}; + void test() { t1(0, 1); t2<>(); @@ -174,5 +179,6 @@ void test() { t3(0, 1); t4(0, 1); t5(0, 1); + t6(0, 0); } } diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index ce2c060a176045..20a19d731ae169 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -36,7 +36,7 @@ using r1i2 = r1<char>; // expected-error {{constraints not satisfied for class t template<typename... Ts> requires false_v<requires (Ts... ts) {requires ((sizeof(ts) == 2) && ...);}> // expected-note@-1 {{because 'false_v<requires (short ts, unsigned short ts) { requires (sizeof (ts) == 2) && (sizeof (ts) == 2); }>'}} -// expected-note@-2 {{because 'false_v<requires (short ts) { requires sizeof (ts) == 2; }>' evaluated to false}} +// expected-note@-2 {{because 'false_v<requires (short ts) { requires (sizeof (ts) == 2); }>' evaluated to false}} struct r2 {}; using r2i1 = r2<short, unsigned short>; // expected-error {{constraints not satisfied for class template 'r2' [with Ts = <short, unsigned short>]}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits