Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

[owenpan]

Summary

See https://bugs.llvm.org/show_bug.cgi?id=38713

[sammccall]

Thanks!

lib/Format/UnwrappedLineFormatter.cpp

486 ↗

(On Diff #162646)

Just to check my understanding...
we want to treat default the same as case, but the heuristics are different:

• case should only appear in a switch (but might be followed by a complex expression)
• default has lots of meanings (but we can disambiguate: check if it's followed by a colon)

You could consider // default: in switch statement above this line.

unittests/Format/FormatTest.cpp

1012 ↗

(On Diff #162646)

the intent of this test might be clearer if the cases were formatted as case 0: { return false; } on one line

[owenpan]

lib/Format/UnwrappedLineFormatter.cpp

486 ↗

(On Diff #162646)

Exactly!

unittests/Format/FormatTest.cpp

1012 ↗

(On Diff #162646)

I used the same test case that exposed this bug. Please see it in the bug report.

Because of the bug, "case" and "default" case labels were handled differently. The former was correctly left alone, but the latter was merged into one line. Therefore, the test case checks that neither is merged.

[owenpan]

Added a comment suggested by @sammccall

[JonasToth]

Commited for Owen in r341284.

cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp

if (Style.Language != FormatStyle::LK_Java &&
         Line.First->isOneOf(tok::at, tok::minus, tok::plus))
       return 0;
 
     // Check that the current line allows merging. This depends on whether we
     // are in a control flow statements as well as several style flags.
     if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
         (Line.First->Next && Line.First->Next->is(tok::kw_else)))
       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_while, tok::kw_do, tok::kw_try,
                             tok::kw___try, tok::kw_catch, tok::kw___finally,
                             tok::kw_for, tok::r_brace, Keywords.kw___except)) {
       if (!Style.AllowShortBlocksOnASingleLine)
         return 0;
       // Don't merge when we can't except the case when
       // the control statement block is empty
       if (!Style.AllowShortIfStatementsOnASingleLine &&

cfe/trunk/unittests/Format/FormatTest.cpp

EXPECT_EQ("DEBUG({\n"
                    "    break;\n"
                    "  // On B:\n"
                    "  case B:\n"
                    "    g();\n"
                    "    break;\n"
                    "  }\n"
                    "});",
                    getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+            "case 0: {\n"
+            "  return false;\n"
+            "}\n"
+            "default: {\n"
+            "  return true;\n"
+            "}\n"
+            "}",
+            format("switch (n)\n"
+                   "{\n"
+                   "case 0: {\n"
+                   "  return false;\n"
+                   "}\n"
+                   "default: {\n"
+                   "  return true;\n"
+                   "}\n"
+                   "}",
+                   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
                "case (b):\n"
                "  return;\n"
                "}");
 
   verifyFormat("switch (a) {\n"
                "case some_namespace::\n"
                "    some_constant:\n"