[PATCH] Improve error recovery around colon.

Serge Pavlov sepavloff at gmail.com
Wed Mar 5 20:21:52 PST 2014



================
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;
+
----------------
Richard Smith wrote:
> 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.
Remade patch excluding this context info.

================
Comment at: lib/Parse/ParseExprCXX.cpp:458
@@ +457,3 @@
+          TPA.Revert();
+          return false;
+        }
----------------
Richard Smith wrote:
> Should this be a 'break' rather than a 'return'? We should presumably still annotate the prior tokens as a scope specifier.
This function doesn't do much after exit from loop, but using 'break' is better of course.

================
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}} \
----------------
Richard Smith wrote:
> 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'.
Implemented in this way.

================
Comment at: test/Parser/recovery.cpp:160
@@ +159,3 @@
+                     // expected-error{{expected ':' after 'case'}}
+    }
+  }
----------------
Richard Smith wrote:
> 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 '::'.
Indeed, this case didn't handled properly.


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



More information about the cfe-commits mailing list