clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

[DaanDeMeyer]

Summary

do
  a++;
while (true);

becomes

do a++;
while (true);

if AllowShortLoopsOnASingleLine is enabled.

One of the projects where I want to introduce clang-format formats single-line statements following a do on the same line as the do statement. This is one of the blockers in introducing clang-format for this project. I'm not sure if it's fine to put this behind an existing option or if a new option (AllowShortDoWhileStatementsOnASingleLine) should be introduced for this.

This is my first contribution to clang-format so apologies if I missed anything obvious. Any comments or help with any further steps to get this merged would be appreciated.

[MyDeveloperDay]

You need to add unit tests

[DaanDeMeyer]

Added unit tests and fixed the case where stuff follows the do statement (like a comment).

[MyDeveloperDay]

I think its look good, please add the extra tests, then lets give people a couple of days

unittests/Format/FormatTest.cpp

570 ↗

(On Diff #246244)

horrible code though they are, could you add a test for the label and comment case

do
label:
a++
white(true);

and

do
// comment
a++
white(true);

[DaanDeMeyer]

Added extra unit tests

[mitchell-stellar]

Assuming this passes all existing tests, LGTM.

[DaanDeMeyer]

Is there CI infra that runs for each revision? I verified all the format unit tests still pass but I haven't run the entire test suite on my machine.

[mitchell-stellar]

Not that I am aware of. Whoever ends up doing the merge will likely run the necessary tests before committing. If you've run as many as you can, then hopefully all will be fine.

[MyDeveloperDay]

LGTM, thank you for adding the extra test, please mark the inline comments as done

clang/lib/Format/UnwrappedLineFormatter.cpp

if (I[1]->First->is(TT_FunctionLBrace) &&
       }
       return MergedLines;
     }
     if (TheLine->First->is(tok::kw_if)) {
       return Style.AllowShortIfStatementsOnASingleLine
                  ? tryMergeSimpleControlStatement(I, E, Limit)
                  : 0;
     }
-    if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+    if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
       return Style.AllowShortLoopsOnASingleLine
                  ? tryMergeSimpleControlStatement(I, E, Limit)
                  : 0;
     }
     if (TheLine->First->isOneOf(tok::kw_case, tok::kw_default)) {
       return Style.AllowShortCaseLabelsOnASingleLine
                  ? tryMergeShortCaseLabels(I, E, Limit)
                  : 0;
if (Style.BraceWrapping.AfterControlStatement ==
         I[1]->First->is(tok::l_brace) &&
         Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
       return 0;
     if (I[1]->InPPDirective != (*I)->InPPDirective ||
         (I[1]->InPPDirective && I[1]->First->HasUnescapedNewline))
       return 0;
     Limit = limitConsideringMacros(I + 1, E, Limit);
     AnnotatedLine &Line = **I;
-    if (Line.Last->isNot(tok::r_paren))
+    if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+      return 0;
+    // Only merge do while if do is the only statement on the line.
+    if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
       return 0;
     if (1 + I[1]->Last->TotalLength > Limit)
       return 0;
     if (I[1]->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for, tok::kw_while,
                              TT_LineComment))
       return 0;
     // Only inline simple if's (no nested if or else), unless specified
     if (Style.AllowShortIfStatementsOnASingleLine != FormatStyle::SIS_Always) {

clang/unittests/Format/FormatTest.cpp

verifyFormat("for (;;)\n"
                "  for (;;) continue;",
                AllowsMergedLoops);
   verifyFormat("for (;;) // Can't merge this\n"
                "  continue;",
                AllowsMergedLoops);
   verifyFormat("for (;;) /* still don't merge */\n"
                "  continue;",
                AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+               "  a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+               "  a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  verifyFormat("do\n"
+               "  // Don't merge\n"
+               "  a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+               "  do\n"
+               "  label:\n"
+               "    a++;\n"
+               "  while (true);\n"
+               "}",
+               AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
   AllowSimpleBracedStatements.ColumnLimit = 40;
   AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine =
       FormatStyle::SBS_Always;