timwoj updated this revision to Diff 328308. timwoj added a comment. Rebase onto master again
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