[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/owenca closed https://github.com/llvm/llvm-project/pull/70768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
owenca wrote:
BTW, neither `FormatTok->Previous` nor `FormatTok->getPreviousNonComment()`
would have worked.
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/70768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/HazardyKnusperkeks approved this pull request. https://github.com/llvm/llvm-project/pull/70768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
owenca wrote:
Added test cases with comments before the null statement.
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/owenca updated
https://github.com/llvm/llvm-project/pull/70768
>From 7e6030532f99b4128807631e993609ce40171043 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Mon, 30 Oct 2023 22:50:27 -0700
Subject: [PATCH 1/2] [clang-format] Treat empty for/while loops as short loops
A for/while loop with only a semicolon as its body should be treated as an
empty loop.
Fixes #61708.
---
clang/lib/Format/UnwrappedLineParser.cpp | 11 +--
clang/unittests/Format/FormatTest.cpp| 13 +++--
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index 30f9bcbfa29308e..94e238039c2a707 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
+ Tokens->getPreviousToken()->is(tok::r_paren)) {
+nextToken();
+addUnwrappedLine();
+return;
+ }
handleAttributes();
parseLoopBody(KeepBraces, /*WrapRightBrace=*/true);
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 15c12c2b8b0da3f..4c09a7eb106e79e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1355,18 +1355,19 @@ TEST_F(FormatTest,
FormatIfWithoutCompoundStatementButElseWith) {
}
TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
+ verifyFormat("while (true)\n"
+ " ;");
+ verifyFormat("for (;;)\n"
+ " ;");
+
FormatStyle AllowsMergedLoops = getLLVMStyle();
AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
verifyFormat("while (true) continue;", AllowsMergedLoops);
verifyFormat("for (;;) continue;", AllowsMergedLoops);
verifyFormat("for (int : vec) v *= 2;", AllowsMergedLoops);
verifyFormat("BOOST_FOREACH (int , vec) v *= 2;", AllowsMergedLoops);
- verifyFormat("while (true)\n"
- " ;",
- AllowsMergedLoops);
- verifyFormat("for (;;)\n"
- " ;",
- AllowsMergedLoops);
+ verifyFormat("while (true);", AllowsMergedLoops);
+ verifyFormat("for (;;);", AllowsMergedLoops);
verifyFormat("for (;;)\n"
" for (;;) continue;",
AllowsMergedLoops);
>From cd9b3d77210c9612c1ac513c10a583d50f168096 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Wed, 1 Nov 2023 12:23:25 -0700
Subject: [PATCH 2/2] Added test cases with comments before the semicolon
---
clang/unittests/Format/FormatTest.cpp | 13 +
1 file changed, 13 insertions(+)
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 4c09a7eb106e79e..80903e7630c8073 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1362,6 +1362,7 @@ TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
FormatStyle AllowsMergedLoops = getLLVMStyle();
AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
+
verifyFormat("while (true) continue;", AllowsMergedLoops);
verifyFormat("for (;;) continue;", AllowsMergedLoops);
verifyFormat("for (int : vec) v *= 2;", AllowsMergedLoops);
@@ -1405,6 +1406,7 @@ TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
" a++;\n"
"while (true);",
AllowsMergedLoops);
+
// Without braces labels are interpreted differently.
verifyFormat("{\n"
" do\n"
@@ -1413,6 +1415,17 @@ TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
" while (true);\n"
"}",
AllowsMergedLoops);
+
+ // Don't merge if there are comments before the null statement.
+ verifyFormat("while (1) //\n"
+ " ;",
+ AllowsMergedLoops);
+ verifyFormat("for (;;) /**/\n"
+ " ;",
+ AllowsMergedLoops);
+ verifyFormat("while (true) /**/\n"
+ " ;",
+ "while (true) /**/;", AllowsMergedLoops);
}
TEST_F(FormatTest, FormatShortBracedStatements) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
owenca wrote:
I didn't use `getPreviousNonComment()` because there are 3 more cases where we
either can't or probably shouldn't merge the semicolon:
1.
```
while (1) //
;
```
2.
```
while (1) /**/
;
```
3.
```
while (1)
/**/ ;
```
Like `while (1) /**/ ;`, case 3 probably doesn't matter in practice.
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
HazardyKnusperkeks wrote:
Just asking.
But I believe if you use `getPreviousNonComment()` it would be formatted as
that.
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/70768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
owenca wrote:
Does it matter in the real world? Do we really need to decide whether the
following is “short”?
```
/**/ while /**/ ( /**/ true /**/ ) /**/ ; //
```
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
HazardyKnusperkeks wrote:
Is `while (true) /*comment*/;` also short?
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/HazardyKnusperkeks edited https://github.com/llvm/llvm-project/pull/70768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/HazardyKnusperkeks commented: The code is okay, but I do not share the intention. https://github.com/llvm/llvm-project/pull/70768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Author: Owen Pan (owenca)
Changes
A for/while loop with only a semicolon as its body should be treated as an
empty loop.
Fixes #61708.
---
Full diff: https://github.com/llvm/llvm-project/pull/70768.diff
2 Files Affected:
- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+9-2)
- (modified) clang/unittests/Format/FormatTest.cpp (+7-6)
``diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index 30f9bcbfa29308e..94e238039c2a707 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
+ Tokens->getPreviousToken()->is(tok::r_paren)) {
+nextToken();
+addUnwrappedLine();
+return;
+ }
handleAttributes();
parseLoopBody(KeepBraces, /*WrapRightBrace=*/true);
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 15c12c2b8b0da3f..4c09a7eb106e79e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1355,18 +1355,19 @@ TEST_F(FormatTest,
FormatIfWithoutCompoundStatementButElseWith) {
}
TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
+ verifyFormat("while (true)\n"
+ " ;");
+ verifyFormat("for (;;)\n"
+ " ;");
+
FormatStyle AllowsMergedLoops = getLLVMStyle();
AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
verifyFormat("while (true) continue;", AllowsMergedLoops);
verifyFormat("for (;;) continue;", AllowsMergedLoops);
verifyFormat("for (int : vec) v *= 2;", AllowsMergedLoops);
verifyFormat("BOOST_FOREACH (int , vec) v *= 2;", AllowsMergedLoops);
- verifyFormat("while (true)\n"
- " ;",
- AllowsMergedLoops);
- verifyFormat("for (;;)\n"
- " ;",
- AllowsMergedLoops);
+ verifyFormat("while (true);", AllowsMergedLoops);
+ verifyFormat("for (;;);", AllowsMergedLoops);
verifyFormat("for (;;)\n"
" for (;;) continue;",
AllowsMergedLoops);
``
https://github.com/llvm/llvm-project/pull/70768
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Treat empty for/while loops as short loops (PR #70768)
https://github.com/owenca created
https://github.com/llvm/llvm-project/pull/70768
A for/while loop with only a semicolon as its body should be treated as an
empty loop.
Fixes #61708.
>From 7e6030532f99b4128807631e993609ce40171043 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Mon, 30 Oct 2023 22:50:27 -0700
Subject: [PATCH] [clang-format] Treat empty for/while loops as short loops
A for/while loop with only a semicolon as its body should be treated as an
empty loop.
Fixes #61708.
---
clang/lib/Format/UnwrappedLineParser.cpp | 11 +--
clang/unittests/Format/FormatTest.cpp| 13 +++--
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index 30f9bcbfa29308e..94e238039c2a707 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3117,9 +3117,16 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool
HasParens) {
FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
}
- // Event control.
- if (Style.isVerilog())
+
+ if (Style.isVerilog()) {
+// Event control.
parseVerilogSensitivityList();
+ } else if (Style.AllowShortLoopsOnASingleLine && FormatTok->is(tok::semi) &&
+ Tokens->getPreviousToken()->is(tok::r_paren)) {
+nextToken();
+addUnwrappedLine();
+return;
+ }
handleAttributes();
parseLoopBody(KeepBraces, /*WrapRightBrace=*/true);
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 15c12c2b8b0da3f..4c09a7eb106e79e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1355,18 +1355,19 @@ TEST_F(FormatTest,
FormatIfWithoutCompoundStatementButElseWith) {
}
TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
+ verifyFormat("while (true)\n"
+ " ;");
+ verifyFormat("for (;;)\n"
+ " ;");
+
FormatStyle AllowsMergedLoops = getLLVMStyle();
AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
verifyFormat("while (true) continue;", AllowsMergedLoops);
verifyFormat("for (;;) continue;", AllowsMergedLoops);
verifyFormat("for (int : vec) v *= 2;", AllowsMergedLoops);
verifyFormat("BOOST_FOREACH (int , vec) v *= 2;", AllowsMergedLoops);
- verifyFormat("while (true)\n"
- " ;",
- AllowsMergedLoops);
- verifyFormat("for (;;)\n"
- " ;",
- AllowsMergedLoops);
+ verifyFormat("while (true);", AllowsMergedLoops);
+ verifyFormat("for (;;);", AllowsMergedLoops);
verifyFormat("for (;;)\n"
" for (;;) continue;",
AllowsMergedLoops);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
