[clang-format] Match all block braces

[HazardyKnusperkeks]

Summary

Now every left and right brace of any block (even the block macros) point to each other through the MatchingParen.

This commit on it self is NFC (as far as I can see), it is a preparation for other change(s).

The brace removing part has been rewritten, because that took advantage of the MatchingParen field.

[owenpan]

This wouldn't work for deeply nested loops:

$ cat test.cpp
for (;;) {
  for (;;) {
    for (;;) {
      a;
    }
  }
}
$ clang-format -style='{RemoveBracesLLVM: true}' test.cpp
for (;;)
  for (;;)
    for (;;)
      a;

Expected output:

for (;;) {
  for (;;)
    for (;;)
      a;
}

[owenpan]

Please see D139257 for a different approach.

[HazardyKnusperkeks]

In D138358#3968610, @owenpan wrote:

Please see D139257 for a different approach.

All my hard work... *sniff*.

[owenpan]

In D138358#3968759, @HazardyKnusperkeks wrote:

In D138358#3968610, @owenpan wrote:

Please see D139257 for a different approach.

All my hard work... *sniff*.

I can definitely sympathise as I've abandoned a lot patches, the vast majority of which were never even submitted.

I had spent a lot of time playing with your patch but couldn't decide if it would break removing optional braces. Because I liked your idea of linking block braces early in the parser instead but wanted to avoid changing the signature of parseBlock() in doing so, I came up with a different approach. It was only then that I found the litmus test.

clang/lib/Format/UnwrappedLineParser.h

bool parseLevel(const FormatToken *OpeningBrace = nullptr,
                   bool CanContainBracedList = true,
                   TokenType NextLBracesType = TT_Unknown,
                   IfStmtKind *IfKind = nullptr,
                   FormatToken **IfLeftBrace = nullptr);
   bool mightFitOnOneLine(UnwrappedLine &Line,
                          const FormatToken *OpeningBrace = nullptr) const;
   FormatToken *parseBlock(bool MustBeDeclaration = false,
                           unsigned AddLevels = 1u, bool MunchSemi = true,
-                          bool KeepBraces = true, IfStmtKind *IfKind = nullptr,
+                          bool *KeepBraces = nullptr,
+                          IfStmtKind *IfKind = nullptr,
                           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);

clang/lib/Format/UnwrappedLineParser.cpp

case tok::l_brace:
         ParseDefault();
         continue;
       }
       if (CanContainBracedList && !FormatTok->is(TT_MacroBlockBegin) &&
           tryToParseBracedList()) {
         continue;
       }
       parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
-                 /*MunchSemi=*/true, /*KeepBraces=*/true, /*IfKind=*/nullptr,
+                 /*MunchSemi=*/true, /*KeepBraces=*/nullptr, /*IfKind=*/nullptr,
                  /*UnindentWhitesmithsBraces=*/false, CanContainBracedList,
                  NextLBracesType);
       ++StatementCount;
       assert(StatementCount > 0 && "StatementCount overflow!");
       addUnwrappedLine();
       break;
     case tok::r_brace:
       if (OpeningBrace) {
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;
 }
 
+/// \brief Parses a Block.
+/// \param[in] MustBeDeclaration If the block is for a declaration.
+/// \param[in] AddLevels How many levels should be added to the lines.
+/// \param[in] MunchSemi If a trailing semi should be munched.
+/// \param[in,out] KeepBraces If not provided it's the same as if it is provided
+/// and set to \c true. If provided it will be set to \c true, if the braces of
+/// this block have to be kept (and thus also the braces of the outer scopes).
+/// \param[in] IfKind <Todo>
+/// \param[in] UnindentWhitesmithsBraces If the brace has to be unindented,
+/// because of Whitesmith brace indenting.
+/// \param[in] CanContainBracedList If the block can contain braced init lists.
+/// \param[in] NextLBracesType The type for the next found left braces, when
+/// it's set to something different than TT_Unknown.
+/// \return <Todo>
 FormatToken *UnwrappedLineParser::parseBlock(
-    bool MustBeDeclaration, unsigned AddLevels, bool MunchSemi, bool KeepBraces,
-    IfStmtKind *IfKind, bool UnindentWhitesmithsBraces,
+    bool MustBeDeclaration, unsigned AddLevels, bool MunchSemi,
+    bool *KeepBraces, IfStmtKind *IfKind, bool UnindentWhitesmithsBraces,
     bool CanContainBracedList, TokenType NextLBracesType) {
   auto HandleVerilogBlockLabel = [this]() {
     // ":" name
     if (Style.isVerilog() && FormatTok->is(tok::colon)) {
       nextToken();
       if (Keywords.isVerilogIdentifier(*FormatTok))
         nextToken();
     }
auto RemoveBraces = [=]() mutable {
     if (!SimpleBlock)
       return false;
     assert(Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace));
     assert(FormatTok->is(tok::r_brace));
     const bool WrappedOpeningBrace = !Tok->Previous;
     if (WrappedOpeningBrace && FollowedByComment)
       return false;
     const bool HasRequiredIfBraces = IfLBrace && !IfLBrace->Optional;
-    if (KeepBraces && !HasRequiredIfBraces)
+    if ((!KeepBraces || *KeepBraces) && !HasRequiredIfBraces)
       return false;
     if (Tok->isNot(TT_ElseLBrace) || !HasRequiredIfBraces) {
       const FormatToken *Previous = Tokens->getPreviousToken();
       assert(Previous);
       if (Previous->is(tok::r_brace) && !Previous->Optional)
         return false;
     }
     assert(!CurrentLines->empty());
     auto &LastLine = CurrentLines->back();
     if (LastLine.Level == InitialLevel + 1 && !mightFitOnOneLine(LastLine))
       return false;
     if (Tok->is(TT_ElseLBrace))
       return true;
     if (WrappedOpeningBrace) {
       assert(Index > 0);
       --Index; // The line above the wrapped l_brace.
       Tok = nullptr;
     }
     return mightFitOnOneLine((*CurrentLines)[Index], Tok);
   };
-  if (RemoveBraces()) {
-    Tok->MatchingParen = FormatTok;
-    FormatTok->MatchingParen = Tok;
-  }
+  if (RemoveBraces())
+    Tok->Optional = FormatTok->Optional = true;
+  else if (KeepBraces)
+    *KeepBraces = true;
+
 
   size_t PPEndHash = computePPHash();
 
   // Munch the closing brace.
   nextToken(/*LevelDifference=*/-AddLevels);
 
   // When this is a function block and there is an unnecessary semicolon
   // afterwards then mark it as optional (so the RemoveSemi pass can get rid of
void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
   }
 
   if (CheckEOF && eof())
     addUnwrappedLine();
 
   --Line->Level;
 }
 
-static void markOptionalBraces(FormatToken *LeftBrace) {
-  if (!LeftBrace)
-    return;
-
+static void markOptionalBraces(FormatToken *LeftBrace, bool Optional) {
+  assert(LeftBrace);
   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;
+  LeftBrace->Optional = Optional;
+  RightBrace->Optional = Optional;
 }
 
 void UnwrappedLineParser::handleAttributes() {
   // Handle AttributeMacro, e.g. `if (x) UNLIKELY`.
   if (FormatTok->is(TT_AttributeMacro))
     nextToken();
   handleCppAttributes();
 }
FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
   FormatToken *IfLeftBrace = nullptr;
   IfStmtKind IfBlockKind = IfStmtKind::NotIf;
 
   if (Keywords.isBlockBegin(*FormatTok, Style)) {
     FormatTok->setFinalizedType(TT_ControlStatementLBrace);
     IfLeftBrace = FormatTok;
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
     parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
-               /*MunchSemi=*/true, KeepIfBraces, &IfBlockKind);
+               /*MunchSemi=*/true, &KeepIfBraces, &IfBlockKind);
     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 ||
+    KeepIfBraces = KeepIfBraces || NestedTooDeep.back() ||
+                   IfBlockKind == IfStmtKind::IfOnly ||
                    IfBlockKind == IfStmtKind::IfElseIf;
   }
 
   bool KeepElseBraces = KeepIfBraces;
+  bool MergeElseIf = false;
   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 (Keywords.isBlockBegin(*FormatTok, Style)) {
       const bool FollowedByIf = Tokens->peekNextToken()->is(tok::kw_if);
       FormatTok->setFinalizedType(TT_ElseLBrace);
       ElseLeftBrace = FormatTok;
       CompoundStatementIndenter Indenter(this, Style, Line->Level);
       IfStmtKind ElseBlockKind = IfStmtKind::NotIf;
       FormatToken *IfLBrace =
           parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
-                     /*MunchSemi=*/true, KeepElseBraces, &ElseBlockKind);
+                     /*MunchSemi=*/true, &KeepElseBraces, &ElseBlockKind);
       if (FormatTok->is(tok::kw_else)) {
         KeepElseBraces = KeepElseBraces ||
                          ElseBlockKind == IfStmtKind::IfOnly ||
                          ElseBlockKind == IfStmtKind::IfElseIf;
       } else if (FollowedByIf && IfLBrace && !IfLBrace->Optional) {
         KeepElseBraces = true;
+        MergeElseIf = true;
         assert(ElseLeftBrace->MatchingParen);
-        markOptionalBraces(ElseLeftBrace);
       }
       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, KeepIfBraces);
       if (Style.RemoveBracesLLVM)
         NestedTooDeep.push_back(TooDeep);
       if (IsPrecededByComment)
         --Line->Level;
     } else {
       parseUnbracedBody(/*CheckEOF=*/true);
     }
+    KeepElseBraces =
+        KeepElseBraces || (ElseLeftBrace && !ElseLeftBrace->Optional);
   } else {
     KeepIfBraces = KeepIfBraces || IfBlockKind == IfStmtKind::IfElse;
     if (NeedsUnwrappedLine)
       addUnwrappedLine();
   }
 
   if (!Style.RemoveBracesLLVM)
     return nullptr;
 
   assert(!NestedTooDeep.empty());
-  KeepElseBraces = KeepElseBraces ||
-                   (ElseLeftBrace && !ElseLeftBrace->MatchingParen) ||
-                   NestedTooDeep.back();
+  KeepElseBraces = KeepElseBraces || NestedTooDeep.back();
 
   NestedTooDeep.pop_back();
 
-  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);
-      IfLeftBrace->MatchingParen = nullptr;
-      IfRightBrace->MatchingParen = nullptr;
-    }
-  }
+  bool DropBraces = !KeepIfBraces && !KeepElseBraces;
+  if (IfLeftBrace)
+    markOptionalBraces(IfLeftBrace, DropBraces);
+  if (ElseLeftBrace)
+    markOptionalBraces(ElseLeftBrace, DropBraces || MergeElseIf);
 
   if (IfKind)
     *IfKind = Kind;
 
   return IfLeftBrace;
 }
 
 void UnwrappedLineParser::parseTryCatch() {
bool ManageWhitesmithsBraces =
         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,
-               /*KeepBraces=*/true, /*IfKind=*/nullptr,
+               /*KeepBraces=*/nullptr, /*IfKind=*/nullptr,
                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);
void UnwrappedLineParser::parseLoopBody(bool KeepBraces, bool WrapRightBrace) {
   keepAncestorBraces();
 
   if (Keywords.isBlockBegin(*FormatTok, Style)) {
     if (!KeepBraces)
       FormatTok->setFinalizedType(TT_ControlStatementLBrace);
     FormatToken *LeftBrace = FormatTok;
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
     parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
-               /*MunchSemi=*/true, KeepBraces);
+               /*MunchSemi=*/true, &KeepBraces);
     if (!KeepBraces) {
       assert(!NestedTooDeep.empty());
       if (!NestedTooDeep.back())
-        markOptionalBraces(LeftBrace);
+        markOptionalBraces(LeftBrace, true);
     }
     if (WrapRightBrace)
       addUnwrappedLine();
   } else {
     parseUnbracedBody();
   }
 
   if (!KeepBraces)