[PATCH] Recovery of switch statement

[Serge Pavlov]

  Thank you for review.


================
Comment at: lib/Parse/ParseStmt.cpp:701-705
@@ -688,5 +700,7 @@
     // not valid.
     SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
+    if (ColonLoc.isInvalid())
+      AfterColonLoc = PrevTokLocation;
     Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
       << FixItHint::CreateInsertion(AfterColonLoc, " ;");
     SubStmt = true;
----------------
Richard Smith wrote:
> This doesn't look right in the case where `ColonLoc` is invalid. `AfterColonLoc` will point to the *start* of the previous token in this case, and we'll provide a fix-it inserting a semicolon in a fairly random place.
> 
> In the case where `ColonLoc` is invalid, we've already hit at least one error, so it seems better to suppress the follow-on diagnostic here.
> 
> I think this will also do the wrong thing if we have a sequence of `case` statements, and the last one is missing a colon or is otherwise malformed; we'll use the last `ColonLoc` for any of the `case` statements we succeeded in handling. Maybe reset `ColonLoc` to `SourceLocation()` each time around the `do` loop to fix this?
Yes, this is complicated point, thank you for the detailed explanation. It looks like there is no reliable way to determine location for error message if ColonLoc is undefined due to previous error. Suppressing follow-on diagnostic gives better results.

Resetting ColonLoc at the beginning of case label is also necessary.

================
Comment at: lib/Sema/SemaStmt.cpp:707-708
@@ +706,4 @@
+  if (!Body) {
+    // Put the synthesized null statement on the same line as the end of switch
+    // condition.
+    SourceLocation SynthesizedNullStmtLocation;
----------------
Richard Smith wrote:
> This comment doesn't explain what's happening here. Something like:
> 
>   // This happens if the body was ill-formed. Synthesize a null statement at the end of the switch condition.
> 
> ... would be better. But our normal approach here would be to simply return `StmtError()` in this case. Any reason not to do that?
It looks like preparing bogus statement body is not necessary, StmtError is enough.


http://reviews.llvm.org/D3137