[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-05 Thread via cfe-commits 

llvmbot wrote:

/pull-request llvm/llvm-project#91165

https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-05 Thread Owen Pan via cfe-commits 

owenca wrote:

/cherry-pick db0ed5533368

https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-05 Thread Owen Pan via cfe-commits 

https://github.com/owenca milestoned 
https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-05 Thread Owen Pan via cfe-commits 

https://github.com/owenca closed https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-05 Thread Owen Pan via cfe-commits 

owenca wrote:

> I wouldn't worry too much about this, but, pedantically, you can be sure it's 
> a fold expression if the ellipsis is followed with or preceded by an operator
> 
> https://eel.is/c++draft/expr.prim.fold#nt:fold-operator

I had thought of that but decided not to bother. From 
https://en.cppreference.com/w/cpp/language/fold:
```
op - any of the following 32 binary operators: + - * / % ^ & | = < > << >> += 
-= *= /= %= ^= &= |= <<= >>= == != <= >= && || , .* ->*
```
Because `<` and `>` are on the list, we still wouldn't be able to tell if 
`...>` is part of a fold expression. Also, I don't like the overhead of up to 
32 comparisons.

https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-04 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)


Changes

Fixes #90966.

---
Full diff: https://github.com/llvm/llvm-project/pull/91045.diff


2 Files Affected:

- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+6-1) 
- (modified) clang/unittests/Format/FormatTest.cpp (+9) 


``diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index e8a8dd58d07eea..7d0278bce9a9c6 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2510,6 +2510,7 @@ bool UnwrappedLineParser::parseParens(TokenType 
AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   auto *LeftParen = FormatTok;
   bool SeenEqual = false;
+  bool MightBeFoldExpr = false;
   const bool MightBeStmtExpr = Tokens->peekNextToken()->is(tok::l_brace);
   nextToken();
   do {
@@ -2521,7 +2522,7 @@ bool UnwrappedLineParser::parseParens(TokenType 
AmpAmpTokenType) {
 parseChildBlock();
   break;
 case tok::r_paren:
-  if (!MightBeStmtExpr && !Line->InMacroBody &&
+  if (!MightBeStmtExpr && !MightBeFoldExpr && !Line->InMacroBody &&
   Style.RemoveParentheses > FormatStyle::RPS_Leave) {
 const auto *Prev = LeftParen->Previous;
 const auto *Next = Tokens->peekNextToken();
@@ -2564,6 +2565,10 @@ bool UnwrappedLineParser::parseParens(TokenType 
AmpAmpTokenType) {
 parseBracedList();
   }
   break;
+case tok::ellipsis:
+  MightBeFoldExpr = true;
+  nextToken();
+  break;
 case tok::equal:
   SeenEqual = true;
   if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 32ba6b6853c799..e6f8e4a06515ea 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27204,8 +27204,14 @@ TEST_F(FormatTest, RemoveParentheses) {
"if ((({ a; })))\n"
"  b;",
Style);
+  verifyFormat("static_assert((std::is_constructible_v && ...));",
+   "static_assert(((std::is_constructible_v && 
...)));",
+   Style);
   verifyFormat("return (0);", "return (((0)));", Style);
   verifyFormat("return (({ 0; }));", "return ((({ 0; })));", Style);
+  verifyFormat("return ((... && std::is_convertible_v));",
+   "return (((... && std::is_convertible_v)));",
+   Style);
 
   Style.RemoveParentheses = FormatStyle::RPS_ReturnStatement;
   verifyFormat("#define Return0 return (0);", Style);
@@ -27213,6 +27219,9 @@ TEST_F(FormatTest, RemoveParentheses) {
   verifyFormat("co_return 0;", "co_return ((0));", Style);
   verifyFormat("return 0;", "return (((0)));", Style);
   verifyFormat("return ({ 0; });", "return ((({ 0; })));", Style);
+  verifyFormat("return (... && std::is_convertible_v);",
+   "return (((... && std::is_convertible_v)));",
+   Style);
   verifyFormat("inline decltype(auto) f() {\n"
"  if (a) {\n"
"return (a);\n"

``




https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-04 Thread Emilia Kond via cfe-commits 

https://github.com/rymiel approved this pull request.

I can definitely think of cases where an expression might contain `...` and 
still have redundant parentheses, i.e. if the ellipsis isn't part of a fold 
expression

For example:
```c++
template 
std::tuple foo() {
return (std::tuple{});
}
```
Those parens around the returned expression are redundant, but they would now 
no longer be removed.

I wouldn't worry too much about this, but, pedantically, you can be sure it's a 
fold expression if the ellipsis is followed with or preceded by an operator

https://eel.is/c++draft/expr.prim.fold#nt:fold-operator

https://github.com/llvm/llvm-project/pull/91045
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-format] Don't remove parentheses of fold expressions (PR #91045)

2024-05-03 Thread Owen Pan via cfe-commits 

https://github.com/owenca created 
https://github.com/llvm/llvm-project/pull/91045

Fixes #90966.

>From 2cc5ef7cca02578262795a7f7ea840d0a1496f74 Mon Sep 17 00:00:00 2001
From: Owen Pan 
Date: Fri, 3 May 2024 22:15:33 -0700
Subject: [PATCH] [clang-format] Don't remove parentheses of fold expressions

Fixes #90966.
---
 clang/lib/Format/UnwrappedLineParser.cpp | 7 ++-
 clang/unittests/Format/FormatTest.cpp| 9 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index e8a8dd58d07eea..7d0278bce9a9c6 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2510,6 +2510,7 @@ bool UnwrappedLineParser::parseParens(TokenType 
AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   auto *LeftParen = FormatTok;
   bool SeenEqual = false;
+  bool MightBeFoldExpr = false;
   const bool MightBeStmtExpr = Tokens->peekNextToken()->is(tok::l_brace);
   nextToken();
   do {
@@ -2521,7 +2522,7 @@ bool UnwrappedLineParser::parseParens(TokenType 
AmpAmpTokenType) {
 parseChildBlock();
   break;
 case tok::r_paren:
-  if (!MightBeStmtExpr && !Line->InMacroBody &&
+  if (!MightBeStmtExpr && !MightBeFoldExpr && !Line->InMacroBody &&
   Style.RemoveParentheses > FormatStyle::RPS_Leave) {
 const auto *Prev = LeftParen->Previous;
 const auto *Next = Tokens->peekNextToken();
@@ -2564,6 +2565,10 @@ bool UnwrappedLineParser::parseParens(TokenType 
AmpAmpTokenType) {
 parseBracedList();
   }
   break;
+case tok::ellipsis:
+  MightBeFoldExpr = true;
+  nextToken();
+  break;
 case tok::equal:
   SeenEqual = true;
   if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 32ba6b6853c799..e6f8e4a06515ea 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27204,8 +27204,14 @@ TEST_F(FormatTest, RemoveParentheses) {
"if ((({ a; })))\n"
"  b;",
Style);
+  verifyFormat("static_assert((std::is_constructible_v && ...));",
+   "static_assert(((std::is_constructible_v && 
...)));",
+   Style);
   verifyFormat("return (0);", "return (((0)));", Style);
   verifyFormat("return (({ 0; }));", "return ((({ 0; })));", Style);
+  verifyFormat("return ((... && std::is_convertible_v));",
+   "return (((... && std::is_convertible_v)));",
+   Style);
 
   Style.RemoveParentheses = FormatStyle::RPS_ReturnStatement;
   verifyFormat("#define Return0 return (0);", Style);
@@ -27213,6 +27219,9 @@ TEST_F(FormatTest, RemoveParentheses) {
   verifyFormat("co_return 0;", "co_return ((0));", Style);
   verifyFormat("return 0;", "return (((0)));", Style);
   verifyFormat("return ({ 0; });", "return ((({ 0; })));", Style);
+  verifyFormat("return (... && std::is_convertible_v);",
+   "return (((... && std::is_convertible_v)));",
+   Style);
   verifyFormat("inline decltype(auto) f() {\n"
"  if (a) {\n"
"return (a);\n"

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits