From 533fbae8d8d87fb151f852b5273e6e1626321567 Mon Sep 17 00:00:00 2001 From: Owen Pan Date: Sun, 12 Dec 2021 18:25:33 -0800 Subject: [PATCH] [clang-format] Add experimental option to remove LLVM braces See the style examples at: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements Differential Revision: https://reviews.llvm.org/D116316 --- clang/docs/ClangFormatStyleOptions.rst | 56 ++++ clang/docs/ReleaseNotes.rst | 3 + clang/include/clang/Format/Format.h | 55 ++++ clang/lib/Format/Format.cpp | 46 +++ clang/lib/Format/FormatToken.h | 3 + clang/lib/Format/TokenAnnotator.cpp | 1 + clang/lib/Format/UnwrappedLineParser.cpp | 249 ++++++++++++++-- clang/lib/Format/UnwrappedLineParser.h | 27 +- clang/unittests/Format/FormatTest.cpp | 362 +++++++++++++++++++++++ 9 files changed, 774 insertions(+), 28 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 9bc62587c8fb..b752df864c56 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -3402,6 +3402,62 @@ the configuration (without a prefix: ``Auto``). /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of * information */ +**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14` + Remove optional braces of control statements (``if``, ``else``, ``for``, + and ``while``) in C++ according to the LLVM coding style. + + .. warning:: + + This option will be renamed and expanded to support other styles. + + .. warning:: + + Setting this option to `true` could lead to incorrect code formatting due + to clang-format's lack of complete semantic information. As such, extra + care should be taken to review code changes made by this option. + + .. code-block:: c++ + + false: true: + + if (isa(D)) { vs. if (isa(D)) + handleFunctionDecl(D); handleFunctionDecl(D); + } else if (isa(D)) { else if (isa(D)) + handleVarDecl(D); handleVarDecl(D); + } + + if (isa(D)) { vs. if (isa(D)) { + for (auto *A : D.attrs()) { for (auto *A : D.attrs()) + if (shouldProcessAttr(A)) { if (shouldProcessAttr(A)) + handleAttr(A); handleAttr(A); + } } + } + } + + if (isa(D)) { vs. if (isa(D)) + for (auto *A : D.attrs()) { for (auto *A : D.attrs()) + handleAttr(A); handleAttr(A); + } + } + + if (auto *D = (T)(D)) { vs. if (auto *D = (T)(D)) { + if (shouldProcess(D)) { if (shouldProcess(D)) + handleVarDecl(D); handleVarDecl(D); + } else { else + markAsIgnored(D); markAsIgnored(D); + } } + } + + if (a) { vs. if (a) + b(); b(); + } else { else if (c) + if (c) { d(); + d(); else + } else { e(); + e(); + } + } + **SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clang-format 14` Specifies the use of empty lines to separate definition blocks, including classes, structs, enums, and functions. diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8b039b20c0d6..b01f3035fc00 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -310,6 +310,9 @@ clang-format `const` `volatile` `static` `inline` `constexpr` `restrict` to be controlled relative to the `type`. +- Option ``RemoveBracesLLVM`` has been added to remove optional braces of + control statements for the LLVM style. + - Option ``SeparateDefinitionBlocks`` has been added to insert or remove empty lines between definition blocks including functions, classes, structs, enums, and namespaces. diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index fe35b4bd69c9..7841a7a9949d 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3049,6 +3049,60 @@ struct FormatStyle { bool ReflowComments; // clang-format on + /// Remove optional braces of control statements (``if``, ``else``, ``for``, + /// and ``while``) in C++ according to the LLVM coding style. + /// \warning + /// This option will be renamed and expanded to support other styles. + /// \endwarning + /// \warning + /// Setting this option to `true` could lead to incorrect code formatting due + /// to clang-format's lack of complete semantic information. As such, extra + /// care should be taken to review code changes made by this option. + /// \endwarning + /// \code + /// false: true: + /// + /// if (isa(D)) { vs. if (isa(D)) + /// handleFunctionDecl(D); handleFunctionDecl(D); + /// } else if (isa(D)) { else if (isa(D)) + /// handleVarDecl(D); handleVarDecl(D); + /// } + /// + /// if (isa(D)) { vs. if (isa(D)) { + /// for (auto *A : D.attrs()) { for (auto *A : D.attrs()) + /// if (shouldProcessAttr(A)) { if (shouldProcessAttr(A)) + /// handleAttr(A); handleAttr(A); + /// } } + /// } + /// } + /// + /// if (isa(D)) { vs. if (isa(D)) + /// for (auto *A : D.attrs()) { for (auto *A : D.attrs()) + /// handleAttr(A); handleAttr(A); + /// } + /// } + /// + /// if (auto *D = (T)(D)) { vs. if (auto *D = (T)(D)) { + /// if (shouldProcess(D)) { if (shouldProcess(D)) + /// handleVarDecl(D); handleVarDecl(D); + /// } else { else + /// markAsIgnored(D); markAsIgnored(D); + /// } } + /// } + /// + /// if (a) { vs. if (a) + /// b(); b(); + /// } else { else if (c) + /// if (c) { d(); + /// d(); else + /// } else { e(); + /// e(); + /// } + /// } + /// \endcode + /// \version 14 + bool RemoveBracesLLVM; + /// \brief The style if definition blocks should be separated. enum SeparateDefinitionStyle { /// Leave definition blocks as they are. @@ -3858,6 +3912,7 @@ struct FormatStyle { QualifierOrder == R.QualifierOrder && RawStringFormats == R.RawStringFormats && ReferenceAlignment == R.ReferenceAlignment && + RemoveBracesLLVM == R.RemoveBracesLLVM && SeparateDefinitionBlocks == R.SeparateDefinitionBlocks && ShortNamespaceLines == R.ShortNamespaceLines && SortIncludes == R.SortIncludes && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index ffb1b14b76b6..ce37ec7c9a5d 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -781,6 +781,7 @@ template <> struct MappingTraits { IO.mapOptional("RawStringFormats", Style.RawStringFormats); IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment); IO.mapOptional("ReflowComments", Style.ReflowComments); + IO.mapOptional("RemoveBracesLLVM", Style.RemoveBracesLLVM); IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks); IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines); IO.mapOptional("SortIncludes", Style.SortIncludes); @@ -1214,6 +1215,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.UseCRLF = false; LLVMStyle.UseTab = FormatStyle::UT_Never; LLVMStyle.ReflowComments = true; + LLVMStyle.RemoveBracesLLVM = false; LLVMStyle.SpacesInParentheses = false; LLVMStyle.SpacesInSquareBrackets = false; LLVMStyle.SpaceInEmptyBlock = false; @@ -1748,6 +1750,45 @@ FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind Language) const { namespace { +class BracesRemover : public TokenAnalyzer { +public: + BracesRemover(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + + std::pair + analyze(TokenAnnotator &Annotator, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) override { + AffectedRangeMgr.computeAffectedLines(AnnotatedLines); + tooling::Replacements Result; + removeBraces(AnnotatedLines, Result); + return {Result, 0}; + } + +private: + // Remove optional braces. + void removeBraces(SmallVectorImpl &Lines, + tooling::Replacements &Result) { + const auto &SourceMgr = Env.getSourceManager(); + for (AnnotatedLine *Line : Lines) { + removeBraces(Line->Children, Result); + if (!Line->Affected) + continue; + for (FormatToken *Token = Line->First; Token; Token = Token->Next) { + if (!Token->Optional) + continue; + assert(Token->isOneOf(tok::l_brace, tok::r_brace)); + const auto Start = Token == Line->Last + ? Token->WhitespaceRange.getBegin() + : Token->Tok.getLocation(); + const auto Range = + CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc()); + cantFail(Result.add(tooling::Replacement(SourceMgr, Range, ""))); + } + } + } +}; + class JavaScriptRequoter : public TokenAnalyzer { public: JavaScriptRequoter(const Environment &Env, const FormatStyle &Style) @@ -3049,6 +3090,11 @@ reformat(const FormatStyle &Style, StringRef Code, }); } + if (Style.isCpp() && Style.RemoveBracesLLVM) + Passes.emplace_back([&](const Environment &Env) { + return BracesRemover(Env, Expanded).process(); + }); + if (Style.Language == FormatStyle::LK_Cpp) { if (Style.FixNamespaceComments) Passes.emplace_back([&](const Environment &Env) { diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index d410ede32240..837727326373 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -442,6 +442,9 @@ public: /// This starts an array initializer. bool IsArrayInitializer = false; + /// Is optional and can be removed. + bool Optional = false; + /// If this token starts a block, this contains all the unwrapped lines /// in it. SmallVector Children; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 6181880b6456..cc449f9bb039 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -780,6 +780,7 @@ private: unsigned CommaCount = 0; while (CurrentToken) { if (CurrentToken->is(tok::r_brace)) { + assert(Left->Optional == CurrentToken->Optional); Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) { diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 410cce79a087..69fe21cd87f0 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -14,6 +14,7 @@ #include "UnwrappedLineParser.h" #include "FormatToken.h" +#include "TokenAnnotator.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" @@ -315,6 +316,7 @@ void UnwrappedLineParser::reset() { PreprocessorDirectives.clear(); CurrentLines = &Lines; DeclarationScopeStack.clear(); + NestedTooDeep.clear(); PPStack.clear(); Line->FirstStartColumn = FirstStartColumn; } @@ -428,7 +430,47 @@ void UnwrappedLineParser::parseCSharpAttribute() { } while (!eof()); } -void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) { +bool UnwrappedLineParser::precededByCommentOrPPDirective() const { + if (!Lines.empty() && Lines.back().InPPDirective) + return true; + + const FormatToken *Previous = Tokens->getPreviousToken(); + return Previous && Previous->is(tok::comment) && + (Previous->IsMultiline || Previous->NewlinesBefore > 0); +} + +bool UnwrappedLineParser::mightFitOnOneLine() const { + const auto ColumnLimit = Style.ColumnLimit; + if (ColumnLimit == 0) + return true; + + if (Lines.empty()) + return true; + + const auto &PreviousLine = Lines.back(); + const auto &Tokens = PreviousLine.Tokens; + assert(!Tokens.empty()); + const auto *LastToken = Tokens.back().Tok; + assert(LastToken); + if (!LastToken->isOneOf(tok::semi, tok::comment)) + return true; + + AnnotatedLine Line(PreviousLine); + assert(Line.Last == LastToken); + + TokenAnnotator Annotator(Style, Keywords); + Annotator.annotate(Line); + Annotator.calculateFormattingInformation(Line); + + return Line.Level * Style.IndentWidth + LastToken->TotalLength <= ColumnLimit; +} + +// Returns true if a simple block, or false otherwise. (A simple block has a +// single statement that fits on a single line.) +bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind) { + const bool IsPrecededByCommentOrPPDirective = + !Style.RemoveBracesLLVM || precededByCommentOrPPDirective(); + unsigned StatementCount = 0; bool SwitchLabelEncountered = false; do { tok::TokenKind kind = FormatTok->Tok.getKind(); @@ -449,11 +491,24 @@ void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) { if (!FormatTok->is(TT_MacroBlockBegin) && tryToParseBracedList()) continue; parseBlock(); + ++StatementCount; + assert(StatementCount > 0 && "StatementCount overflow!"); addUnwrappedLine(); break; case tok::r_brace: - if (HasOpeningBrace) - return; + if (HasOpeningBrace) { + if (!Style.RemoveBracesLLVM) + return false; + if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 || + IsPrecededByCommentOrPPDirective || + precededByCommentOrPPDirective()) { + return false; + } + const FormatToken *Next = Tokens->peekNextToken(); + if (Next->is(tok::comment) && Next->NewlinesBefore == 0) + return false; + return mightFitOnOneLine(); + } nextToken(); addUnwrappedLine(); break; @@ -493,10 +548,13 @@ void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) { } LLVM_FALLTHROUGH; default: - parseStructuralElement(!HasOpeningBrace); + parseStructuralElement(IfKind, !HasOpeningBrace); + ++StatementCount; + assert(StatementCount > 0 && "StatementCount overflow!"); break; } } while (!eof()); + return false; } void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) { @@ -652,11 +710,13 @@ size_t UnwrappedLineParser::computePPHash() const { return h; } -void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels, - bool MunchSemi, - bool UnindentWhitesmithsBraces) { +UnwrappedLineParser::IfStmtKind +UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels, + bool MunchSemi, + bool UnindentWhitesmithsBraces) { assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) && "'{' or macro block token expected"); + FormatToken *Tok = FormatTok; const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin); FormatTok->setBlockKind(BK_Block); @@ -691,16 +751,28 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels, MustBeDeclaration); if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths) Line->Level += AddLevels; - parseLevel(/*HasOpeningBrace=*/true); + + IfStmtKind IfKind = IfStmtKind::NotIf; + const bool SimpleBlock = parseLevel(/*HasOpeningBrace=*/true, &IfKind); if (eof()) - return; + return IfKind; if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd) : !FormatTok->is(tok::r_brace)) { Line->Level = InitialLevel; FormatTok->setBlockKind(BK_Block); - return; + return IfKind; + } + + if (SimpleBlock && Tok->is(tok::l_brace)) { + assert(FormatTok->is(tok::r_brace)); + const FormatToken *Previous = Tokens->getPreviousToken(); + assert(Previous); + if (Previous->isNot(tok::r_brace) || Previous->Optional) { + Tok->MatchingParen = FormatTok; + FormatTok->MatchingParen = Tok; + } } size_t PPEndHash = computePPHash(); @@ -731,6 +803,8 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels, CurrentLines->size() - 1; } } + + return IfKind; } static bool isGoogScope(const UnwrappedLine &Line) { @@ -1185,7 +1259,8 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() { return addUnwrappedLine(); } -void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) { +void UnwrappedLineParser::parseStructuralElement(IfStmtKind *IfKind, + bool IsTopLevel) { if (Style.Language == FormatStyle::LK_TableGen && FormatTok->is(tok::pp_include)) { nextToken(); @@ -1228,7 +1303,7 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) { if (Style.isJavaScript() && Line->MustBeDeclaration) // field/method declaration. break; - parseIfThenElse(); + parseIfThenElse(IfKind); return; case tok::kw_for: case tok::kw_while: @@ -2138,7 +2213,39 @@ void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) { } while (!eof()); } -void UnwrappedLineParser::parseIfThenElse() { +void UnwrappedLineParser::keepAncestorBraces() { + if (!Style.RemoveBracesLLVM) + return; + + const int MaxNestingLevels = 2; + const int Size = NestedTooDeep.size(); + if (Size >= MaxNestingLevels) + NestedTooDeep[Size - MaxNestingLevels] = true; + NestedTooDeep.push_back(false); +} + +static void markOptionalBraces(FormatToken *LeftBrace) { + if (!LeftBrace) + return; + + assert(LeftBrace->is(tok::l_brace)); + + FormatToken *RightBrace = LeftBrace->MatchingParen; + if (!RightBrace) { + assert(!LeftBrace->Optional); + return; + } + + assert(RightBrace->is(tok::r_brace)); + assert(RightBrace->MatchingParen == LeftBrace); + assert(LeftBrace->Optional == RightBrace->Optional); + + LeftBrace->Optional = true; + RightBrace->Optional = true; +} + +FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind, + bool KeepBraces) { auto HandleAttributes = [this]() { // Handle AttributeMacro, e.g. `if (x) UNLIKELY`. if (FormatTok->is(TT_AttributeMacro)) @@ -2155,10 +2262,17 @@ void UnwrappedLineParser::parseIfThenElse() { if (FormatTok->Tok.is(tok::l_paren)) parseParens(); HandleAttributes(); + bool NeedsUnwrappedLine = false; + keepAncestorBraces(); + + FormatToken *IfLeftBrace = nullptr; + IfStmtKind IfBlockKind = IfStmtKind::NotIf; + if (FormatTok->Tok.is(tok::l_brace)) { + IfLeftBrace = FormatTok; CompoundStatementIndenter Indenter(this, Style, Line->Level); - parseBlock(); + IfBlockKind = parseBlock(); if (Style.BraceWrapping.BeforeElse) addUnwrappedLine(); else @@ -2169,22 +2283,48 @@ void UnwrappedLineParser::parseIfThenElse() { parseStructuralElement(); --Line->Level; } + + bool KeepIfBraces = false; + if (Style.RemoveBracesLLVM) { + assert(!NestedTooDeep.empty()); + KeepIfBraces = (IfLeftBrace && !IfLeftBrace->MatchingParen) || + NestedTooDeep.back() || IfBlockKind == IfStmtKind::IfOnly || + IfBlockKind == IfStmtKind::IfElseIf; + } + + FormatToken *ElseLeftBrace = nullptr; + IfStmtKind Kind = IfStmtKind::IfOnly; + if (FormatTok->Tok.is(tok::kw_else)) { + if (Style.RemoveBracesLLVM) { + NestedTooDeep.back() = false; + Kind = IfStmtKind::IfElse; + } nextToken(); HandleAttributes(); if (FormatTok->Tok.is(tok::l_brace)) { + ElseLeftBrace = FormatTok; CompoundStatementIndenter Indenter(this, Style, Line->Level); - parseBlock(); + if (parseBlock() == IfStmtKind::IfOnly) + Kind = IfStmtKind::IfElseIf; addUnwrappedLine(); } else if (FormatTok->Tok.is(tok::kw_if)) { FormatToken *Previous = Tokens->getPreviousToken(); - bool PrecededByComment = Previous && Previous->is(tok::comment); - if (PrecededByComment) { + const bool IsPrecededByComment = Previous && Previous->is(tok::comment); + if (IsPrecededByComment) { addUnwrappedLine(); ++Line->Level; } - parseIfThenElse(); - if (PrecededByComment) + bool TooDeep = true; + if (Style.RemoveBracesLLVM) { + Kind = IfStmtKind::IfElseIf; + TooDeep = NestedTooDeep.pop_back_val(); + } + ElseLeftBrace = + parseIfThenElse(/*IfKind=*/nullptr, KeepBraces || KeepIfBraces); + if (Style.RemoveBracesLLVM) + NestedTooDeep.push_back(TooDeep); + if (IsPrecededByComment) --Line->Level; } else { addUnwrappedLine(); @@ -2194,9 +2334,40 @@ void UnwrappedLineParser::parseIfThenElse() { addUnwrappedLine(); --Line->Level; } - } else if (NeedsUnwrappedLine) { - addUnwrappedLine(); + } else { + if (Style.RemoveBracesLLVM) + KeepIfBraces = KeepIfBraces || IfBlockKind == IfStmtKind::IfElse; + if (NeedsUnwrappedLine) + addUnwrappedLine(); } + + if (!Style.RemoveBracesLLVM) + return nullptr; + + assert(!NestedTooDeep.empty()); + const bool KeepElseBraces = + (ElseLeftBrace && !ElseLeftBrace->MatchingParen) || NestedTooDeep.back(); + + NestedTooDeep.pop_back(); + + if (!KeepBraces && !KeepIfBraces && !KeepElseBraces) { + markOptionalBraces(IfLeftBrace); + markOptionalBraces(ElseLeftBrace); + } else if (IfLeftBrace) { + FormatToken *IfRightBrace = IfLeftBrace->MatchingParen; + if (IfRightBrace) { + assert(IfRightBrace->MatchingParen == IfLeftBrace); + assert(!IfLeftBrace->Optional); + assert(!IfRightBrace->Optional); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; + } + } + + if (IfKind) + *IfKind = Kind; + + return IfLeftBrace; } void UnwrappedLineParser::parseTryCatch() { @@ -2234,6 +2405,9 @@ void UnwrappedLineParser::parseTryCatch() { if (Style.Language == FormatStyle::LK_Java && FormatTok->is(tok::l_paren)) { parseParens(); } + + keepAncestorBraces(); + if (FormatTok->is(tok::l_brace)) { CompoundStatementIndenter Indenter(this, Style, Line->Level); parseBlock(); @@ -2267,8 +2441,11 @@ void UnwrappedLineParser::parseTryCatch() { parseParens(); continue; } - if (FormatTok->isOneOf(tok::semi, tok::r_brace, tok::eof)) + if (FormatTok->isOneOf(tok::semi, tok::r_brace, tok::eof)) { + if (Style.RemoveBracesLLVM) + NestedTooDeep.pop_back(); return; + } nextToken(); } NeedsUnwrappedLine = false; @@ -2279,6 +2456,10 @@ void UnwrappedLineParser::parseTryCatch() { else NeedsUnwrappedLine = true; } + + if (Style.RemoveBracesLLVM) + NestedTooDeep.pop_back(); + if (NeedsUnwrappedLine) addUnwrappedLine(); } @@ -2385,9 +2566,18 @@ void UnwrappedLineParser::parseForOrWhileLoop() { nextToken(); if (FormatTok->Tok.is(tok::l_paren)) parseParens(); + + keepAncestorBraces(); + if (FormatTok->Tok.is(tok::l_brace)) { + FormatToken *LeftBrace = FormatTok; CompoundStatementIndenter Indenter(this, Style, Line->Level); parseBlock(); + if (Style.RemoveBracesLLVM) { + assert(!NestedTooDeep.empty()); + if (!NestedTooDeep.back()) + markOptionalBraces(LeftBrace); + } addUnwrappedLine(); } else { addUnwrappedLine(); @@ -2395,11 +2585,17 @@ void UnwrappedLineParser::parseForOrWhileLoop() { parseStructuralElement(); --Line->Level; } + + if (Style.RemoveBracesLLVM) + NestedTooDeep.pop_back(); } void UnwrappedLineParser::parseDoWhile() { assert(FormatTok->Tok.is(tok::kw_do) && "'do' expected"); nextToken(); + + keepAncestorBraces(); + if (FormatTok->Tok.is(tok::l_brace)) { CompoundStatementIndenter Indenter(this, Style, Line->Level); parseBlock(); @@ -2412,6 +2608,9 @@ void UnwrappedLineParser::parseDoWhile() { --Line->Level; } + if (Style.RemoveBracesLLVM) + NestedTooDeep.pop_back(); + // FIXME: Add error handling. if (!FormatTok->Tok.is(tok::kw_while)) { addUnwrappedLine(); @@ -2481,6 +2680,9 @@ void UnwrappedLineParser::parseSwitch() { nextToken(); if (FormatTok->Tok.is(tok::l_paren)) parseParens(); + + keepAncestorBraces(); + if (FormatTok->Tok.is(tok::l_brace)) { CompoundStatementIndenter Indenter(this, Style, Line->Level); parseBlock(); @@ -2491,6 +2693,9 @@ void UnwrappedLineParser::parseSwitch() { parseStructuralElement(); --Line->Level; } + + if (Style.RemoveBracesLLVM) + NestedTooDeep.pop_back(); } void UnwrappedLineParser::parseAccessSpecifier() { diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index 0c79723d50fc..8d4d4dca7633 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -81,12 +81,21 @@ public: void parse(); private: + enum class IfStmtKind { + NotIf, // Not an if statement. + IfOnly, // An if statement without the else clause. + IfElse, // An if statement followed by else but not else if. + IfElseIf // An if statement followed by else if. + }; + void reset(); void parseFile(); - void parseLevel(bool HasOpeningBrace); - void parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u, - bool MunchSemi = true, - bool UnindentWhitesmithsBraces = false); + bool precededByCommentOrPPDirective() const; + bool mightFitOnOneLine() const; + bool parseLevel(bool HasOpeningBrace, IfStmtKind *IfKind = nullptr); + IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u, + bool MunchSemi = true, + bool UnindentWhitesmithsBraces = false); void parseChildBlock(); void parsePPDirective(); void parsePPDefine(); @@ -96,13 +105,15 @@ private: void parsePPEndIf(); void parsePPUnknown(); void readTokenWithJavaScriptASI(); - void parseStructuralElement(bool IsTopLevel = false); + void parseStructuralElement(IfStmtKind *IfKind = nullptr, + bool IsTopLevel = false); bool tryToParseBracedList(); bool parseBracedList(bool ContinueOnSemicolons = false, bool IsEnum = false, tok::TokenKind ClosingBraceKind = tok::r_brace); void parseParens(); void parseSquare(bool LambdaIntroducer = false); - void parseIfThenElse(); + void keepAncestorBraces(); + FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false); void parseTryCatch(); void parseForOrWhileLoop(); void parseDoWhile(); @@ -235,6 +246,10 @@ private: // owned outside of and handed into the UnwrappedLineParser. ArrayRef AllTokens; + // Keeps a stack of the states of nested control statements (true if the + // statement contains more than some predefined number of nested statements). + SmallVector NestedTooDeep; + // Represents preprocessor branch type, so we can find matching // #if/#else/#endif directives. enum PPBranchKind { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 27bf1d14e666..fa993857db35 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -18889,6 +18889,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList); CHECK_PARSE_BOOL(Cpp11BracedListStyle); CHECK_PARSE_BOOL(ReflowComments); + CHECK_PARSE_BOOL(RemoveBracesLLVM); CHECK_PARSE_BOOL(SortUsingDeclarations); CHECK_PARSE_BOOL(SpacesInParentheses); CHECK_PARSE_BOOL(SpacesInSquareBrackets); @@ -23334,6 +23335,367 @@ TEST_F(FormatTest, ShortTemplatedArgumentLists) { verifyFormat("struct Z : X {};", Style); } +TEST_F(FormatTest, RemoveBraces) { + FormatStyle Style = getLLVMStyle(); + Style.RemoveBracesLLVM = true; + + // The following eight test cases are fully-braced versions of the examples at + // "llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single- + // statement-bodies-of-if-else-loop-statements". + + // 1. Omit the braces, since the body is simple and clearly associated with + // the if. + verifyFormat("if (isa(D))\n" + " handleFunctionDecl(D);\n" + "else if (isa(D))\n" + " handleVarDecl(D);", + "if (isa(D)) {\n" + " handleFunctionDecl(D);\n" + "} else if (isa(D)) {\n" + " handleVarDecl(D);\n" + "}", + Style); + + // 2. Here we document the condition itself and not the body. + verifyFormat("if (isa(D)) {\n" + " // It is necessary that we explain the situation with this\n" + " // surprisingly long comment, so it would be unclear\n" + " // without the braces whether the following statement is in\n" + " // the scope of the `if`.\n" + " // Because the condition is documented, we can't really\n" + " // hoist this comment that applies to the body above the\n" + " // if.\n" + " handleOtherDecl(D);\n" + "}", + Style); + + // 3. Use braces on the outer `if` to avoid a potential dangling else + // situation. + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " if (shouldProcessAttr(A))\n" + " handleAttr(A);\n" + "}", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " if (shouldProcessAttr(A)) {\n" + " handleAttr(A);\n" + " }\n" + " }\n" + "}", + Style); + + // 4. Use braces for the `if` block to keep it uniform with the else block. + verifyFormat("if (isa(D)) {\n" + " handleFunctionDecl(D);\n" + "} else {\n" + " // In this else case, it is necessary that we explain the\n" + " // situation with this surprisingly long comment, so it\n" + " // would be unclear without the braces whether the\n" + " // following statement is in the scope of the `if`.\n" + " handleOtherDecl(D);\n" + "}", + Style); + + // 5. This should also omit braces. The `for` loop contains only a single + // statement, so it shouldn't have braces. The `if` also only contains a + // single simple statement (the for loop), so it also should omit braces. + verifyFormat("if (isa(D))\n" + " for (auto *A : D.attrs())\n" + " handleAttr(A);", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " handleAttr(A);\n" + " }\n" + "}", + Style); + + // 6. Use braces for the outer `if` since the nested `for` is braced. + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " // In this for loop body, it is necessary that we explain\n" + " // the situation with this surprisingly long comment,\n" + " // forcing braces on the `for` block.\n" + " handleAttr(A);\n" + " }\n" + "}", + Style); + + // 7. Use braces on the outer block because there are more than two levels of + // nesting. + verifyFormat("if (isa(D)) {\n" + " for (auto *A : D.attrs())\n" + " for (ssize_t i : llvm::seq(count))\n" + " handleAttrOnDecl(D, A, i);\n" + "}", + "if (isa(D)) {\n" + " for (auto *A : D.attrs()) {\n" + " for (ssize_t i : llvm::seq(count)) {\n" + " handleAttrOnDecl(D, A, i);\n" + " }\n" + " }\n" + "}", + Style); + + // 8. Use braces on the outer block because of a nested `if`, otherwise the + // compiler would warn: `add explicit braces to avoid dangling else` + verifyFormat("if (auto *D = dyn_cast(D)) {\n" + " if (shouldProcess(D))\n" + " handleVarDecl(D);\n" + " else\n" + " markAsIgnored(D);\n" + "}", + "if (auto *D = dyn_cast(D)) {\n" + " if (shouldProcess(D)) {\n" + " handleVarDecl(D);\n" + " } else {\n" + " markAsIgnored(D);\n" + " }\n" + "}", + Style); + + verifyFormat("if (a)\n" + " b; // comment\n" + "else if (c)\n" + " d; /* comment */\n" + "else\n" + " e;", + "if (a) {\n" + " b; // comment\n" + "} else if (c) {\n" + " d; /* comment */\n" + "} else {\n" + " e;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " b;\n" + " c;\n" + "} else if (d) {\n" + " e;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + "#undef NDEBUG\n" + " b;\n" + "} else {\n" + " c;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " // comment\n" + "} else if (b) {\n" + " c;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " b;\n" + "} else {\n" + " { c; }\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " if (b) // comment\n" + " c;\n" + "} else if (d) {\n" + " e;\n" + "}", + "if (a) {\n" + " if (b) { // comment\n" + " c;\n" + " }\n" + "} else if (d) {\n" + " e;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " if (b) {\n" + " c;\n" + " // comment\n" + " } else if (d) {\n" + " e;\n" + " }\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " if (b)\n" + " c;\n" + "}", + "if (a) {\n" + " if (b) {\n" + " c;\n" + " }\n" + "}", + Style); + + verifyFormat("if (a)\n" + " if (b)\n" + " c;\n" + " else\n" + " d;\n" + "else\n" + " e;", + "if (a) {\n" + " if (b) {\n" + " c;\n" + " } else {\n" + " d;\n" + " }\n" + "} else {\n" + " e;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " // comment\n" + " if (b)\n" + " c;\n" + " else if (d)\n" + " e;\n" + "} else {\n" + " g;\n" + "}", + "if (a) {\n" + " // comment\n" + " if (b) {\n" + " c;\n" + " } else if (d) {\n" + " e;\n" + " }\n" + "} else {\n" + " g;\n" + "}", + Style); + + verifyFormat("if (a)\n" + " b;\n" + "else if (c)\n" + " d;\n" + "else\n" + " e;", + "if (a) {\n" + " b;\n" + "} else {\n" + " if (c) {\n" + " d;\n" + " } else {\n" + " e;\n" + " }\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " if (b)\n" + " c;\n" + " else if (d)\n" + " e;\n" + "} else {\n" + " g;\n" + "}", + "if (a) {\n" + " if (b)\n" + " c;\n" + " else {\n" + " if (d)\n" + " e;\n" + " }\n" + "} else {\n" + " g;\n" + "}", + Style); + + verifyFormat("if (a)\n" + " b;\n" + "else if (c)\n" + " while (d)\n" + " e;\n" + "// comment", + "if (a)\n" + "{\n" + " b;\n" + "} else if (c) {\n" + " while (d) {\n" + " e;\n" + " }\n" + "}\n" + "// comment", + Style); + + verifyFormat("if (a) {\n" + " b;\n" + "} else if (c) {\n" + " d;\n" + "} else {\n" + " e;\n" + " g;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " b;\n" + "} else if (c) {\n" + " d;\n" + "} else {\n" + " e;\n" + "} // comment", + Style); + + verifyFormat("int abs = [](int i) {\n" + " if (i >= 0)\n" + " return i;\n" + " return -i;\n" + "};", + "int abs = [](int i) {\n" + " if (i >= 0) {\n" + " return i;\n" + " }\n" + " return -i;\n" + "};", + Style); + + Style.ColumnLimit = 20; + + verifyFormat("if (a) {\n" + " b = c + // 1 -\n" + " d;\n" + "}", + Style); + + verifyFormat("if (a) {\n" + " b = c >= 0 ? d\n" + " : e;\n" + "}", + "if (a) {\n" + " b = c >= 0 ? d : e;\n" + "}", + Style); + + verifyFormat("if (a)\n" + " b = c > 0 ? d : e;", + "if (a) {\n" + " b = c > 0 ? d : e;\n" + "}", + Style); + + Style.ColumnLimit = 0; + + verifyFormat("if (a)\n" + " b234567890223456789032345678904234567890 = " + "c234567890223456789032345678904234567890;", + "if (a) {\n" + " b234567890223456789032345678904234567890 = " + "c234567890223456789032345678904234567890;\n" + "}", + Style); +} + } // namespace } // namespace format } // namespace clang