[PATCH] Improve error recovery around colon.

[Richard Smith]

================
Comment at: include/clang/Parse/Parser.h:172-181
@@ -171,2 +171,12 @@
 
-  /// \brief When true, we are directly inside an Objective-C messsage
+  /// \brief Enumerates parser states in which error recovery is modified.
+  ///
+  enum RecoveryContextType {
+    ctxGeneric,   /// no special information
+    ctxCaseStmt   /// case statement
+  };
+
+  /// \brief Keeps information about language construct being parsed, to assist
+  /// error recovery.
+  RecoveryContextType RecoveryContext;
+
----------------
Do we really need this? I would think looking at `ColonIsSacred` should be enough. That flag is also set for range-based for loops, bit-fields, class base-specifiers, enum underlying type specifiers, and ?:, and in each of these cases we want this behavior.

================
Comment at: lib/Parse/ParseExprCXX.cpp:458
@@ +457,3 @@
+          TPA.Revert();
+          return false;
+        }
----------------
Should this be a 'break' rather than a 'return'? We should presumably still annotate the prior tokens as a scope specifier.

================
Comment at: test/Parser/recovery.cpp:152-153
@@ +151,4 @@
+    case 0: break;
+    case ns::V1:: break; // expected-error{{'V1' is not a class, namespace, or scoped enumeration}} \
+                         // expected-error{{expected ':' after 'case'}}
+    case C1::V2:: break; // expected-error{{'V2' is not a class, namespace, or scoped enumeration}} \
----------------
These diagnostics could be better. Ideally we'd only issue one error, and it would say something like

  'V1' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?

... maybe with a note pointing to the declaration of 'V1'.

================
Comment at: test/Parser/recovery.cpp:160
@@ +159,3 @@
+                     // expected-error{{expected ':' after 'case'}}
+    }
+  }
----------------
You don't have any tests for this case:

  case V1::  func_1(0);

... which might go down a different path in the parser because it's got an identifier after the '::'.

================
Comment at: test/SemaCXX/nested-name-spec.cpp:16
@@ -15,4 +15,3 @@
 A:: ; // expected-error {{expected unqualified-id}}
-// FIXME: there is a member 'ax'; it's just not a class.
-::A::ax::undef ex3; // expected-error {{no member named 'ax'}}
+::A::ax::undef ex3; // expected-error {{'ax' is not a class, namespace, or scoped enumeration}}
 A::undef1::undef2 ex4; // expected-error {{no member named 'undef1'}}
----------------
Can you split this change out into a separate patch? Also, it'd be great to fix the diagnostic as described above to more clearly explain the problem:

  'ax' cannot appear before '::' because it is not a class, namespace, or scoped enumeration

... with a note pointing to the declaration of 'ax'.


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