This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7f9f94b2e2b: [clang-format] Rework Whitesmiths mode to use
line-level values in… (authored by timwoj, committed by HazardyKnusperkeks).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94500/new/
https://reviews.llvm.org/D94500
Files:
clang/docs/ReleaseNotes.rst
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
@@ -14767,6 +14767,7 @@
WhitesmithsBraceStyle);
*/
+ WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
verifyFormat("namespace a\n"
" {\n"
"class A\n"
@@ -14791,6 +14792,89 @@
" } // namespace a",
WhitesmithsBraceStyle);
+ verifyFormat("namespace a\n"
+ " {\n"
+ "namespace b\n"
+ " {\n"
+ "class A\n"
+ " {\n"
+ " void f()\n"
+ " {\n"
+ " if (true)\n"
+ " {\n"
+ " a();\n"
+ " b();\n"
+ " }\n"
+ " }\n"
+ " void g()\n"
+ " {\n"
+ " return;\n"
+ " }\n"
+ " };\n"
+ "struct B\n"
+ " {\n"
+ " int x;\n"
+ " };\n"
+ " } // namespace b\n"
+ " } // namespace a",
+ WhitesmithsBraceStyle);
+
+ WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+ verifyFormat("namespace a\n"
+ " {\n"
+ "namespace b\n"
+ " {\n"
+ " class A\n"
+ " {\n"
+ " void f()\n"
+ " {\n"
+ " if (true)\n"
+ " {\n"
+ " a();\n"
+ " b();\n"
+ " }\n"
+ " }\n"
+ " void g()\n"
+ " {\n"
+ " return;\n"
+ " }\n"
+ " };\n"
+ " struct B\n"
+ " {\n"
+ " int x;\n"
+ " };\n"
+ " } // namespace b\n"
+ " } // namespace a",
+ WhitesmithsBraceStyle);
+
+ WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+ verifyFormat("namespace a\n"
+ " {\n"
+ " namespace b\n"
+ " {\n"
+ " class A\n"
+ " {\n"
+ " void f()\n"
+ " {\n"
+ " if (true)\n"
+ " {\n"
+ " a();\n"
+ " b();\n"
+ " }\n"
+ " }\n"
+ " void g()\n"
+ " {\n"
+ " return;\n"
+ " }\n"
+ " };\n"
+ " struct B\n"
+ " {\n"
+ " int x;\n"
+ " };\n"
+ " } // namespace b\n"
+ " } // namespace a",
+ WhitesmithsBraceStyle);
+
verifyFormat("void f()\n"
" {\n"
" if (true)\n"
@@ -14825,7 +14909,7 @@
" }\n",
WhitesmithsBraceStyle);
- WhitesmithsBraceStyle.IndentCaseBlocks = true;
+ WhitesmithsBraceStyle.IndentCaseLabels = true;
verifyFormat("void switchTest1(int a)\n"
" {\n"
" switch (a)\n"
@@ -14833,7 +14917,7 @@
" case 2:\n"
" {\n"
" }\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
@@ -14843,7 +14927,7 @@
" switch (a)\n"
" {\n"
" case 0:\n"
- " break;\n"
+ " break;\n"
" case 1:\n"
" {\n"
" break;\n"
@@ -14851,9 +14935,9 @@
" case 2:\n"
" {\n"
" }\n"
- " break;\n"
+ " break;\n"
" default:\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
@@ -14866,17 +14950,17 @@
" {\n"
" foo(x);\n"
" }\n"
- " break;\n"
+ " break;\n"
" default:\n"
" {\n"
" foo(1);\n"
" }\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
- WhitesmithsBraceStyle.IndentCaseBlocks = false;
+ WhitesmithsBraceStyle.IndentCaseLabels = false;
verifyFormat("void switchTest4(int a)\n"
" {\n"
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -86,7 +86,8 @@
void parseFile();
void parseLevel(bool HasOpeningBrace);
void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1u,
- bool MunchSemi = true);
+ bool MunchSemi = true,
+ bool UnindentWhitesmithsBraces = false);
void parseChildBlock();
void parsePPDirective();
void parsePPDefine();
@@ -140,7 +141,12 @@
bool tryToParsePropertyAccessor();
void tryToParseJSFunction();
bool tryToParseSimpleAttribute();
- void addUnwrappedLine();
+
+ // Used by addUnwrappedLine to denote whether to keep or remove a level
+ // when resetting the line state.
+ enum class LineLevel { Remove, Keep };
+
+ void addUnwrappedLine(LineLevel AdjustLevel = LineLevel::Remove);
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
@@ -580,12 +580,18 @@
}
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
- bool MunchSemi) {
+ bool MunchSemi,
+ bool UnindentWhitesmithsBraces) {
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
"'{' or macro block token expected");
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 (AddLevels > 0 && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
+ ++Line->Level;
+
size_t PPStartHash = computePPHash();
unsigned InitialLevel = Line->Level;
@@ -602,9 +608,16 @@
? (UnwrappedLine::kInvalidIndex)
: (CurrentLines->size() - 1 - NbPreprocessorDirectives);
+ // Whitesmiths is weird here. The brace needs to be indented for the namespace
+ // block, but the block itself may not be indented depending on the style
+ // settings. This allows the format to back up one level in those cases.
+ if (UnindentWhitesmithsBraces)
+ --Line->Level;
+
ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
MustBeDeclaration);
- Line->Level += AddLevels;
+ if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
+ Line->Level += AddLevels;
parseLevel(/*HasOpeningBrace=*/true);
if (eof())
@@ -636,6 +649,7 @@
nextToken();
Line->Level = InitialLevel;
+ FormatTok->setBlockKind(BK_Block);
if (PPStartHash == PPEndHash) {
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
@@ -2133,12 +2147,28 @@
DeclarationScopeStack.size() > 1)
? 1u
: 0u;
- parseBlock(/*MustBeDeclaration=*/true, AddLevels);
+ bool ManageWhitesmithsBraces =
+ AddLevels == 0u &&
+ Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;
+
+ // If we're in Whitesmiths mode, indent the brace if we're not indenting
+ // the whole block.
+ if (ManageWhitesmithsBraces)
+ ++Line->Level;
+
+ parseBlock(/*MustBeDeclaration=*/true, AddLevels,
+ /*MunchSemi=*/true,
+ /*UnindentWhitesmithsBraces=*/ManageWhitesmithsBraces);
+
// 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(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);
+
+ if (ManageWhitesmithsBraces)
+ --Line->Level;
}
// FIXME: Add error handling.
}
@@ -2224,6 +2254,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();
}
@@ -2236,25 +2271,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++;
}
}
@@ -2922,17 +2951,29 @@
llvm::dbgs() << "\n";
}
-void UnwrappedLineParser::addUnwrappedLine() {
+void UnwrappedLineParser::addUnwrappedLine(LineLevel AdjustLevel) {
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 ClosesWhitesmithsBlock =
+ 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 (ClosesWhitesmithsBlock && AdjustLevel == LineLevel::Remove)
+ --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
@@ -1286,13 +1286,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 (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
(Line.Type == LT_PreprocessorDirective ||
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -206,6 +206,9 @@
- Option ``ShortNamespaceLines`` has been added to give better control
over ``FixNamespaceComments`` when determining a namespace length.
+- Support for Whitesmiths has been improved, with fixes for ``namespace`` blocks
+ and ``case`` blocks and labels.
+
libclang
--------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
