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

[Owen Pan via Phabricator via cfe-commits]

owenpan marked an inline comment as done.
owenpan added inline comments.


================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:486
       return 0;
+    if (Line.First->is(tok::kw_default)) {
+      const FormatToken *Tok = Line.First->getNextNonComment();
----------------
sammccall wrote:
> 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.
Exactly!


================
Comment at: unittests/Format/FormatTest.cpp:1012
+                   "{\n"
+                   "case 0: {\n"
+                   "  return false;\n"
----------------
sammccall wrote:
> the intent of this test might be clearer if the cases were formatted as `case 0: { return false; }` on one line
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.


Repository:
  rC Clang

https://reviews.llvm.org/D51294