Skip to content

Commit c7d7ace

Browse files
timwojmydeveloperday
timwoj
authored and
mydeveloperday
committed
[clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser
This commit removes the old way of handling Whitesmiths mode in favor of just setting the levels during parsing and letting the formatter handle it from there. It requires a bit of special-casing during the parsing, but ends up a bit cleaner than before. It also removes some of switch/case unit tests that don't really make much sense when dealing with Whitesmiths. Differential Revision: https://reviews.llvm.org/D94500 (cherry picked from commit f7f9f94)
1 parent 0e16414 commit c7d7ace

File tree

5 files changed

+170
-41
lines changed

5 files changed

+170
-41
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,9 @@ clang-format
389389
``AlignConsecutiveDeclarations`` and ``AlignConsecutiveMacros`` have been modified to allow
390390
alignment across empty lines and/or comments.
391391

392+
- Support for Whitesmiths has been improved, with fixes for ``namespace`` blocks
393+
and ``case`` blocks and labels.
394+
392395
libclang
393396
--------
394397

‎clang/lib/Format/UnwrappedLineFormatter.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,13 +1281,6 @@ void UnwrappedLineFormatter::formatFirstToken(
12811281
if (Newlines)
12821282
Indent = NewlineIndent;
12831283

1284-
// If in Whitemsmiths mode, indent start and end of blocks
1285-
if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
1286-
if (RootToken.isOneOf(tok::l_brace, tok::r_brace, tok::kw_case,
1287-
tok::kw_default))
1288-
Indent += Style.IndentWidth;
1289-
}
1290-
12911284
// Preprocessor directives get indented before the hash only if specified
12921285
if (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
12931286
(Line.Type == LT_PreprocessorDirective ||

‎clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 66 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -579,17 +579,23 @@ size_t UnwrappedLineParser::computePPHash() const {
579579
return h;
580580
}
581581

582-
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
583-
bool MunchSemi) {
582+
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
583+
bool MunchSemi,
584+
bool UnindentWhitesmithsBraces) {
584585
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
585586
"'{' or macro block token expected");
586587
const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
587588
FormatTok->setBlockKind(BK_Block);
588589

590+
// For Whitesmiths mode, jump to the next level prior to skipping over the
591+
// braces.
592+
if (AddLevels > 0 && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
593+
++Line->Level;
594+
589595
size_t PPStartHash = computePPHash();
590596

591597
unsigned InitialLevel = Line->Level;
592-
nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
598+
nextToken(/*LevelDifference=*/AddLevels);
593599

594600
if (MacroBlock && FormatTok->is(tok::l_paren))
595601
parseParens();
@@ -602,10 +608,16 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
602608
? (UnwrappedLine::kInvalidIndex)
603609
: (CurrentLines->size() - 1 - NbPreprocessorDirectives);
604610

611+
// Whitesmiths is weird here. The brace needs to be indented for the namespace
612+
// block, but the block itself may not be indented depending on the style
613+
// settings. This allows the format to back up one level in those cases.
614+
if (UnindentWhitesmithsBraces)
615+
--Line->Level;
616+
605617
ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
606618
MustBeDeclaration);
607-
if (AddLevel)
608-
++Line->Level;
619+
if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
620+
Line->Level += AddLevels;
609621
parseLevel(/*HasOpeningBrace=*/true);
610622

611623
if (eof())
@@ -621,7 +633,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
621633
size_t PPEndHash = computePPHash();
622634

623635
// Munch the closing brace.
624-
nextToken(/*LevelDifference=*/AddLevel ? -1 : 0);
636+
nextToken(/*LevelDifference=*/-AddLevels);
625637

626638
if (MacroBlock && FormatTok->is(tok::l_paren))
627639
parseParens();
@@ -637,6 +649,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
637649
nextToken();
638650

639651
Line->Level = InitialLevel;
652+
FormatTok->setBlockKind(BK_Block);
640653

641654
if (PPStartHash == PPEndHash) {
642655
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
@@ -2128,15 +2141,34 @@ void UnwrappedLineParser::parseNamespace() {
21282141
if (ShouldBreakBeforeBrace(Style, InitialToken))
21292142
addUnwrappedLine();
21302143

2131-
bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||
2132-
(Style.NamespaceIndentation == FormatStyle::NI_Inner &&
2133-
DeclarationScopeStack.size() > 1);
2134-
parseBlock(/*MustBeDeclaration=*/true, AddLevel);
2144+
unsigned AddLevels =
2145+
Style.NamespaceIndentation == FormatStyle::NI_All ||
2146+
(Style.NamespaceIndentation == FormatStyle::NI_Inner &&
2147+
DeclarationScopeStack.size() > 1)
2148+
? 1u
2149+
: 0u;
2150+
bool ManageWhitesmithsBraces =
2151+
AddLevels == 0u &&
2152+
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;
2153+
2154+
// If we're in Whitesmiths mode, indent the brace if we're not indenting
2155+
// the whole block.
2156+
if (ManageWhitesmithsBraces)
2157+
++Line->Level;
2158+
2159+
parseBlock(/*MustBeDeclaration=*/true, AddLevels,
2160+
/*MunchSemi=*/true,
2161+
/*UnindentWhitesmithsBraces=*/ManageWhitesmithsBraces);
2162+
21352163
// Munch the semicolon after a namespace. This is more common than one would
21362164
// think. Putting the semicolon into its own line is very ugly.
21372165
if (FormatTok->Tok.is(tok::semi))
21382166
nextToken();
2139-
addUnwrappedLine();
2167+
2168+
addUnwrappedLine(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);
2169+
2170+
if (ManageWhitesmithsBraces)
2171+
--Line->Level;
21402172
}
21412173
// FIXME: Add error handling.
21422174
}
@@ -2222,6 +2254,11 @@ void UnwrappedLineParser::parseDoWhile() {
22222254
return;
22232255
}
22242256

2257+
// If in Whitesmiths mode, the line with the while() needs to be indented
2258+
// to the same level as the block.
2259+
if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
2260+
++Line->Level;
2261+
22252262
nextToken();
22262263
parseStructuralElement();
22272264
}
@@ -2234,25 +2271,19 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
22342271
if (LeftAlignLabel)
22352272
Line->Level = 0;
22362273

2237-
bool RemoveWhitesmithsCaseIndent =
2238-
(!Style.IndentCaseBlocks &&
2239-
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths);
2240-
2241-
if (RemoveWhitesmithsCaseIndent)
2242-
--Line->Level;
2243-
22442274
if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
22452275
FormatTok->Tok.is(tok::l_brace)) {
22462276

2247-
CompoundStatementIndenter Indenter(
2248-
this, Line->Level, Style.BraceWrapping.AfterCaseLabel,
2249-
Style.BraceWrapping.IndentBraces || RemoveWhitesmithsCaseIndent);
2277+
CompoundStatementIndenter Indenter(this, Line->Level,
2278+
Style.BraceWrapping.AfterCaseLabel,
2279+
Style.BraceWrapping.IndentBraces);
22502280
parseBlock(/*MustBeDeclaration=*/false);
22512281
if (FormatTok->Tok.is(tok::kw_break)) {
22522282
if (Style.BraceWrapping.AfterControlStatement ==
22532283
FormatStyle::BWACS_Always) {
22542284
addUnwrappedLine();
2255-
if (RemoveWhitesmithsCaseIndent) {
2285+
if (!Style.IndentCaseBlocks &&
2286+
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
22562287
Line->Level++;
22572288
}
22582289
}
@@ -2920,17 +2951,29 @@ LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine &Line,
29202951
llvm::dbgs() << "\n";
29212952
}
29222953

2923-
void UnwrappedLineParser::addUnwrappedLine() {
2954+
void UnwrappedLineParser::addUnwrappedLine(LineLevel AdjustLevel) {
29242955
if (Line->Tokens.empty())
29252956
return;
29262957
LLVM_DEBUG({
29272958
if (CurrentLines == &Lines)
29282959
printDebugInfo(*Line);
29292960
});
2961+
2962+
// If this line closes a block when in Whitesmiths mode, remember that
2963+
// information so that the level can be decreased after the line is added.
2964+
// This has to happen after the addition of the line since the line itself
2965+
// needs to be indented.
2966+
bool ClosesWhitesmithsBlock =
2967+
Line->MatchingOpeningBlockLineIndex != UnwrappedLine::kInvalidIndex &&
2968+
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;
2969+
29302970
CurrentLines->push_back(std::move(*Line));
29312971
Line->Tokens.clear();
29322972
Line->MatchingOpeningBlockLineIndex = UnwrappedLine::kInvalidIndex;
29332973
Line->FirstStartColumn = 0;
2974+
2975+
if (ClosesWhitesmithsBlock && AdjustLevel == LineLevel::Remove)
2976+
--Line->Level;
29342977
if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
29352978
CurrentLines->append(
29362979
std::make_move_iterator(PreprocessorDirectives.begin()),

‎clang/lib/Format/UnwrappedLineParser.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ class UnwrappedLineParser {
8585
void reset();
8686
void parseFile();
8787
void parseLevel(bool HasOpeningBrace);
88-
void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
89-
bool MunchSemi = true);
88+
void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1u,
89+
bool MunchSemi = true,
90+
bool UnindentWhitesmithsBraces = false);
9091
void parseChildBlock();
9192
void parsePPDirective();
9293
void parsePPDefine();
@@ -140,7 +141,12 @@ class UnwrappedLineParser {
140141
bool tryToParsePropertyAccessor();
141142
void tryToParseJSFunction();
142143
bool tryToParseSimpleAttribute();
143-
void addUnwrappedLine();
144+
145+
// Used by addUnwrappedLine to denote whether to keep or remove a level
146+
// when resetting the line state.
147+
enum class LineLevel { Remove, Keep };
148+
149+
void addUnwrappedLine(LineLevel AdjustLevel = LineLevel::Remove);
144150
bool eof() const;
145151
// LevelDifference is the difference of levels after and before the current
146152
// token. For example:

‎clang/unittests/Format/FormatTest.cpp

Lines changed: 92 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14741,6 +14741,7 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
1474114741
WhitesmithsBraceStyle);
1474214742
*/
1474314743

14744+
WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
1474414745
verifyFormat("namespace a\n"
1474514746
" {\n"
1474614747
"class A\n"
@@ -14765,6 +14766,89 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
1476514766
" } // namespace a",
1476614767
WhitesmithsBraceStyle);
1476714768

14769+
verifyFormat("namespace a\n"
14770+
" {\n"
14771+
"namespace b\n"
14772+
" {\n"
14773+
"class A\n"
14774+
" {\n"
14775+
" void f()\n"
14776+
" {\n"
14777+
" if (true)\n"
14778+
" {\n"
14779+
" a();\n"
14780+
" b();\n"
14781+
" }\n"
14782+
" }\n"
14783+
" void g()\n"
14784+
" {\n"
14785+
" return;\n"
14786+
" }\n"
14787+
" };\n"
14788+
"struct B\n"
14789+
" {\n"
14790+
" int x;\n"
14791+
" };\n"
14792+
" } // namespace b\n"
14793+
" } // namespace a",
14794+
WhitesmithsBraceStyle);
14795+
14796+
WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
14797+
verifyFormat("namespace a\n"
14798+
" {\n"
14799+
"namespace b\n"
14800+
" {\n"
14801+
" class A\n"
14802+
" {\n"
14803+
" void f()\n"
14804+
" {\n"
14805+
" if (true)\n"
14806+
" {\n"
14807+
" a();\n"
14808+
" b();\n"
14809+
" }\n"
14810+
" }\n"
14811+
" void g()\n"
14812+
" {\n"
14813+
" return;\n"
14814+
" }\n"
14815+
" };\n"
14816+
" struct B\n"
14817+
" {\n"
14818+
" int x;\n"
14819+
" };\n"
14820+
" } // namespace b\n"
14821+
" } // namespace a",
14822+
WhitesmithsBraceStyle);
14823+
14824+
WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
14825+
verifyFormat("namespace a\n"
14826+
" {\n"
14827+
" namespace b\n"
14828+
" {\n"
14829+
" class A\n"
14830+
" {\n"
14831+
" void f()\n"
14832+
" {\n"
14833+
" if (true)\n"
14834+
" {\n"
14835+
" a();\n"
14836+
" b();\n"
14837+
" }\n"
14838+
" }\n"
14839+
" void g()\n"
14840+
" {\n"
14841+
" return;\n"
14842+
" }\n"
14843+
" };\n"
14844+
" struct B\n"
14845+
" {\n"
14846+
" int x;\n"
14847+
" };\n"
14848+
" } // namespace b\n"
14849+
" } // namespace a",
14850+
WhitesmithsBraceStyle);
14851+
1476814852
verifyFormat("void f()\n"
1476914853
" {\n"
1477014854
" if (true)\n"
@@ -14799,15 +14883,15 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
1479914883
" }\n",
1480014884
WhitesmithsBraceStyle);
1480114885

14802-
WhitesmithsBraceStyle.IndentCaseBlocks = true;
14886+
WhitesmithsBraceStyle.IndentCaseLabels = true;
1480314887
verifyFormat("void switchTest1(int a)\n"
1480414888
" {\n"
1480514889
" switch (a)\n"
1480614890
" {\n"
1480714891
" case 2:\n"
1480814892
" {\n"
1480914893
" }\n"
14810-
" break;\n"
14894+
" break;\n"
1481114895
" }\n"
1481214896
" }\n",
1481314897
WhitesmithsBraceStyle);
@@ -14817,17 +14901,17 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
1481714901
" switch (a)\n"
1481814902
" {\n"
1481914903
" case 0:\n"
14820-
" break;\n"
14904+
" break;\n"
1482114905
" case 1:\n"
1482214906
" {\n"
1482314907
" break;\n"
1482414908
" }\n"
1482514909
" case 2:\n"
1482614910
" {\n"
1482714911
" }\n"
14828-
" break;\n"
14912+
" break;\n"
1482914913
" default:\n"
14830-
" break;\n"
14914+
" break;\n"
1483114915
" }\n"
1483214916
" }\n",
1483314917
WhitesmithsBraceStyle);
@@ -14840,17 +14924,17 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
1484014924
" {\n"
1484114925
" foo(x);\n"
1484214926
" }\n"
14843-
" break;\n"
14927+
" break;\n"
1484414928
" default:\n"
1484514929
" {\n"
1484614930
" foo(1);\n"
1484714931
" }\n"
14848-
" break;\n"
14932+
" break;\n"
1484914933
" }\n"
1485014934
" }\n",
1485114935
WhitesmithsBraceStyle);
1485214936

14853-
WhitesmithsBraceStyle.IndentCaseBlocks = false;
14937+
WhitesmithsBraceStyle.IndentCaseLabels = false;
1485414938

1485514939
verifyFormat("void switchTest4(int a)\n"
1485614940
" {\n"

0 commit comments

Comments
 (0)