Skip to content

Commit 4a11e4d

Browse files
itzexpoexpoitzexpoexpo
committed
Fix incorrect handling of left brace wrapping
1 parent 42b1918 commit 4a11e4d

File tree

3 files changed

+65
-100
lines changed

3 files changed

+65
-100
lines changed

‎clang/lib/Format/UnwrappedLineFormatter.cpp‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,14 @@ class LineJoiner {
266266
}
267267
}
268268

269+
// Try merging record blocks that have had their left brace wrapped.
270+
if (TheLine->First->isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union) &&
271+
NextLine.First->is(tok::l_brace) && NextLine.First == NextLine.Last &&
272+
I + 2 != E && !I[2]->First->is(tok::r_brace)) {
273+
if (unsigned MergedLines = tryMergeSimpleBlock(I, E, Limit))
274+
return MergedLines;
275+
}
276+
269277
const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
270278
// Handle empty record blocks where the brace has already been wrapped.
271279
if (PreviousLine && TheLine->Last->is(tok::l_brace) &&
@@ -491,17 +499,17 @@ class LineJoiner {
491499
: 0;
492500
}
493501

494-
const bool MergeShortRecord = [this, &NextLine]() {
502+
const bool TryMergeShortRecord = [this, &NextLine]() {
495503
switch (Style.AllowShortRecordOnASingleLine) {
496-
case FormatStyle::SRS_Always:
497-
return true;
504+
case FormatStyle::SRS_Never:
505+
return false;
498506
case FormatStyle::SRS_EmptyIfAttached:
499507
case FormatStyle::SRS_Empty:
500508
return NextLine.First->is(tok::r_brace);
501-
case FormatStyle::SRS_Never:
502-
return false;
509+
case FormatStyle::SRS_Always:
510+
return true;
503511
}
504-
}();
512+
}() && !Style.BraceWrapping.SplitEmptyRecord;
505513

506514
if (TheLine->Last->is(tok::l_brace)) {
507515
bool ShouldMerge = false;
@@ -516,9 +524,7 @@ class LineJoiner {
516524
// NOTE: We use AfterClass (whereas AfterStruct exists) for both
517525
// classes and structs, but it seems that wrapping is still handled
518526
// correctly elsewhere.
519-
ShouldMerge =
520-
!Style.BraceWrapping.AfterClass ||
521-
(MergeShortRecord && !Style.BraceWrapping.SplitEmptyRecord);
527+
ShouldMerge = !Style.BraceWrapping.AfterClass || TryMergeShortRecord;
522528
}
523529
} else if (TheLine->InPPDirective ||
524530
!TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
@@ -951,9 +957,15 @@ class LineJoiner {
951957
return 0;
952958
Limit -= 2;
953959
unsigned MergedLines = 0;
954-
if (Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never ||
955-
(I[1]->First == I[1]->Last && I + 2 != E &&
956-
I[2]->First->is(tok::r_brace))) {
960+
961+
bool TryMergeBlock =
962+
Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never;
963+
bool TryMergeRecord =
964+
Style.AllowShortRecordOnASingleLine == FormatStyle::SRS_Always;
965+
bool NextIsEmptyBlock = I[1]->First == I[1]->Last && I + 2 != E &&
966+
I[2]->First->is(tok::r_brace);
967+
968+
if (TryMergeBlock || TryMergeRecord || NextIsEmptyBlock) {
957969
MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
958970
// If we managed to merge the block, count the statement header, which
959971
// is on a separate line.

‎clang/lib/Format/UnwrappedLineParser.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4160,11 +4160,8 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr, bool IsJavaRecord) {
41604160
if (ParseAsExpr) {
41614161
parseChildBlock();
41624162
} else {
4163-
if (Style.AllowShortRecordOnASingleLine != FormatStyle::SRS_Always &&
4164-
ShouldBreakBeforeBrace(Style, InitialToken,
4165-
*Tokens->peekNextToken())) {
4163+
if (ShouldBreakBeforeBrace(Style, InitialToken, *Tokens->peekNextToken()))
41664164
addUnwrappedLine();
4167-
}
41684165

41694166
unsigned AddLevels = Style.IndentAccessModifiers ? 2u : 1u;
41704167
parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false);

‎clang/unittests/Format/FormatTest.cpp‎

Lines changed: 40 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -15347,162 +15347,118 @@ TEST_F(FormatTest, NeverMergeShortRecords) {
1534715347
Style);
1534815348
}
1534915349

15350-
TEST_F(FormatTest, AllowShortRecordOnASingleLine) {
15350+
TEST_F(FormatTest, AllowShortRecordOnASingleLineNonSplit) {
1535115351
FormatStyle Style = getLLVMStyle();
1535215352

1535315353
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
1535415354
Style.BraceWrapping.SplitEmptyRecord = false;
1535515355

1535615356
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Never;
15357-
1535815357
verifyFormat("class foo {\n"
1535915358
" void bar();\n"
1536015359
"};",
1536115360
Style);
1536215361
verifyFormat("class foo {\n};", Style);
1536315362

15364-
verifyFormat("struct foo {\n"
15365-
" int bar;\n"
15366-
"};",
15367-
Style);
15368-
verifyFormat("struct foo {\n};", Style);
15369-
15370-
verifyFormat("union foo {\n"
15371-
" int bar;\n"
15372-
"};",
15373-
Style);
15374-
verifyFormat("union foo {\n};", Style);
15375-
1537615363
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_EmptyIfAttached;
15377-
1537815364
verifyFormat("class foo {\n"
1537915365
" void bar();\n"
1538015366
"};",
1538115367
Style);
1538215368
verifyFormat("class foo {};", Style);
1538315369

15384-
verifyFormat("struct foo {\n"
15385-
" void bar();\n"
15386-
"};",
15387-
Style);
15388-
verifyFormat("struct foo {};", Style);
15389-
15390-
verifyFormat("union foo {\n"
15391-
" void bar();\n"
15392-
"};",
15393-
Style);
15394-
verifyFormat("union foo {};", Style);
15395-
1539615370
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Empty;
15397-
1539815371
verifyFormat("class foo {\n"
1539915372
" void bar();\n"
1540015373
"};",
1540115374
Style);
1540215375
verifyFormat("class foo {};", Style);
1540315376

15404-
verifyFormat("struct foo {\n"
15405-
" int bar;\n"
15406-
"};",
15407-
Style);
15408-
verifyFormat("struct foo {};", Style);
15409-
15410-
verifyFormat("union foo {\n"
15411-
" int bar;\n"
15412-
"};",
15413-
Style);
15414-
verifyFormat("union foo {};", Style);
15415-
1541615377
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Always;
15417-
1541815378
verifyFormat("class foo { void bar(); };", Style);
1541915379
verifyFormat("class foo {};", Style);
1542015380

15421-
verifyFormat("struct foo { int bar; };", Style);
15422-
verifyFormat("struct foo {};", Style);
15423-
15424-
verifyFormat("union foo { int bar; };", Style);
15425-
verifyFormat("union foo {};", Style);
15426-
1542715381
Style.BraceWrapping.AfterClass = true;
1542815382
Style.BraceWrapping.AfterStruct = true;
1542915383
Style.BraceWrapping.AfterUnion = true;
1543015384

1543115385
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Never;
15432-
1543315386
verifyFormat("class foo\n{\n"
1543415387
" void bar();\n"
1543515388
"};",
1543615389
Style);
1543715390
verifyFormat("class foo\n{};", Style);
1543815391

15439-
verifyFormat("struct foo\n{\n"
15440-
" int bar;\n"
15441-
"};",
15442-
Style);
15443-
verifyFormat("struct foo\n{};", Style);
15444-
15445-
verifyFormat("union foo\n{\n"
15446-
" int bar;\n"
15447-
"};",
15448-
Style);
15449-
verifyFormat("union foo\n{};", Style);
15450-
1545115392
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_EmptyIfAttached;
15452-
1545315393
verifyFormat("class foo\n{\n"
1545415394
" void bar();\n"
1545515395
"};",
1545615396
Style);
1545715397
verifyFormat("class foo\n{};", Style);
1545815398

15459-
verifyFormat("struct foo\n{\n"
15399+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Empty;
15400+
verifyFormat("class foo\n{\n"
1546015401
" void bar();\n"
1546115402
"};",
1546215403
Style);
15463-
verifyFormat("struct foo\n{};", Style);
15404+
verifyFormat("class foo {};", Style);
1546415405

15465-
verifyFormat("union foo\n{\n"
15466-
" void bar();\n"
15467-
"};",
15468-
Style);
15469-
verifyFormat("union foo\n{};", Style);
15406+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Always;
15407+
verifyFormat("class foo { void bar(); };", Style);
15408+
verifyFormat("class foo {};", Style);
15409+
}
1547015410

15471-
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Empty;
15411+
TEST_F(FormatTest, AllowShortRecordOnASingleLineSplit) {
15412+
FormatStyle Style = getLLVMStyle();
1547215413

15473-
verifyFormat("class foo\n{\n"
15414+
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
15415+
Style.BraceWrapping.SplitEmptyRecord = true;
15416+
15417+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Never;
15418+
verifyFormat("class foo {\n"
1547415419
" void bar();\n"
1547515420
"};",
1547615421
Style);
15477-
verifyFormat("class foo {};", Style);
15422+
verifyFormat("class foo {\n};", Style);
1547815423

15479-
verifyFormat("struct foo\n{\n"
15480-
" int bar;\n"
15424+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_EmptyIfAttached;
15425+
verifyFormat("class foo {\n"
15426+
" void bar();\n"
1548115427
"};",
1548215428
Style);
15483-
verifyFormat("struct foo {};", Style);
15429+
verifyFormat("class foo {};", Style);
1548415430

15485-
verifyFormat("union foo\n{\n"
15486-
" int bar;\n"
15431+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Empty;
15432+
verifyFormat("class foo {\n"
15433+
" void bar();\n"
1548715434
"};",
1548815435
Style);
15489-
verifyFormat("union foo {};", Style);
15436+
verifyFormat("class foo {};", Style);
1549015437

1549115438
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Always;
15492-
1549315439
verifyFormat("class foo { void bar(); };", Style);
1549415440
verifyFormat("class foo {};", Style);
1549515441

15496-
verifyFormat("struct foo { int bar; };", Style);
15497-
verifyFormat("struct foo {};", Style);
15442+
Style.BraceWrapping.AfterClass = true;
15443+
Style.BraceWrapping.AfterStruct = true;
15444+
Style.BraceWrapping.AfterUnion = true;
15445+
15446+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Never;
15447+
verifyFormat("class foo\n{\n}", Style);
15448+
verifyFormat("struct foo\n{\n}", Style);
15449+
verifyFormat("union foo\n{\n}", Style);
1549815450

15499-
verifyFormat("union foo { int bar; };", Style);
15500-
verifyFormat("union foo {};", Style);
15451+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_EmptyIfAttached;
15452+
verifyFormat("class foo\n{\n}", Style);
15453+
verifyFormat("struct foo\n{\n}", Style);
15454+
verifyFormat("union foo\n{\n}", Style);
1550115455

15502-
// Ensure option gets overriden by SplitEmptyRecord
1550315456
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Empty;
15504-
Style.BraceWrapping.SplitEmptyRecord = true;
15457+
verifyFormat("class foo\n{\n}", Style);
15458+
verifyFormat("struct foo\n{\n}", Style);
15459+
verifyFormat("union foo\n{\n}", Style);
1550515460

15461+
Style.AllowShortRecordOnASingleLine = FormatStyle::SRS_Always;
1550615462
verifyFormat("class foo\n{\n}", Style);
1550715463
verifyFormat("struct foo\n{\n}", Style);
1550815464
verifyFormat("union foo\n{\n}", Style);

0 commit comments

Comments
 (0)