[clang] [Clang] omit parentheses in fold expressions with a single expansion (PR #110761)
Oleksandr T. via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 5 13:10:34 PDT 2024
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.tarasiuk at outlook.com>
Date: Wed, 2 Oct 2024 02:13:51 +0300
Subject: [PATCH 1/8] [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 at -1 {{because 'false_v<requires (short ts, unsigned short ts) { requires (sizeof (ts) == 2) && (sizeof (ts) == 2); }>'}}
-// expected-note at -2 {{because 'false_v<requires (short ts) { requires (sizeof (ts) == 2); }>' evaluated to false}}
+// expected-note at -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.tarasiuk at outlook.com>
Date: Wed, 2 Oct 2024 15:59:26 +0300
Subject: [PATCH 2/8] 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.tarasiuk at outlook.com>
Date: Thu, 3 Oct 2024 02:25:43 +0300
Subject: [PATCH 3/8] 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.tarasiuk at outlook.com>
Date: Thu, 3 Oct 2024 18:18:02 +0300
Subject: [PATCH 4/8] 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.tarasiuk at outlook.com>
Date: Thu, 3 Oct 2024 18:31:24 +0300
Subject: [PATCH 5/8] 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.tarasiuk at outlook.com>
Date: Fri, 4 Oct 2024 23:37:06 +0300
Subject: [PATCH 6/8] 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 at -1 {{because 'false_v<requires (short ts, unsigned short ts) { requires (sizeof (ts) == 2) && (sizeof (ts) == 2); }>'}}
-// expected-note at -2 {{because 'false_v<requires (short ts) { requires sizeof (ts) == 2; }>' evaluated to false}}
+// expected-note at -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 159abbdd7b148c175aa51b66fd0849318e504ccd Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Fri, 4 Oct 2024 23:38:55 +0300
Subject: [PATCH 7/8] cleanup
---
clang/lib/Sema/SemaExpr.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 976152da39a408..ae7bcedfb28c73 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20219,7 +20219,6 @@ 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;
>From 14375b87623074569cc91080a660b3bb5a1ee513 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Sat, 5 Oct 2024 23:00:54 +0300
Subject: [PATCH 8/8] extend ParenExpr with constraints to prevent parentheses
from being ignored
---
clang/include/clang/AST/Expr.h | 15 ++++++++++++++-
clang/lib/Sema/SemaExpr.cpp | 3 ++-
clang/lib/Sema/TreeTransform.h | 13 +++++--------
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 57353855c51e7c..5af0d72935ac50 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -2171,10 +2171,15 @@ class ParenExpr : public Expr {
SourceLocation L, R;
Stmt *Val;
public:
+ enum TransformConstraint : uint8_t {
+ None = 0, // No specific preservation required
+ Preserve = 1, // Parentheses have always to be preserved
+ };
+
ParenExpr(SourceLocation l, SourceLocation r, Expr *val)
: Expr(ParenExprClass, val->getType(), val->getValueKind(),
val->getObjectKind()),
- L(l), R(r), Val(val) {
+ L(l), R(r), Val(val), TC(TransformConstraint::None) {
setDependence(computeDependence(this));
}
@@ -2206,6 +2211,14 @@ class ParenExpr : public Expr {
const_child_range children() const {
return const_child_range(&Val, &Val + 1);
}
+
+ TransformConstraint getTransformConstraint() const { return TC; }
+ void setTransformConstraint(TransformConstraint Constraint) {
+ TC = Constraint;
+ }
+
+private:
+ TransformConstraint TC;
};
/// UnaryOperator - This represents the unary-expression's (except sizeof and
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ae7bcedfb28c73..dd3f6a99cbe03d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20220,7 +20220,8 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) {
if (parenLoc.isInvalid() || parenLoc.isMacroID())
return;
// Don't warn for dependent expressions.
- if (ParenE->isTypeDependent())
+ if (ParenE->isTypeDependent() ||
+ ParenE->getTransformConstraint() == ParenExpr::Preserve)
return;
Expr *E = ParenE->IgnoreParens();
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 062e8b3e8c8e00..43e295338f6585 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -15609,14 +15609,6 @@ 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 && !E->isTypeDependent())
- Pattern = Pattern->IgnoreParens();
-
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(
getSema(), LeftFold ? I : *NumExpansions - I - 1);
@@ -15669,6 +15661,11 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) {
return true;
}
+ if (*NumExpansions == 1) {
+ if (ParenExpr *PE = dyn_cast<ParenExpr>(Result.get()))
+ PE->setTransformConstraint(ParenExpr::Preserve);
+ }
+
// If we had no init and an empty pack, and we're not retaining an expansion,
// then produce a fallback value or error.
if (Result.isUnset())
More information about the cfe-commits
mailing list