[clang-format][NFC] Don't call mightFitOnOneLine() unnecessarily

[owenpan]

Summary

Clean up UnwrappedLineParser for RemoveBracesLLVM to avoid calling mightFitOnOneLine() as much as possible.

[sstwcw]

clang/lib/Format/UnwrappedLineParser.cpp

2730

[curdeius]

LGTM. I like it! Thanks!

clang/lib/Format/UnwrappedLineParser.cpp

523

2830

[owenpan]

Fixes typos and makes KeepElseBraces depend on KeepIfBraces.

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(BracedListLBrace)                                                       \
   TYPE(CastRParen)                                                             \
   TYPE(ClassLBrace)                                                            \
   TYPE(CompoundRequirementLBrace)                                              \
   TYPE(ConditionalExpr)                                                        \
   TYPE(ConflictAlternative)                                                    \
   TYPE(ConflictEnd)                                                            \
   TYPE(ConflictStart)                                                          \
+  /* l_brace of if/for/while */                                                \
+  TYPE(ControlStatementLBrace)                                                 \
   TYPE(CppCastLParen)                                                          \
   TYPE(CtorInitializerColon)                                                   \
   TYPE(CtorInitializerComma)                                                   \
   TYPE(DesignatedInitializerLSquare)                                           \
   TYPE(DesignatedInitializerPeriod)                                            \
   TYPE(DictLiteral)                                                            \
+  TYPE(ElseLBrace)                                                             \
   TYPE(EnumLBrace)                                                             \
   TYPE(FatArrow)                                                               \
   TYPE(ForEachMacro)                                                           \
   TYPE(FunctionAnnotationRParen)                                               \
   TYPE(FunctionDeclarationName)                                                \
   TYPE(FunctionLBrace)                                                         \
   TYPE(FunctionLikeOrFreestandingMacro)                                        \
   TYPE(FunctionTypeLParen)                                                     \

clang/lib/Format/UnwrappedLineParser.h

enum class IfStmtKind {
     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();
   bool precededByCommentOrPPDirective() const;
-  bool parseLevel(bool HasOpeningBrace, bool CanContainBracedList,
+  bool parseLevel(const FormatToken *OpeningBrace, bool CanContainBracedList,
                   IfStmtKind *IfKind = nullptr,
                   TokenType NextLBracesType = TT_Unknown);
   bool mightFitOnOneLine(UnwrappedLine &Line) const;
   IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
-                        bool MunchSemi = true,
+                        bool MunchSemi = true, bool KeepBraces = true,
                         bool UnindentWhitesmithsBraces = false,
                         bool CanContainBracedList = true,
                         TokenType NextLBracesType = TT_Unknown);
   void parseChildBlock(bool CanContainBracedList = true,
                        TokenType NextLBracesType = TT_Unknown);
   void parsePPDirective();
   void parsePPDefine();
   void parsePPIf(bool IfDef);
private:
   void parseParens(TokenType AmpAmpTokenType = TT_Unknown);
   void parseSquare(bool LambdaIntroducer = false);
   void keepAncestorBraces();
   void parseUnbracedBody(bool CheckEOF = false);
   void handleAttributes();
   bool handleCppAttributes();
   FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
   void parseTryCatch();
-  void parseLoopBody(bool TryRemoveBraces, bool WrapRightBrace);
+  void parseLoopBody(bool KeepBraces, bool WrapRightBrace);
   void parseForOrWhileLoop();
   void parseDoWhile();
   void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
   void parseModuleImport();
   void parseNew();

clang/lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseFile() {
   // The top-level context in a file always has declarations, except for pre-
   // processor directives and JavaScript files.
   bool MustBeDeclaration = !Line->InPPDirective && !Style.isJavaScript();
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                           MustBeDeclaration);
   if (Style.Language == FormatStyle::LK_TextProto)
     parseBracedList();
   else
-    parseLevel(/*HasOpeningBrace=*/false, /*CanContainBracedList=*/true);
+    parseLevel(/*OpeningBrace=*/nullptr, /*CanContainBracedList=*/true);
   // Make sure to format the remaining tokens.
   //
   // LK_TextProto is special since its top-level is parsed as the body of a
   // braced list, which does not necessarily have natural line separators such
   // as a semicolon. Comments after the last entry that have been determined to
   // not belong to that line, as in:
   //   key: value
   //   // endfile comment
if (!Lines.empty() && Lines.back().InPPDirective)
     return true;
 
   const FormatToken *Previous = Tokens->getPreviousToken();
   return Previous && Previous->is(tok::comment) &&
          (Previous->IsMultiline || Previous->NewlinesBefore > 0);
 }
 
 /// \brief Parses a level, that is ???.
-/// \param HasOpeningBrace If that level is started by an opening brace.
+/// \param OpeningBrace Opening brace (\p nullptr if absent) of that level
 /// \param CanContainBracedList If the content can contain (at any level) a
 /// braced list.
 /// \param NextLBracesType The type for left brace found in this level.
-/// \returns true if a simple block, or false otherwise. (A simple block has a
-/// single statement.)
-bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace,
+/// \returns true if a simple block of if/else/for/while, or false otherwise.
+/// (A simple block has a single statement.)
+bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
                                      bool CanContainBracedList,
                                      IfStmtKind *IfKind,
                                      TokenType NextLBracesType) {
   auto NextLevelLBracesType = NextLBracesType == TT_CompoundRequirementLBrace
                                   ? TT_BracedListLBrace
                                   : TT_Unknown;
   const bool IsPrecededByCommentOrPPDirective =
       !Style.RemoveBracesLLVM || precededByCommentOrPPDirective();
   bool HasLabel = false;
   unsigned StatementCount = 0;
   bool SwitchLabelEncountered = false;
   do {
     if (FormatTok->getType() == TT_AttributeMacro) {
       nextToken();
       continue;
     }
     tok::TokenKind kind = FormatTok->Tok.getKind();
     if (FormatTok->getType() == TT_MacroBlockBegin)
       kind = tok::l_brace;
     else if (FormatTok->getType() == TT_MacroBlockEnd)
       kind = tok::r_brace;
 
-    auto ParseDefault = [this, HasOpeningBrace, IfKind, NextLevelLBracesType,
+    auto ParseDefault = [this, OpeningBrace, IfKind, NextLevelLBracesType,
                          &HasLabel, &StatementCount] {
-      parseStructuralElement(IfKind, !HasOpeningBrace, NextLevelLBracesType,
+      parseStructuralElement(IfKind, !OpeningBrace, NextLevelLBracesType,
                              HasLabel ? nullptr : &HasLabel);
       ++StatementCount;
       assert(StatementCount > 0 && "StatementCount overflow!");
     };
 
     switch (kind) {
     case tok::comment:
       nextToken();
       addUnwrappedLine();
       break;
     case tok::l_brace:
       if (NextLBracesType != TT_Unknown)
         FormatTok->setFinalizedType(NextLBracesType);
       else if (FormatTok->Previous &&
                FormatTok->Previous->ClosesRequiresClause) {
         // We need the 'default' case here to correctly parse a function
         // l_brace.
         ParseDefault();
         continue;
       }
       if (CanContainBracedList && !FormatTok->is(TT_MacroBlockBegin) &&
           tryToParseBracedList())
         continue;
       parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
-                 /*MunchSemi=*/true, /*UnindentWhitesmithBraces=*/false,
-                 CanContainBracedList,
-                 /*NextLBracesType=*/NextLBracesType);
+                 /*MunchSemi=*/true, /*KeepBraces=*/true,
+                 /*UnindentWhitesmithsBraces=*/false, CanContainBracedList,
+                 NextLBracesType);
       ++StatementCount;
       assert(StatementCount > 0 && "StatementCount overflow!");
       addUnwrappedLine();
       break;
     case tok::r_brace:
-      if (HasOpeningBrace) {
-        if (!Style.RemoveBracesLLVM)
+      if (OpeningBrace) {
+        if (!Style.RemoveBracesLLVM ||
+            !OpeningBrace->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace))
           return false;
         if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 || HasLabel ||
             IsPrecededByCommentOrPPDirective ||
             precededByCommentOrPPDirective())
           return false;
         const FormatToken *Next = Tokens->peekNextToken();
         return Next->isNot(tok::comment) || Next->NewlinesBefore > 0;
       }
for (auto &Token : Tokens) {
     Token.Tok->copyFrom(*SavedToken.Tok);
     Token.Children = std::move(SavedToken.Children);
     delete SavedToken.Tok;
   }
 
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
 }
 
-UnwrappedLineParser::IfStmtKind
-UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
-                                bool MunchSemi, bool UnindentWhitesmithsBraces,
-                                bool CanContainBracedList,
-                                TokenType NextLBracesType) {
+UnwrappedLineParser::IfStmtKind UnwrappedLineParser::parseBlock(
+    bool MustBeDeclaration, unsigned AddLevels, bool MunchSemi, bool KeepBraces,
+    bool UnindentWhitesmithsBraces, bool CanContainBracedList,
   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);
 
   // For Whitesmiths mode, jump to the next level prior to skipping over the
   // braces.
if (UnindentWhitesmithsBraces)
     --Line->Level;
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                           MustBeDeclaration);
   if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
     Line->Level += AddLevels;
 
   IfStmtKind IfKind = IfStmtKind::NotIf;
-  const bool SimpleBlock = parseLevel(
-      /*HasOpeningBrace=*/true, CanContainBracedList, &IfKind, NextLBracesType);
+  const bool SimpleBlock =
+      parseLevel(Tok, CanContainBracedList, &IfKind, NextLBracesType);
 
   if (eof())
     return IfKind;
 
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
                  : !FormatTok->is(tok::r_brace)) {
     Line->Level = InitialLevel;
     FormatTok->setBlockKind(BK_Block);
     return IfKind;
   }
 
-  if (SimpleBlock && Tok->is(tok::l_brace)) {
+  if (SimpleBlock && !KeepBraces &&
+      Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace)) {
     assert(FormatTok->is(tok::r_brace));
     const FormatToken *Previous = Tokens->getPreviousToken();
     assert(Previous);
     if (Previous->isNot(tok::r_brace) || Previous->Optional) {
       assert(!CurrentLines->empty());
       if (mightFitOnOneLine(CurrentLines->back())) {
         Tok->MatchingParen = FormatTok;
         FormatTok->MatchingParen = Tok;
case tok::kw_enum:
     return Style.BraceWrapping.AfterEnum;
   default:
     return false;
   }
 }
 
 void UnwrappedLineParser::parseChildBlock(
     bool CanContainBracedList, clang::format::TokenType NextLBracesType) {
+  assert(FormatTok->is(tok::l_brace));
   FormatTok->setBlockKind(BK_Block);
+  const FormatToken *OpeningBrace = FormatTok;
   nextToken();
   {
     bool SkipIndent = (Style.isJavaScript() &&
                        (isGoogScope(*Line) || isIIFE(*Line, Keywords)));
     ScopedLineState LineState(*this);
     ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                             /*MustBeDeclaration=*/false);
     Line->Level += SkipIndent ? 0 : 1;
-    parseLevel(/*HasOpeningBrace=*/true, CanContainBracedList,
-               /*IfKind=*/nullptr, NextLBracesType);
+    parseLevel(OpeningBrace, CanContainBracedList, /*IfKind=*/nullptr,
+               NextLBracesType);
     flushComments(isOnNewLine(*FormatTok));
     Line->Level -= SkipIndent ? 0 : 1;
   }
   nextToken();
 }
 
 void UnwrappedLineParser::parsePPDirective() {
   assert(FormatTok->is(tok::hash) && "'#' expected");

 
 FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
                                                   bool KeepBraces) {
   assert(FormatTok->is(tok::kw_if) && "'if' expected");
   nextToken();
   if (FormatTok->is(tok::exclaim))
     nextToken();
 
-  bool KeepIfBraces = false;
-  bool KeepElseBraces = false;
+  bool KeepIfBraces = true;
   if (FormatTok->is(tok::kw_consteval)) {
-    KeepIfBraces = true;
-    KeepElseBraces = true;
     nextToken();
   } else {
+    if (Style.RemoveBracesLLVM)
+      KeepIfBraces = KeepBraces;
     if (FormatTok->isOneOf(tok::kw_constexpr, tok::identifier))
       nextToken();
     if (FormatTok->is(tok::l_paren))
       parseParens();
   }
   handleAttributes();
 
   bool NeedsUnwrappedLine = false;
   keepAncestorBraces();
 
   FormatToken *IfLeftBrace = nullptr;
   IfStmtKind IfBlockKind = IfStmtKind::NotIf;
 
   if (FormatTok->is(tok::l_brace)) {
+    FormatTok->setFinalizedType(TT_ControlStatementLBrace);
     IfLeftBrace = FormatTok;
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
-    IfBlockKind = parseBlock();
+    IfBlockKind = parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
+                             /*MunchSemi=*/true, KeepIfBraces);
     if (Style.BraceWrapping.BeforeElse)
       addUnwrappedLine();
     else
       NeedsUnwrappedLine = true;
   } else {
     parseUnbracedBody();
   }
 
   if (Style.RemoveBracesLLVM) {
     assert(!NestedTooDeep.empty());
     KeepIfBraces = KeepIfBraces ||
                    (IfLeftBrace && !IfLeftBrace->MatchingParen) ||
                    NestedTooDeep.back() || IfBlockKind == IfStmtKind::IfOnly ||
                    IfBlockKind == IfStmtKind::IfElseIf;
   }
 
+  bool KeepElseBraces = KeepIfBraces;
   FormatToken *ElseLeftBrace = nullptr;
   IfStmtKind Kind = IfStmtKind::IfOnly;
 
   if (FormatTok->is(tok::kw_else)) {
     if (Style.RemoveBracesLLVM) {
       NestedTooDeep.back() = false;
       Kind = IfStmtKind::IfElse;
     }
     nextToken();
     handleAttributes();
     if (FormatTok->is(tok::l_brace)) {
+      FormatTok->setFinalizedType(TT_ElseLBrace);
       ElseLeftBrace = FormatTok;
       CompoundStatementIndenter Indenter(this, Style, Line->Level);
-      if (parseBlock() == IfStmtKind::IfOnly)
+      if (parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
+                     /*MunchSemi=*/true, KeepElseBraces) == IfStmtKind::IfOnly)
         Kind = IfStmtKind::IfElseIf;
       addUnwrappedLine();
     } else if (FormatTok->is(tok::kw_if)) {
       const FormatToken *Previous = Tokens->getPreviousToken();
       assert(Previous);
       const bool IsPrecededByComment = Previous->is(tok::comment);
       if (IsPrecededByComment) {
         addUnwrappedLine();
         ++Line->Level;
       }
       bool TooDeep = true;
       if (Style.RemoveBracesLLVM) {
         Kind = IfStmtKind::IfElseIf;
         TooDeep = NestedTooDeep.pop_back_val();
       }
-      ElseLeftBrace =
-          parseIfThenElse(/*IfKind=*/nullptr, KeepBraces || KeepIfBraces);
+      ElseLeftBrace = parseIfThenElse(/*IfKind=*/nullptr, KeepIfBraces);
       if (Style.RemoveBracesLLVM)
         NestedTooDeep.push_back(TooDeep);
       if (IsPrecededByComment)
         --Line->Level;
     } else {
       parseUnbracedBody(/*CheckEOF=*/true);
     }
   } else {
     if (Style.RemoveBracesLLVM)
       KeepIfBraces = KeepIfBraces || IfBlockKind == IfStmtKind::IfElse;
     if (NeedsUnwrappedLine)
       addUnwrappedLine();
   }
 
   if (!Style.RemoveBracesLLVM)
     return nullptr;
 
   assert(!NestedTooDeep.empty());
   KeepElseBraces = KeepElseBraces ||
                    (ElseLeftBrace && !ElseLeftBrace->MatchingParen) ||
                    NestedTooDeep.back();
 
   NestedTooDeep.pop_back();
 
-  if (!KeepBraces && !KeepIfBraces && !KeepElseBraces) {
+  if (!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);
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);
+    parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/true,
+               /*KeepBraces=*/true, 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->is(tok::semi))
       nextToken();
 
     addUnwrappedLine(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);
 
if (FormatTok->is(tok::l_paren)) {
       if (FormatTok->is(tok::l_brace))
         parseChildBlock();
       return;
     }
     nextToken();
   } while (!eof());
 }
 
-void UnwrappedLineParser::parseLoopBody(bool TryRemoveBraces,
-                                        bool WrapRightBrace) {
+void UnwrappedLineParser::parseLoopBody(bool KeepBraces, bool WrapRightBrace) {
   keepAncestorBraces();
 
   if (FormatTok->is(tok::l_brace)) {
+    if (!KeepBraces)
+      FormatTok->setFinalizedType(TT_ControlStatementLBrace);
     FormatToken *LeftBrace = FormatTok;
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
-    parseBlock();
-    if (TryRemoveBraces) {
+    parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
+               /*MunchSemi=*/true, KeepBraces);
+    if (!KeepBraces) {
       assert(!NestedTooDeep.empty());
       if (!NestedTooDeep.back())
         markOptionalBraces(LeftBrace);
     }
     if (WrapRightBrace)
       addUnwrappedLine();
   } else {
     parseUnbracedBody();
   }
 
-  if (TryRemoveBraces)
+  if (!KeepBraces)
     NestedTooDeep.pop_back();
 }
 
 void UnwrappedLineParser::parseForOrWhileLoop() {
   assert(FormatTok->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) &&
          "'for', 'while' or foreach macro expected");
+  const bool KeepBraces = !Style.RemoveBracesLLVM ||
+                          !FormatTok->isOneOf(tok::kw_for, tok::kw_while);
+
   nextToken();
   // JS' for await ( ...
   if (Style.isJavaScript() && FormatTok->is(Keywords.kw_await))
     nextToken();
   if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
     nextToken();
   if (FormatTok->is(tok::l_paren))
     parseParens();
 
-  parseLoopBody(Style.RemoveBracesLLVM, true);
+  parseLoopBody(KeepBraces, /*WrapRightBrace=*/true);
 }
 
 void UnwrappedLineParser::parseDoWhile() {
   assert(FormatTok->is(tok::kw_do) && "'do' expected");
   nextToken();
 
-  parseLoopBody(false, Style.BraceWrapping.BeforeWhile);
+  parseLoopBody(/*KeepBraces=*/true, Style.BraceWrapping.BeforeWhile);
 
   // FIXME: Add error handling.
   if (!FormatTok->is(tok::kw_while)) {
     addUnwrappedLine();
     return;
   }
 
   // If in Whitesmiths mode, the line with the while() needs to be indented
if (!Tok->is(tok::r_square))
     return false;
   Tok = Tokens->getNextToken();
   if (Tok->is(tok::semi))
     return false;
   return true;
 }
 
 void UnwrappedLineParser::parseJavaEnumBody() {
+  assert(FormatTok->is(tok::l_brace));
+  const FormatToken *OpeningBrace = FormatTok;
+
   // Determine whether the enum is simple, i.e. does not have a semicolon or
   // constants with class bodies. Simple enums can be formatted like braced
   // lists, contracted to a single line, etc.
   unsigned StoredPosition = Tokens->getPosition();
   bool IsSimple = true;
   FormatToken *Tok = Tokens->getNextToken();
   while (!Tok->is(tok::eof)) {
     if (Tok->is(tok::r_brace))
if (FormatTok->is(tok::l_brace)) {
       addUnwrappedLine();
       break;
     } else {
       nextToken();
     }
   }
 
   // Parse the class body after the enum's ";" if any.
-  parseLevel(/*HasOpeningBrace=*/true, /*CanContainBracedList=*/true);
+  parseLevel(OpeningBrace, /*CanContainBracedList=*/true);
   nextToken();
   --Line->Level;
   addUnwrappedLine();
 }
 
 void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   const FormatToken &InitialToken = *FormatTok;
   nextToken();