[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