[PATCH] Improve error recovery around colon.
Serge Pavlov
sepavloff at gmail.com
Sun Mar 16 10:12:10 PDT 2014
================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:65-66
@@ -64,2 +64,4 @@
InGroup<KeywordCompat>;
+def note_declared_here : Note<"%0 declared here">;
+
----------------
Richard Smith wrote:
> We already have this: `diag::note_declared_at`.
Overlooked it. Thank you for the hint.
================
Comment at: lib/Parse/ParseExprCXX.cpp:459
@@ +458,3 @@
+ NamedDecl *IdDecl = 0;
+ if (ColonIsSacred && !ObjectType &&
+ Actions.IsValidIfFollowedByColon(getCurScope(), SS, II, IdLoc,
----------------
Richard Smith wrote:
> Why do you exclude the `ObjectType` case here? If we have:
>
> struct S { int n; } s;
>
> int k = true ? s.n :: 0;
>
> ... we should issue the correction.
Fixed.
================
Comment at: lib/Parse/ParseExprCXX.cpp:460-461
@@ +459,4 @@
+ if (ColonIsSacred && !ObjectType &&
+ Actions.IsValidIfFollowedByColon(getCurScope(), SS, II, IdLoc,
+ CCLoc, &IdDecl)) {
+ Diag(CCLoc, diag::err_nested_name_spec_is_not_class)
----------------
Richard Smith wrote:
> I don't like coupling this check to the check in `ActOnCXXNestedNameSpecifier`. If these don't match *exactly*, we can fail to emit a diagnostic for the failed lookup here.
>
> Can you instead pass in a `bool *`, have `ActOnCXXNestedNameSpecifier` set it if finds the nested name specifier names something that's not valid on the LHS of a `::`, and check that flag here? (And remove `IsValidIfFollowedByColon`.)
IsValidIfFollowedByColon is removed, declaration of ActOnCXXNestedNameSpecifier changed accordingly.
================
Comment at: lib/Parse/ParseStmt.cpp:642-650
@@ -641,7 +641,11 @@
if (TryConsumeToken(tok::colon, ColonLoc)) {
} else if (TryConsumeToken(tok::semi, ColonLoc)) {
// Treat "case blah;" as a typo for "case blah:".
Diag(ColonLoc, diag::err_expected_after)
<< "'case'" << tok::colon
<< FixItHint::CreateReplacement(ColonLoc, ":");
+ } else if (TryConsumeToken(tok::coloncolon, ColonLoc)) {
+ Diag(ColonLoc, diag::err_expected_after)
+ << "'case'" << tok::colon
+ << FixItHint::CreateReplacement(ColonLoc, ":");
} else {
----------------
Richard Smith wrote:
> Please fold these together:
>
> } else if (TryConsumeToken(tok::semi, ColonLoc) ||
> TryConsumeToken(tok::coloncolon, ColonLoc)) {
Done.
================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:509
@@ +508,3 @@
+ LookupResult R(*this, Found.getLookupNameInfo(), LookupOrdinaryName);
+ if (SS.isSet() && LookupCtx)
+ LookupQualifiedName(R, LookupCtx);
----------------
Richard Smith wrote:
> I think we should do this if `SS` is not set too (that is, if we have an `ObjectType`, we should perform qualified lookup into it, rather than unqualified lookup).
Fixed.
http://llvm-reviews.chandlerc.com/D2870
More information about the cfe-commits
mailing list