[clang-format] Correctly format loops and `if` statements even if preceded with comments.

[curdeius]

Summary

Fixes https://github.com/llvm/llvm-project/issues/53758.

Braces in loops and in if statements with leading (block) comments were formatted according to BraceWrapping.AfterFunction and not AllowShortBlocksOnASingleLine/AllowShortLoopsOnASingleLine/AllowShortIfStatementsOnASingleLine.

Previously, the code:

while (true) {
  f();
}
/*comment*/ while (true) {
  f();
}

was incorrectly formatted to:

while (true) {
  f();
}
/*comment*/ while (true) { f(); }

when using config:

BasedOnStyle: LLVM
BreakBeforeBraces: Custom
BraceWrapping:
  AfterFunction: false
AllowShortBlocksOnASingleLine: false
AllowShortLoopsOnASingleLine: false

and it was (correctly but by chance) formatted to:

while (true) {
  f();
}
/*comment*/ while (true) {
  f();
}

when using enabling brace wrapping after functions:

BasedOnStyle: LLVM
BreakBeforeBraces: Custom
BraceWrapping:
  AfterFunction: true
AllowShortBlocksOnASingleLine: false
AllowShortLoopsOnASingleLine: false

[curdeius]

Test ifs and FOREACH loops.

[sstwcw]

clang/lib/Format/UnwrappedLineFormatter.cpp

379

Why not use FirstNonComment instead of TheLine->First down here?

[curdeius]

clang/lib/Format/UnwrappedLineFormatter.cpp

379

Because I haven't added a test case for it and prefer doing it in a different revision (and because as you see, I already got confused enough :) ).

clang/lib/Format/UnwrappedLineFormatter.cpp

auto ShouldMergeShortFunctions = [this, &I, &NextLine, PreviousLine,
         }
       }
 
       return false;
     };
 
     bool MergeShortFunctions = ShouldMergeShortFunctions();
 
+    const FormatToken *FirstNonComment = TheLine->First;
+    if (FirstNonComment->is(tok::comment)) {
+      FirstNonComment = FirstNonComment->getNextNonComment();
+      if (!FirstNonComment)
+        return 0;
+    }
+    // FIXME: There are probably cases where we should use FirstNonComment
+    // instead of TheLine->First.
+
     if (Style.CompactNamespaces) {
       if (auto nsToken = TheLine->First->getNamespaceToken()) {
         int i = 0;
         unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
         for (; I + 1 + i != E &&
                nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
                closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
                I[i + 1]->Last->TotalLength < Limit;
if (Style.CompactNamespaces) {
         return i;
       }
     }
 
     // Try to merge a function block with left brace unwrapped.
     if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First != TheLine->Last)
       return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
     // Try to merge a control statement block with left brace unwrapped.
-    if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
-        TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
+    if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
+        FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
                                  TT_ForEachMacro)) {
       return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
                  ? tryMergeSimpleBlock(I, E, Limit)
                  : 0;
     }
     // Try to merge a control statement block with left brace wrapped.
     if (NextLine.First->is(tok::l_brace)) {
       if ((TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
                                    tok::kw_for, tok::kw_switch, tok::kw_try,
                                    tok::kw_do, TT_ForEachMacro) ||
            (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
             TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
           Style.BraceWrapping.AfterControlStatement ==
               FormatStyle::BWACS_MultiLine) {
         // If possible, merge the next line's wrapped left brace with the
         // current line. Otherwise, leave it on the next line, as this is a

clang/unittests/Format/FormatTest.cpp

verifyFormat("{\n"
                "    a++;\n"
                "  while (true);\n"
                "}",
                AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine,
+            FormatStyle::SIS_Never);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+  verifyFormat("for (;;) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("/*comment*/ for (;;) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("BOOST_FOREACH (int v, vec) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("/*comment*/ BOOST_FOREACH (int v, vec) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("while (true) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("/*comment*/ while (true) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("if (true) {\n"
+               "  f();\n"
+               "}");
+  verifyFormat("/*comment*/ if (true) {\n"
+               "  f();\n"
+               "}");
+
   AllowSimpleBracedStatements.IfMacros.push_back("MYIF");
   // Where line-lengths matter, a 2-letter synonym that maintains line length.
   // Not IF to avoid any confusion that IF is somehow special.
   AllowSimpleBracedStatements.IfMacros.push_back("FI");
   AllowSimpleBracedStatements.ColumnLimit = 40;
   AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine =
       FormatStyle::SBS_Always;