timwoj created this revision.
timwoj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D94500
Files:
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnalyzer.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13525,7 +13525,7 @@
/*
verifyFormat("class A;\n"
"namespace B\n"
- " {\n"
+ "{\n"
"class C;\n"
"// Comment\n"
"class D\n"
@@ -13539,12 +13539,12 @@
" F\n"
" }\n"
" };\n"
- " } // namespace B\n",
+ "} // namespace B\n",
WhitesmithsBraceStyle);
*/
verifyFormat("namespace a\n"
- " {\n"
+ "{\n"
"class A\n"
" {\n"
" void f()\n"
@@ -13564,7 +13564,7 @@
" {\n"
" int x;\n"
" };\n"
- " } // namespace a",
+ "} // namespace a",
WhitesmithsBraceStyle);
verifyFormat("void f()\n"
@@ -13597,11 +13597,11 @@
" do\n"
" {\n"
" c();\n"
- " } while (false)\n"
+ " } while (false);\n"
+ " return;\n"
" }\n",
WhitesmithsBraceStyle);
- WhitesmithsBraceStyle.IndentCaseBlocks = true;
verifyFormat("void switchTest1(int a)\n"
" {\n"
" switch (a)\n"
@@ -13609,7 +13609,7 @@
" case 2:\n"
" {\n"
" }\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13619,7 @@
" switch (a)\n"
" {\n"
" case 0:\n"
- " break;\n"
+ " break;\n"
" case 1:\n"
" {\n"
" break;\n"
@@ -13627,9 +13627,9 @@
" case 2:\n"
" {\n"
" }\n"
- " break;\n"
+ " break;\n"
" default:\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
@@ -13642,65 +13642,12 @@
" {\n"
" foo(x);\n"
" }\n"
- " break;\n"
+ " break;\n"
" default:\n"
" {\n"
" foo(1);\n"
" }\n"
- " break;\n"
- " }\n"
- " }\n",
- WhitesmithsBraceStyle);
-
- WhitesmithsBraceStyle.IndentCaseBlocks = false;
-
- verifyFormat("void switchTest4(int a)\n"
- " {\n"
- " switch (a)\n"
- " {\n"
- " case 2:\n"
- " {\n"
- " }\n"
- " break;\n"
- " }\n"
- " }\n",
- WhitesmithsBraceStyle);
-
- verifyFormat("void switchTest5(int a)\n"
- " {\n"
- " switch (a)\n"
- " {\n"
- " case 0:\n"
- " break;\n"
- " case 1:\n"
- " {\n"
- " foo();\n"
- " break;\n"
- " }\n"
- " case 2:\n"
- " {\n"
- " }\n"
- " break;\n"
- " default:\n"
- " break;\n"
- " }\n"
- " }\n",
- WhitesmithsBraceStyle);
-
- verifyFormat("void switchTest6(int a)\n"
- " {\n"
- " switch (a)\n"
- " {\n"
- " case 0:\n"
- " {\n"
- " foo(x);\n"
- " }\n"
- " break;\n"
- " default:\n"
- " {\n"
- " foo(1);\n"
- " }\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -140,7 +140,7 @@
bool tryToParsePropertyAccessor();
void tryToParseJSFunction();
bool tryToParseSimpleAttribute();
- void addUnwrappedLine();
+ void addUnwrappedLine(bool RemoveLevel = true);
bool eof() const;
// LevelDifference is the difference of levels after and before the current
// token. For example:
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -586,6 +586,11 @@
const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
FormatTok->setBlockKind(BK_Block);
+ // For Whitesmiths mode, jump to the next level prior to skipping over the
+ // braces.
+ if (AddLevel && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
+ ++Line->Level;
+
size_t PPStartHash = computePPHash();
unsigned InitialLevel = Line->Level;
@@ -604,7 +609,7 @@
ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
MustBeDeclaration);
- if (AddLevel)
+ if (AddLevel && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
++Line->Level;
parseLevel(/*HasOpeningBrace=*/true);
@@ -637,6 +642,7 @@
nextToken();
Line->Level = InitialLevel;
+ FormatTok->setBlockKind(BK_Block);
if (PPStartHash == PPEndHash) {
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
@@ -2142,12 +2148,13 @@
bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||
(Style.NamespaceIndentation == FormatStyle::NI_Inner &&
DeclarationScopeStack.size() > 1);
+
parseBlock(/*MustBeDeclaration=*/true, AddLevel);
// Munch the semicolon after a namespace. This is more common than one would
// think. Putting the semicolon into its own line is very ugly.
if (FormatTok->Tok.is(tok::semi))
nextToken();
- addUnwrappedLine();
+ addUnwrappedLine(AddLevel);
}
// FIXME: Add error handling.
}
@@ -2233,6 +2240,11 @@
return;
}
+ // If in Whitesmiths mode, the line with the while() needs to be indented
+ // to the same level as the block
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
+ ++Line->Level;
+
nextToken();
parseStructuralElement();
}
@@ -2245,25 +2257,19 @@
if (LeftAlignLabel)
Line->Level = 0;
- bool RemoveWhitesmithsCaseIndent =
- (!Style.IndentCaseBlocks &&
- Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths);
-
- if (RemoveWhitesmithsCaseIndent)
- --Line->Level;
-
if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
FormatTok->Tok.is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(
- this, Line->Level, Style.BraceWrapping.AfterCaseLabel,
- Style.BraceWrapping.IndentBraces || RemoveWhitesmithsCaseIndent);
+ CompoundStatementIndenter Indenter(this, Line->Level,
+ Style.BraceWrapping.AfterCaseLabel,
+ Style.BraceWrapping.IndentBraces);
parseBlock(/*MustBeDeclaration=*/false);
if (FormatTok->Tok.is(tok::kw_break)) {
if (Style.BraceWrapping.AfterControlStatement ==
FormatStyle::BWACS_Always) {
addUnwrappedLine();
- if (RemoveWhitesmithsCaseIndent) {
+ if (!Style.IndentCaseBlocks &&
+ Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
Line->Level++;
}
}
@@ -2931,17 +2937,29 @@
llvm::dbgs() << "\n";
}
-void UnwrappedLineParser::addUnwrappedLine() {
+void UnwrappedLineParser::addUnwrappedLine(bool RemoveLevel) {
if (Line->Tokens.empty())
return;
LLVM_DEBUG({
if (CurrentLines == &Lines)
printDebugInfo(*Line);
});
+
+ // If this line closes a block when in Whitesmiths mode, remember that
+ // information so that the level can be decreased after the line is added.
+ // This has to happen after the addition of the line since the line itself
+ // needs to be indented.
+ bool ClosesBlock =
+ Line->MatchingOpeningBlockLineIndex != UnwrappedLine::kInvalidIndex &&
+ Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;
+
CurrentLines->push_back(std::move(*Line));
Line->Tokens.clear();
Line->MatchingOpeningBlockLineIndex = UnwrappedLine::kInvalidIndex;
Line->FirstStartColumn = 0;
+
+ if (ClosesBlock && RemoveLevel)
+ --Line->Level;
if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
CurrentLines->append(
std::make_move_iterator(PreprocessorDirectives.begin()),
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1229,13 +1229,6 @@
if (Newlines)
Indent = NewlineIndent;
- // If in Whitemsmiths mode, indent start and end of blocks
- if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
- if (RootToken.isOneOf(tok::l_brace, tok::r_brace, tok::kw_case,
- tok::kw_default))
- Indent += Style.IndentWidth;
- }
-
// Preprocessor directives get indented before the hash only if specified
if (Line.Type == LT_PreprocessorDirective ||
Line.Type == LT_ImportStatement) {
Index: clang/lib/Format/TokenAnalyzer.cpp
===================================================================
--- clang/lib/Format/TokenAnalyzer.cpp
+++ clang/lib/Format/TokenAnalyzer.cpp
@@ -68,7 +68,6 @@
IdentifierTable IdentTable(getFormattingLangOpts(Style));
FormatTokenLexer Lex(Env.getSourceManager(), Env.getFileID(),
Env.getFirstStartColumn(), Style, Encoding, Allocator,
-
IdentTable);
ArrayRef<FormatToken *> Toks(Lex.lex());
SmallVector<FormatToken *, 10> Tokens(Toks.begin(), Toks.end());
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -815,6 +815,8 @@
Expanded.BraceWrapping.BeforeCatch = true;
Expanded.BraceWrapping.BeforeElse = true;
Expanded.BraceWrapping.BeforeLambdaBody = true;
+ Expanded.IndentCaseLabels = true;
+ Expanded.IndentCaseBlocks = false;
break;
case FormatStyle::BS_GNU:
Expanded.BraceWrapping = {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
