timwoj updated this revision to Diff 322689.
timwoj added a comment.
Rebased on main
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
@@ -14721,6 +14721,7 @@
WhitesmithsBraceStyle);
*/
+ WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
verifyFormat("namespace a\n"
" {\n"
"class A\n"
@@ -14745,6 +14746,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"
@@ -14779,7 +14863,7 @@
" }\n",
WhitesmithsBraceStyle);
- WhitesmithsBraceStyle.IndentCaseBlocks = true;
+ WhitesmithsBraceStyle.IndentCaseLabels = true;
verifyFormat("void switchTest1(int a)\n"
" {\n"
" switch (a)\n"
@@ -14787,7 +14871,7 @@
" case 2:\n"
" {\n"
" }\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
@@ -14797,7 +14881,7 @@
" switch (a)\n"
" {\n"
" case 0:\n"
- " break;\n"
+ " break;\n"
" case 1:\n"
" {\n"
" break;\n"
@@ -14805,9 +14889,9 @@
" case 2:\n"
" {\n"
" }\n"
- " break;\n"
+ " break;\n"
" default:\n"
- " break;\n"
+ " break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
@@ -14820,17 +14904,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, bool AddLevel = true,
- 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, bool AddLevel,
- 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 (AddLevel && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
+ ++Line->Level;
+
size_t PPStartHash = computePPHash();
unsigned InitialLevel = Line->Level;
@@ -602,9 +608,15 @@
? (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);
- if (AddLevel)
+ if (AddLevel && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
++Line->Level;
parseLevel(/*HasOpeningBrace=*/true);
@@ -637,6 +649,7 @@
nextToken();
Line->Level = InitialLevel;
+ FormatTok->setBlockKind(BK_Block);
if (PPStartHash == PPEndHash) {
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
@@ -2131,12 +2144,27 @@
bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||
(Style.NamespaceIndentation == FormatStyle::NI_Inner &&
DeclarationScopeStack.size() > 1);
- parseBlock(/*MustBeDeclaration=*/true, AddLevel);
+ bool ManageWhitesmithsBraces =
+ !AddLevel && 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, AddLevel,
+ /*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(AddLevel ? LineLevel::Remove : LineLevel::Keep);
+
+ if (ManageWhitesmithsBraces)
+ --Line->Level;
}
// FIXME: Add error handling.
}
@@ -2222,6 +2250,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();
}
@@ -2234,25 +2267,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++;
}
}
@@ -2920,17 +2947,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
@@ -1281,13 +1281,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
@@ -189,6 +189,9 @@
#include "B/A.h"
#include "B/a.h"
+- Support for Whitesmiths has been improved, with fixes for ``namespace`` blocks
+ and ``case`` blocks and labels.
+
libclang
--------
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
