Skip to content

Commit 39e9a64

Browse files
ksyxtstellar
authored andcommitted
[clang-format] Fix DefSeparator empty line issues
- Add or remove empty lines surrounding union blocks. - Fixes #53229, in which keywords like class and struct in a line ending with left brace or whose next line is left brace only, will be falsely recognized as definition line, causing extra empty lines inserted surrounding blocks with no need to be formatted. Reviewed By: MyDeveloperDay, curdeius, HazardyKnusperkeks, owenpan Differential Revision: https://reviews.llvm.org/D119067 (cherry picked from commit a70549a)
1 parent 47ea1e1 commit 39e9a64

File tree

2 files changed

+54
-17
lines changed

2 files changed

+54
-17
lines changed

clang/lib/Format/DefinitionBlockSeparator.cpp

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,31 @@ void DefinitionBlockSeparator::separateBlocks(
3535
const bool IsNeverStyle =
3636
Style.SeparateDefinitionBlocks == FormatStyle::SDS_Never;
3737
const AdditionalKeywords &ExtraKeywords = Tokens.getKeywords();
38-
auto LikelyDefinition = [this, ExtraKeywords](const AnnotatedLine *Line,
39-
bool ExcludeEnum = false) {
38+
auto GetBracketLevelChange = [](const FormatToken *Tok) {
39+
if (Tok->isOneOf(tok::l_brace, tok::l_paren, tok::l_square))
40+
return 1;
41+
if (Tok->isOneOf(tok::r_brace, tok::r_paren, tok::r_square))
42+
return -1;
43+
return 0;
44+
};
45+
auto LikelyDefinition = [&](const AnnotatedLine *Line,
46+
bool ExcludeEnum = false) {
4047
if ((Line->MightBeFunctionDecl && Line->mightBeFunctionDefinition()) ||
4148
Line->startsWithNamespace())
4249
return true;
43-
FormatToken *CurrentToken = Line->First;
44-
while (CurrentToken) {
45-
if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) ||
46-
(Style.isJavaScript() && CurrentToken->is(ExtraKeywords.kw_function)))
47-
return true;
48-
if (!ExcludeEnum && CurrentToken->is(tok::kw_enum))
49-
return true;
50-
CurrentToken = CurrentToken->Next;
50+
int BracketLevel = 0;
51+
for (const FormatToken *CurrentToken = Line->First; CurrentToken;
52+
CurrentToken = CurrentToken->Next) {
53+
if (BracketLevel == 0) {
54+
if ((CurrentToken->isOneOf(tok::kw_class, tok::kw_struct,
55+
tok::kw_union) ||
56+
(Style.isJavaScript() &&
57+
CurrentToken->is(ExtraKeywords.kw_function))))
58+
return true;
59+
if (!ExcludeEnum && CurrentToken->is(tok::kw_enum))
60+
return true;
61+
}
62+
BracketLevel += GetBracketLevelChange(CurrentToken);
5163
}
5264
return false;
5365
};
@@ -102,14 +114,17 @@ void DefinitionBlockSeparator::separateBlocks(
102114
IsPPConditional(OpeningLineIndex - 1);
103115
};
104116
const auto HasEnumOnLine = [&]() {
105-
FormatToken *CurrentToken = CurrentLine->First;
106117
bool FoundEnumKeyword = false;
107-
while (CurrentToken) {
108-
if (CurrentToken->is(tok::kw_enum))
109-
FoundEnumKeyword = true;
110-
else if (FoundEnumKeyword && CurrentToken->is(tok::l_brace))
111-
return true;
112-
CurrentToken = CurrentToken->Next;
118+
int BracketLevel = 0;
119+
for (const FormatToken *CurrentToken = CurrentLine->First; CurrentToken;
120+
CurrentToken = CurrentToken->Next) {
121+
if (BracketLevel == 0) {
122+
if (CurrentToken->is(tok::kw_enum))
123+
FoundEnumKeyword = true;
124+
else if (FoundEnumKeyword && CurrentToken->is(tok::l_brace))
125+
return true;
126+
}
127+
BracketLevel += GetBracketLevelChange(CurrentToken);
113128
}
114129
return FoundEnumKeyword && I + 1 < Lines.size() &&
115130
Lines[I + 1]->First->is(tok::l_brace);

clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,15 @@ TEST_F(DefinitionBlockSeparatorTest, Basic) {
109109
"};",
110110
Style);
111111

112+
verifyFormat("union foo {\n"
113+
" int i, j;\n"
114+
"};\n"
115+
"\n"
116+
"union bar {\n"
117+
" int j, k;\n"
118+
"};",
119+
Style);
120+
112121
verifyFormat("class foo {\n"
113122
" int i, j;\n"
114123
"};\n"
@@ -311,6 +320,9 @@ TEST_F(DefinitionBlockSeparatorTest, Always) {
311320
"int bar3(int j, int k, const enum Bar b) {\n"
312321
" // A comment\n"
313322
" int r = j % k;\n"
323+
" if (struct S = getS()) {\n"
324+
" // if condition\n"
325+
" }\n"
314326
" return r;\n"
315327
"}\n";
316328
std::string Postfix = "\n"
@@ -364,6 +376,9 @@ TEST_F(DefinitionBlockSeparatorTest, Never) {
364376
"int bar3(int j, int k, const enum Bar b) {\n"
365377
" // A comment\n"
366378
" int r = j % k;\n"
379+
" if (struct S = getS()) {\n"
380+
" // if condition\n"
381+
" }\n"
367382
" return r;\n"
368383
"}\n"
369384
"} // namespace";
@@ -425,6 +440,10 @@ TEST_F(DefinitionBlockSeparatorTest, OpeningBracketOwnsLine) {
425440
"{\n"
426441
" // A comment\n"
427442
" int r = j % k;\n"
443+
" if (struct S = getS())\n"
444+
" {\n"
445+
" // if condition\n"
446+
" }\n"
428447
" return r;\n"
429448
"}\n"
430449
"} // namespace NS",
@@ -473,6 +492,9 @@ TEST_F(DefinitionBlockSeparatorTest, Leave) {
473492
"int bar3(int j, int k, const enum Bar b) {\n"
474493
" // A comment\n"
475494
" int r = j % k;\n"
495+
" if (struct S = getS()) {\n"
496+
" // if condition\n"
497+
" }\n"
476498
" return r;\n"
477499
"}\n"
478500
"} // namespace";

0 commit comments

Comments
 (0)