Skip to content

Commit

Permalink
[clang-format] Don't merge a short block for SBS_Never (#88238)
Browse files Browse the repository at this point in the history
Also fix unit tests.

Fixes #87484.
  • Loading branch information
owenca committed Apr 11, 2024
1 parent fca5191 commit 51f1681
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 26 deletions.
2 changes: 2 additions & 0 deletions clang/lib/Format/FormatToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
8 changes: 6 additions & 2 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]))) {
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion clang/unittests/Format/BracesRemoverTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ TEST_F(BracesRemoverTest, RemoveBraces) {
verifyFormat("if (a) {\n"
" b;\n"
"} else {\n"
" { c; }\n"
" {\n"
" c;\n"
" }\n"
"}",
Style);

Expand Down
84 changes: 67 additions & 17 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -13575,7 +13598,8 @@ TEST_F(FormatTest, IncorrectAccessSpecifier) {
verifyFormat("public\n"
"B {}");
verifyFormat("public\n"
"{}");
"{\n"
"}");
verifyFormat("public\n"
"B { int x; }");
}
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -14095,7 +14144,8 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
" }\n"
" }\n"
" }\n"
" {}\n"
" {\n"
" }\n"
"}\n"
"} // namespace n");

Expand Down
7 changes: 4 additions & 3 deletions clang/unittests/Format/FormatTestMacroExpansion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
24 changes: 24 additions & 0 deletions clang/unittests/Format/TokenAnnotatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2856,6 +2856,30 @@ 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_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"
" foo();\n"
" }\n"
"}");
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
} // namespace format
} // namespace clang

0 comments on commit 51f1681

Please sign in to comment.