On Tue, Jun 14, 2011 at 10:41 AM, Douglas Gregor <[email protected]> wrote: > > On Jun 12, 2011, at 10:50 PM, David Majnemer wrote: > >> Author: majnemer >> Date: Mon Jun 13 00:50:12 2011 >> New Revision: 132903 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=132903&view=rev >> Log: >> Improve the diagnostics generated for switch statements missing expressions >> >> - Move the diagnostic to the case statement instead of at the end of the >> switch >> - Add a fix-it hint as to how to fix the compilation error > > I don't consider this a proper use of Fix-Its. If we encounter code like > > switch (z) { > case 4: > } > > it's almost surely the case that the user got distracted and forgot to write > the code for that case, and it's highly unlikely that > > ; > > is the right code for this case. Moreover, placing the ';' right after the > ':' like this: > > switch (z) { > case 4:; > } > > is going to hide the issue, especially if we're in a system that > automatically applies Fix-Its. I removed the Fix-it in r132994. > > Please revert this patch. I'm sorry that the FIXME there was misleading, but > the FIXME is wrong and we shouldn't be suggesting any Fix-Its here because > there isn't a clearly correct fix. Per discussion on #llvm, I kept the bulk of the patch as it does make the original diagnostic more clear. > > - Doug > >> Modified: >> cfe/trunk/lib/Parse/ParseStmt.cpp >> cfe/trunk/test/Parser/switch-recovery.cpp >> >> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=132903&r1=132902&r2=132903&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original) >> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Mon Jun 13 00:50:12 2011 >> @@ -502,6 +502,7 @@ >> StmtTy *DeepestParsedCaseStmt = 0; >> >> // While we have case statements, eat and stack them. >> + SourceLocation ColonLoc; >> do { >> SourceLocation CaseLoc = MissingCase ? Expr.get()->getExprLoc() : >> ConsumeToken(); // eat the >> 'case'. >> @@ -539,7 +540,6 @@ >> >> ColonProtection.restore(); >> >> - SourceLocation ColonLoc; >> if (Tok.is(tok::colon)) { >> ColonLoc = ConsumeToken(); >> >> @@ -589,8 +589,9 @@ >> } else { >> // Nicely diagnose the common error "switch (X) { case 4: }", which is >> // not valid. >> - // FIXME: add insertion hint. >> - Diag(Tok, diag::err_label_end_of_compound_statement); >> + SourceLocation ExpectedLoc = PP.getLocForEndOfToken(ColonLoc); >> + Diag(ExpectedLoc, diag::err_label_end_of_compound_statement) >> + << FixItHint::CreateInsertion(ExpectedLoc, ";"); >> SubStmt = true; >> } >> >> @@ -634,7 +635,9 @@ >> >> // Diagnose the common error "switch (X) {... default: }", which is not >> valid. >> if (Tok.is(tok::r_brace)) { >> - Diag(Tok, diag::err_label_end_of_compound_statement); >> + SourceLocation ExpectedLoc = PP.getLocForEndOfToken(ColonLoc); >> + Diag(ExpectedLoc, diag::err_label_end_of_compound_statement) >> + << FixItHint::CreateInsertion(ExpectedLoc, ";"); >> return StmtError(); >> } >> >> >> Modified: cfe/trunk/test/Parser/switch-recovery.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/switch-recovery.cpp?rev=132903&r1=132902&r2=132903&view=diff >> ============================================================================== >> --- cfe/trunk/test/Parser/switch-recovery.cpp (original) >> +++ cfe/trunk/test/Parser/switch-recovery.cpp Mon Jun 13 00:50:12 2011 >> @@ -156,3 +156,17 @@ >> } >> } >> } >> + >> +void missing_statement_case(int x) { >> + switch (x) { >> + case 1: >> + case 0: // expected-error {{label at end of compound statement: >> expected statement}} >> + } >> +} >> + >> +void missing_statement_default(int x) { >> + switch (x) { >> + case 0: >> + default: // expected-error {{label at end of compound statement: >> expected statement}} >> + } >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
