curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixes https://github.com/llvm/llvm-project/issues/53758.
Braces in loops with leading (block) comments were formatted according to
BraceWrapping.AfterFunction and not
AllowShortBlocksOnASingleLine/AllowShortLoopsOnASingleLine.
Previously, the code:
while (true) {
f();
}
/*comment*/ while (true) {
f();
}
was incorrectly formatted to:
while (true) {
f();
}
/*comment*/ while (true) { f(); }
when using config:
BasedOnStyle: LLVM
BreakBeforeBraces: Custom
BraceWrapping:
AfterFunction: false
AllowShortBlocksOnASingleLine: false
AllowShortLoopsOnASingleLine: false
and it was (correctly but by chance) formatted to:
while (true) {
f();
}
/*comment*/ while (true) {
f();
}
when using enabling brace wrapping after functions:
BasedOnStyle: LLVM
BreakBeforeBraces: Custom
BraceWrapping:
AfterFunction: true
AllowShortBlocksOnASingleLine: false
AllowShortLoopsOnASingleLine: false
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119649
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1520,6 +1520,22 @@
TEST_F(FormatTest, FormatShortBracedStatements) {
FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+ EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+ EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+ EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+ verifyFormat("for (;;) {\n"
+ " f();\n"
+ "}");
+ verifyFormat("/*comment*/ for (;;) {\n"
+ " f();\n"
+ "}");
+ verifyFormat("while (true) {\n"
+ " f();\n"
+ "}");
+ verifyFormat("/*comment*/ while (true) {\n"
+ " f();\n"
+ "}");
+
AllowSimpleBracedStatements.IfMacros.push_back("MYIF");
// Where line-lengths matter, a 2-letter synonym that maintains line length.
// Not IF to avoid any confusion that IF is somehow special.
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -319,6 +319,15 @@
bool MergeShortFunctions = ShouldMergeShortFunctions();
+ const FormatToken *FirstNonComment = TheLine->First;
+ if (FirstNonComment->is(tok::comment)) {
+ FirstNonComment = FirstNonComment->getNextNonComment();
+ if (!FirstNonComment)
+ return 0;
+ }
+ // FIXME: There are probably cases where we should use FirstNonComment
+ // instead of TheLine->First.
+
if (Style.CompactNamespaces) {
if (auto nsToken = TheLine->First->getNamespaceToken()) {
int i = 0;
@@ -358,9 +367,9 @@
if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First !=
TheLine->Last)
return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
// Try to merge a control statement block with left brace unwrapped.
- if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
- TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
- TT_ForEachMacro)) {
+ if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
+ FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
+ TT_ForEachMacro)) {
return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
? tryMergeSimpleBlock(I, E, Limit)
: 0;
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1520,6 +1520,22 @@
TEST_F(FormatTest, FormatShortBracedStatements) {
FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+ EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+ EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+ EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+ verifyFormat("for (;;) {\n"
+ " f();\n"
+ "}");
+ verifyFormat("/*comment*/ for (;;) {\n"
+ " f();\n"
+ "}");
+ verifyFormat("while (true) {\n"
+ " f();\n"
+ "}");
+ verifyFormat("/*comment*/ while (true) {\n"
+ " f();\n"
+ "}");
+
AllowSimpleBracedStatements.IfMacros.push_back("MYIF");
// Where line-lengths matter, a 2-letter synonym that maintains line length.
// Not IF to avoid any confusion that IF is somehow special.
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -319,6 +319,15 @@
bool MergeShortFunctions = ShouldMergeShortFunctions();
+ const FormatToken *FirstNonComment = TheLine->First;
+ if (FirstNonComment->is(tok::comment)) {
+ FirstNonComment = FirstNonComment->getNextNonComment();
+ if (!FirstNonComment)
+ return 0;
+ }
+ // FIXME: There are probably cases where we should use FirstNonComment
+ // instead of TheLine->First.
+
if (Style.CompactNamespaces) {
if (auto nsToken = TheLine->First->getNamespaceToken()) {
int i = 0;
@@ -358,9 +367,9 @@
if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First != TheLine->Last)
return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
// Try to merge a control statement block with left brace unwrapped.
- if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
- TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
- TT_ForEachMacro)) {
+ if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
+ FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
+ TT_ForEachMacro)) {
return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
? tryMergeSimpleBlock(I, E, Limit)
: 0;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
