[clang] [clang-format] Don't merge a short block for SBS_Never (PR #88238)
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 10 00:42:41 PDT 2024
https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/88238
>From 4122a4f0e189afa7aff1010f0061b4f4d2b2a627 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Wed, 10 Apr 2024 00:03:21 -0700
Subject: [PATCH 1/2] [clang-format] Don't merge a short block for SBS_Never
Also fix unit tests.
Fixes #87484.
---
clang/lib/Format/FormatToken.h | 2 +
clang/lib/Format/UnwrappedLineFormatter.cpp | 8 +-
clang/lib/Format/UnwrappedLineParser.cpp | 7 +-
clang/unittests/Format/BracesRemoverTest.cpp | 4 +-
clang/unittests/Format/FormatTest.cpp | 84 +++++++++++++++----
.../Format/FormatTestMacroExpansion.cpp | 7 +-
clang/unittests/Format/TokenAnnotatorTest.cpp | 20 +++++
7 files changed, 106 insertions(+), 26 deletions(-)
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 48b6a9092a8c09..f651e6228c206d 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -35,6 +35,8 @@ namespace format {
TYPE(BinaryOperator) \
TYPE(BitFieldColon) \
TYPE(BlockComment) \
+ /* l_brace of a block that is not the body of a (e.g. loop) statement. */ \
+ TYPE(BlockLBrace) \
TYPE(BracedListLBrace) \
/* The colon at the end of a case label. */ \
TYPE(CaseLabelColon) \
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index fb31980ab9f491..4ae54e56331bdc 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -796,8 +796,12 @@ class LineJoiner {
}
}
- if (const auto *LastNonComment = Line.getLastNonComment();
- LastNonComment && LastNonComment->is(tok::l_brace)) {
+ if (Line.endsWith(tok::l_brace)) {
+ if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never &&
+ Line.First->is(TT_BlockLBrace)) {
+ return 0;
+ }
+
if (IsSplitBlock && Line.First == Line.Last &&
I > AnnotatedLines.begin() &&
(I[-1]->endsWith(tok::kw_else) || IsCtrlStmt(*I[-1]))) {
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index c1f7e2874beb24..603268f771ac52 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -395,9 +395,10 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
ParseDefault();
continue;
}
- if (!InRequiresExpression && FormatTok->isNot(TT_MacroBlockBegin) &&
- tryToParseBracedList()) {
- continue;
+ if (!InRequiresExpression && FormatTok->isNot(TT_MacroBlockBegin)) {
+ if (tryToParseBracedList())
+ continue;
+ FormatTok->setFinalizedType(TT_BlockLBrace);
}
parseBlock();
++StatementCount;
diff --git a/clang/unittests/Format/BracesRemoverTest.cpp b/clang/unittests/Format/BracesRemoverTest.cpp
index 5155eefb9e08c9..2e983b887ffcb2 100644
--- a/clang/unittests/Format/BracesRemoverTest.cpp
+++ b/clang/unittests/Format/BracesRemoverTest.cpp
@@ -209,7 +209,9 @@ TEST_F(BracesRemoverTest, RemoveBraces) {
verifyFormat("if (a) {\n"
" b;\n"
"} else {\n"
- " { c; }\n"
+ " {\n"
+ " c;\n"
+ " }\n"
"}",
Style);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index f312a9e21158a9..4906b3350b5b22 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -52,7 +52,13 @@ TEST_F(FormatTest, FormatsUnwrappedLinesAtFirstFormat) {
}
TEST_F(FormatTest, FormatsNestedBlockStatements) {
- verifyFormat("{\n {\n {}\n }\n}", "{{{}}}");
+ verifyFormat("{\n"
+ " {\n"
+ " {\n"
+ " }\n"
+ " }\n"
+ "}",
+ "{{{}}}");
}
TEST_F(FormatTest, FormatsNestedCall) {
@@ -5669,7 +5675,10 @@ TEST_F(FormatTest, LayoutCodeInMacroDefinitions) {
getLLVMStyleWithColumns(14));
}
-TEST_F(FormatTest, LayoutRemainingTokens) { verifyFormat("{}"); }
+TEST_F(FormatTest, LayoutRemainingTokens) {
+ verifyFormat("{\n"
+ "}");
+}
TEST_F(FormatTest, MacroDefinitionInsideStatement) {
verifyFormat("int x,\n"
@@ -6577,7 +6586,11 @@ TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
}
TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
- verifyFormat("{\n { a #c; }\n}");
+ verifyFormat("{\n"
+ " {\n"
+ " a #c;\n"
+ " }\n"
+ "}");
}
TEST_F(FormatTest, FormatUnbalancedStructuralElements) {
@@ -6937,13 +6950,13 @@ TEST_F(FormatTest, FormatNestedBlocksInMacros) {
}
TEST_F(FormatTest, PutEmptyBlocksIntoOneLine) {
- verifyFormat("{}");
verifyFormat("enum E {};");
verifyFormat("enum E {}");
FormatStyle Style = getLLVMStyle();
Style.SpaceInEmptyBlock = true;
verifyFormat("void f() { }", "void f() {}", Style);
Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+ verifyFormat("{ }", Style);
verifyFormat("while (true) { }", "while (true) {}", Style);
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
Style.BraceWrapping.BeforeElse = false;
@@ -11527,10 +11540,18 @@ TEST_F(FormatTest, UnderstandsNewAndDelete) {
"void new (link p);\n"
"void delete (link p);");
- verifyFormat("{ p->new(); }\n"
- "{ p->delete(); }",
- "{ p->new (); }\n"
- "{ p->delete (); }");
+ verifyFormat("{\n"
+ " p->new();\n"
+ "}\n"
+ "{\n"
+ " p->delete();\n"
+ "}",
+ "{\n"
+ " p->new ();\n"
+ "}\n"
+ "{\n"
+ " p->delete ();\n"
+ "}");
FormatStyle AfterPlacementOperator = getLLVMStyle();
AfterPlacementOperator.SpaceBeforeParens = FormatStyle::SBPO_Custom;
@@ -12352,7 +12373,9 @@ TEST_F(FormatTest, FormatsCasts) {
// FIXME: single value wrapped with paren will be treated as cast.
verifyFormat("void f(int i = (kValue)*kMask) {}");
- verifyFormat("{ (void)F; }");
+ verifyFormat("{\n"
+ " (void)F;\n"
+ "}");
// Don't break after a cast's
verifyFormat("int aaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
@@ -13575,7 +13598,8 @@ TEST_F(FormatTest, IncorrectAccessSpecifier) {
verifyFormat("public\n"
"B {}");
verifyFormat("public\n"
- "{}");
+ "{\n"
+ "}");
verifyFormat("public\n"
"B { int x; }");
}
@@ -13632,10 +13656,31 @@ TEST_F(FormatTest, DoesNotTouchUnwrappedLinesWithErrors) {
}
TEST_F(FormatTest, IncorrectCodeErrorDetection) {
- verifyFormat("{\n {}", "{\n{\n}");
- verifyFormat("{\n {}", "{\n {\n}");
- verifyFormat("{\n {}", "{\n {\n }");
- verifyFormat("{\n {}\n}\n}", "{\n {\n }\n }\n}");
+ verifyFormat("{\n"
+ " {\n"
+ " }",
+ "{\n"
+ "{\n"
+ "}");
+ verifyFormat("{\n"
+ " {\n"
+ " }",
+ "{\n"
+ " {\n"
+ "}");
+ verifyFormat("{\n"
+ " {\n"
+ " }");
+ verifyFormat("{\n"
+ " {\n"
+ " }\n"
+ "}\n"
+ "}",
+ "{\n"
+ " {\n"
+ " }\n"
+ " }\n"
+ "}");
verifyFormat("{\n"
" {\n"
@@ -14080,10 +14125,14 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
"}");
verifyFormat("void foo() {\n"
" { // asdf\n"
- " { int a; }\n"
+ " {\n"
+ " int a;\n"
+ " }\n"
" }\n"
" {\n"
- " { int b; }\n"
+ " {\n"
+ " int b;\n"
+ " }\n"
" }\n"
"}");
verifyFormat("namespace n {\n"
@@ -14095,7 +14144,8 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
" }\n"
" }\n"
" }\n"
- " {}\n"
+ " {\n"
+ " }\n"
"}\n"
"} // namespace n");
diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index 85ab6ea3794e80..d391fe3d715c38 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -43,9 +43,10 @@ TEST_F(FormatTestMacroExpansion, UnexpandConfiguredMacros) {
"STMT",
Style);
verifyFormat("void f() { ID(a *b); }", Style);
- verifyFormat(R"(ID(
- { ID(a *b); });
-)",
+ verifyFormat("ID(\n"
+ " {\n"
+ " ID(a *b);\n"
+ " });",
Style);
verifyIncompleteFormat("ID3({, ID(a *b),\n"
" ;\n"
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index c3153cf6b16f07..20ef7f259d8353 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2856,6 +2856,26 @@ TEST_F(TokenAnnotatorTest, UnderstandsElaboratedTypeSpecifier) {
EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_FunctionLBrace);
}
+TEST_F(TokenAnnotatorTest, BlockLBrace) {
+ auto Tokens = annotate("{\n"
+ " {\n"
+ " foo();\n"
+ " }\n"
+ "}");
+ ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::l_brace, TT_BlockLBrace);
+ EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_BlockLBrace);
+
+ Tokens = annotate("void bar() {\n"
+ " {\n"
+ " foo();\n"
+ " }\n"
+ "}");
+ ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+ EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_BlockLBrace);
+}
+
} // namespace
} // namespace format
} // namespace clang
>From a2d63001d6726ad95b90351a4b048348628065bd Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Wed, 10 Apr 2024 00:39:52 -0700
Subject: [PATCH 2/2] Add EXPECT_BRACE_KIND() tests.
---
clang/unittests/Format/TokenAnnotatorTest.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 20ef7f259d8353..da02ced8c7a949 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2864,7 +2864,9 @@ TEST_F(TokenAnnotatorTest, BlockLBrace) {
"}");
ASSERT_EQ(Tokens.size(), 9u) << Tokens;
EXPECT_TOKEN(Tokens[0], tok::l_brace, TT_BlockLBrace);
+ EXPECT_BRACE_KIND(Tokens[0], BK_Block);
EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_BlockLBrace);
+ EXPECT_BRACE_KIND(Tokens[1], BK_Block);
Tokens = annotate("void bar() {\n"
" {\n"
@@ -2873,7 +2875,9 @@ TEST_F(TokenAnnotatorTest, BlockLBrace) {
"}");
ASSERT_EQ(Tokens.size(), 13u) << Tokens;
EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+ EXPECT_BRACE_KIND(Tokens[4], BK_Block);
EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_BlockLBrace);
+ EXPECT_BRACE_KIND(Tokens[5], BK_Block);
}
} // namespace
More information about the cfe-commits
mailing list