[PATCH] Recovery of switch statement
Richard Smith
richard at metafoo.co.uk
Fri Apr 11 13:52:02 PDT 2014
================
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;
----------------
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?
================
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;
----------------
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?
http://reviews.llvm.org/D3137
More information about the cfe-commits
mailing list