[PATCH] Improve error recovery around colon.

[Richard Smith]

================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:65-66
@@ -64,2 +64,4 @@
   InGroup<KeywordCompat>;
+def note_declared_here : Note<"%0 declared here">;
+
 
----------------
We already have this: `diag::note_declared_at`.

================
Comment at: lib/Parse/ParseExprCXX.cpp:459
@@ +458,3 @@
+        NamedDecl *IdDecl = 0;
+        if (ColonIsSacred && !ObjectType &&
+            Actions.IsValidIfFollowedByColon(getCurScope(), SS, II, IdLoc,
----------------
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.

================
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)
----------------
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`.)

================
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 {
----------------
Please fold these together:

    } else if (TryConsumeToken(tok::semi, ColonLoc) ||
               TryConsumeToken(tok::coloncolon, ColonLoc)) {

================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:509
@@ +508,3 @@
+    LookupResult R(*this, Found.getLookupNameInfo(), LookupOrdinaryName);
+    if (SS.isSet() && LookupCtx)
+      LookupQualifiedName(R, LookupCtx);
----------------
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).


http://llvm-reviews.chandlerc.com/D2870