[clang-format] Fix a bug in merging blocks with a wrapped l_brace

[owenpan]

Summary

When the opening brace of a control statement block is wrapped, we must check the previous line to determine whether to try to merge the block.

Fixes https://github.com/llvm/llvm-project/issues/38639.
Fixes https://github.com/llvm/llvm-project/issues/48007.
Fixes https://github.com/llvm/llvm-project/issues/57421.

[HazardyKnusperkeks]

clang/lib/Format/UnwrappedLineFormatter.cpp

716

switch?

[owenpan]

clang/lib/Format/UnwrappedLineFormatter.cpp

716

It wasn't in the list, and I don't think it should be. We can add it in a separate patch if the need for merging switch statements ever arises.

clang/lib/Format/UnwrappedLineFormatter.cpp

if (Line.First->is(tok::kw_case) ||
       return 0;
     }
     // default: in switch statement
     if (Line.First->is(tok::kw_default)) {
       const FormatToken *Tok = Line.First->getNextNonComment();
       if (Tok && Tok->is(tok::colon))
         return 0;
     }
-    if (Line.First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while, tok::kw_do,
-                            tok::kw_try, tok::kw___try, tok::kw_catch,
-                            tok::kw___finally, tok::kw_for, TT_ForEachMacro,
-                            tok::r_brace, Keywords.kw___except)) {
-      if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
-        return 0;
-      if (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Empty &&
-          !I[1]->First->is(tok::r_brace)) {
+
+    auto IsCtrlStmt = [](const auto &Line) {
+      return Line.First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
+                                 tok::kw_do, tok::kw_for, TT_ForEachMacro);
+    };
+
+    const bool IsSplitBlock =
+        Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never ||
+        (Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Empty &&
+         I[1]->First->isNot(tok::r_brace));
+
+    if (IsCtrlStmt(Line) ||
+        Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch,
+                            tok::kw___finally, tok::r_brace,
+                            Keywords.kw___except)) {
+      if (IsSplitBlock)
         return 0;
-      }
       // Don't merge when we can't except the case when
       // the control statement block is empty
       if (!Style.AllowShortIfStatementsOnASingleLine &&
           Line.First->isOneOf(tok::kw_if, tok::kw_else) &&
           !Style.BraceWrapping.AfterControlStatement &&
           !I[1]->First->is(tok::r_brace)) {
         return 0;
       }
if (IsCtrlStmt(Line) ||
       // is '}', so this isn't correct here.
       if (Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch,
                               Keywords.kw___except, tok::kw___finally)) {
         return 0;
       }
     }
 
     if (Line.Last->is(tok::l_brace)) {
+      if (IsSplitBlock && Line.First == Line.Last &&
+          I > AnnotatedLines.begin() &&
+          (I[-1]->endsWith(tok::kw_else) || IsCtrlStmt(*I[-1]))) {
+        return 0;
+      }
       FormatToken *Tok = I[1]->First;
       auto ShouldMerge = [Tok]() {
         if (Tok->isNot(tok::r_brace) || Tok->MustBreakBefore)
           return false;
         const FormatToken *Next = Tok->getNextNonComment();
         return !Next || Next->is(tok::semi);
       };
 

clang/unittests/Format/FormatTest.cpp

verifyFormat("for (;;)\n"
                "}",
                AllowSimpleBracedStatements);
   verifyFormat("BOOST_FOREACH (int v, vec) {}", AllowSimpleBracedStatements);
   verifyFormat("BOOST_FOREACH (int v, vec)\n"
                "{\n"
                "  f();\n"
                "}",
                AllowSimpleBracedStatements);
+
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
+
+  verifyFormat("while (i > 0)\n"
+               "{\n"
+               "  --i;\n"
+               "}",
+               Style);
+
+  verifyFormat("if (a)\n"
+               "{\n"
+               "  ++b;\n"
+               "}",
+               Style);
+
+  verifyFormat("if (a)\n"
+               "{\n"
+               "  b = 1;\n"
+               "} else\n"
+               "{\n"
+               "  b = 0;\n"
+               "}",
+               Style);
+
+  verifyFormat("if (a)\n"
+               "{\n"
+               "  b = 1;\n"
+               "} else if (c)\n"
+               "{\n"
+               "  b = 2;\n"
+               "} else\n"
+               "{\n"
+               "  b = 0;\n"
+               "}",
+               Style);
+
+  Style.BraceWrapping.BeforeElse = true;
+
+  verifyFormat("if (a)\n"
+               "{\n"
+               "  b = 1;\n"
+               "}\n"
+               "else\n"
+               "{\n"
+               "  b = 0;\n"
+               "}",
+               Style);
+
+  verifyFormat("if (a)\n"
+               "{\n"
+               "  b = 1;\n"
+               "}\n"
+               "else if (c)\n"
+               "{\n"
+               "  b = 2;\n"
+               "}\n"
+               "else\n"
+               "{\n"
+               "  b = 0;\n"
+               "}",
+               Style);
 }
 
 TEST_F(FormatTest, UnderstandsMacros) {
   verifyFormat("#define A (parentheses)");
   verifyFormat("/* comment */ #define A (parentheses)");
   verifyFormat("/* comment */ /* another comment */ #define A (parentheses)");
   // Even the partial code should never be merged.
   EXPECT_EQ("/* comment */ #define A (parentheses)\n"
verifyFormat("if (a + b >\n"
                "  f();\n"
                "}",
                "if (a + b > c)\n"
                "  f();",
                Style);
 
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
-  // TODO: Delete the following line after #57421 is fixed.
-  Style.BraceWrapping.AfterFunction = true;
 
   verifyFormat("if (a) //\n"
                "{\n"
                "  b = 1;\n"
                "}",
                "if (a) //\n"
                "  b = 1;",
                Style);