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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to