[clang-format] Link the braces of a block in UnwrappedLineParser

[owenpan]

Summary

This includes TT_MacroBlockBegin and TT_MacroBlockEnd as well.

We can no longer use MatchingParen of FormatToken as an indicator to mark optional braces. Instead, we directly set Optional of an l_brace first and reset it later if it turns out that the braces are not optional.

Also added a test case for deeply nested loops.

[HazardyKnusperkeks]

clang/lib/Format/UnwrappedLineParser.cpp

2714

This should be named differently. For me reset means assigning false.

[owenpan]

Updated resetOptional() to make resetting Optional more obvious.

[owenpan]

I have second thoughts about linking block braces in UnwrappedLineParser. Consider the following example:

#if A
while (a) { // Linked to the r_brace below
#else
while(b) { // Also linked to the r_brace below
#endif
  foo();
}  // Linked to the l_brace of while(b) above

The l_brace of while (a) is still linked to the r_brace that has been re-linked to another l_bace.

Because MatchingParen will be reset in the annotator anyway, linking it for block braces in UnwrappedLineParser has little or no value. I will revert this patch if no one objects.

[owenpan]

See https://github.com/llvm/llvm-project/issues/59417.

[HazardyKnusperkeks]

In D139257#3983332, @owenpan wrote:

I have second thoughts about linking block braces in UnwrappedLineParser. Consider the following example:

#if A
while (a) { // Linked to the r_brace below
#else
while(b) { // Also linked to the r_brace below
#endif
  foo();
}  // Linked to the l_brace of while(b) above

The l_brace of while (a) is still linked to the r_brace that has been re-linked to another l_bace.

Because MatchingParen will be reset in the annotator anyway, linking it for block braces in UnwrappedLineParser has little or no value. I will revert this patch if no one objects.

I don't like it, but that seems to be the reasonable approach.

clang/lib/Format/UnwrappedLineParser.cpp

FormatToken *UnwrappedLineParser::parseBlock(
 
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
                  : !FormatTok->is(tok::r_brace)) {
     Line->Level = InitialLevel;
     FormatTok->setBlockKind(BK_Block);
     return IfLBrace;
   }
 
+  Tok->MatchingParen = FormatTok;
+  FormatTok->MatchingParen = Tok;
+
   const bool IsFunctionRBrace =
       FormatTok->is(tok::r_brace) && Tok->is(TT_FunctionLBrace);
 
   auto RemoveBraces = [=]() mutable {
     if (!SimpleBlock)
       return false;
     assert(Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace));
     assert(FormatTok->is(tok::r_brace));
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;
-  }
+  Tok->Optional = RemoveBraces();
 
   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
static void markOptionalBraces(FormatToken *LeftBrace) {
   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;
+  RightBrace->Optional = LeftBrace->Optional;
+}
+
+static void resetOptional(FormatToken *LBrace) {
+  if (!LBrace)
+    return;
+
+  const auto *RBrace = LBrace->MatchingParen;
+  LBrace->Optional = RBrace && RBrace->Optional;
 }
 
 void UnwrappedLineParser::handleAttributes() {
   // Handle AttributeMacro, e.g. `if (x) UNLIKELY`.
   if (FormatTok->is(TT_AttributeMacro))
     nextToken();
   handleCppAttributes();
 }
if (Keywords.isBlockBegin(*FormatTok, Style)) {
     else
       NeedsUnwrappedLine = true;
   } else {
     parseUnbracedBody();
   }
 
   if (Style.RemoveBracesLLVM) {
     assert(!NestedTooDeep.empty());
-    KeepIfBraces = KeepIfBraces ||
-                   (IfLeftBrace && !IfLeftBrace->MatchingParen) ||
+    KeepIfBraces = KeepIfBraces || (IfLeftBrace && !IfLeftBrace->Optional) ||
                    NestedTooDeep.back() || IfBlockKind == IfStmtKind::IfOnly ||
                    IfBlockKind == IfStmtKind::IfElseIf;
   }
 
   bool KeepElseBraces = KeepIfBraces;
   FormatToken *ElseLeftBrace = nullptr;
   IfStmtKind Kind = IfStmtKind::IfOnly;
 
if (Keywords.isBlockBegin(*FormatTok, Style)) {
           parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
                      /*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;
+        assert(ElseLeftBrace->Optional);
         assert(ElseLeftBrace->MatchingParen);
-        markOptionalBraces(ElseLeftBrace);
+        ElseLeftBrace->MatchingParen->Optional = true;
       }
       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();
if (NeedsUnwrappedLine)
       addUnwrappedLine();
   }
 
   if (!Style.RemoveBracesLLVM)
     return nullptr;
 
   assert(!NestedTooDeep.empty());
   KeepElseBraces = KeepElseBraces ||
-                   (ElseLeftBrace && !ElseLeftBrace->MatchingParen) ||
+                   (ElseLeftBrace && !ElseLeftBrace->Optional) ||
                    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;
-    }
   }
 
+  resetOptional(IfLeftBrace);
+  resetOptional(ElseLeftBrace);
+
   if (IfKind)
     *IfKind = Kind;
 
   return IfLeftBrace;
 }
 
 void UnwrappedLineParser::parseTryCatch() {
   assert(FormatTok->isOneOf(tok::kw_try, tok::kw___try) && "'try' expected");
if (Keywords.isBlockBegin(*FormatTok, Style)) {
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
     parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
                /*MunchSemi=*/true, KeepBraces);
     if (!KeepBraces) {
       assert(!NestedTooDeep.empty());
       if (!NestedTooDeep.back())
         markOptionalBraces(LeftBrace);
     }
+    resetOptional(LeftBrace);
     if (WrapRightBrace)
       addUnwrappedLine();
   } else {
     parseUnbracedBody();
   }
 
   if (!KeepBraces)
     NestedTooDeep.pop_back();

clang/unittests/Format/BracesRemoverTest.cpp

verifyFormat("// clang-format off\n"
                "  ++j;",
                "// clang-format off\n"
                "// comment\n"
                "while (i > 0) { --i; }\n"
                "// clang-format on\n"
                "while (j < 0) { ++j; }",
                Style);
 
+  verifyFormat("for (;;) {\n"
+               "  for (;;)\n"
+               "    for (;;)\n"
+               "      a;\n"
+               "}",
+               "for (;;) {\n"
+               "  for (;;) {\n"
+               "    for (;;) {\n"
+               "      a;\n"
+               "    }\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"