[PATCH] Improve error recovery around colon.

[Serge Pavlov]

================
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