[PATCH] D44449: [Parser] Fix assertion-on-invalid for unexpected typename.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, seems reasonable to me.



================
Comment at: clang/lib/Parse/ParseDecl.cpp:3076
         DS.SetRangeEnd(Tok.getAnnotationEndLoc());
         ConsumeAnnotationToken(); // The typename
       }
----------------
vsapsai wrote:
> Here we potentially can leave annotation token unconsumed. But I wasn't able to create a test case that would trigger a problem at this point.
I think you're talking about the `break` a few lines back?

I think we actually get away with this, but only because `SetTypeSpecType` can only fail if there was already a type specifier, which we checked for on line 3019, so `isInvalid` is never `true`.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:3148
       DS.SetRangeEnd(Tok.getAnnotationEndLoc());
       ConsumeAnnotationToken(); // The typename
 
----------------
vsapsai wrote:
> We didn't consume the annotation token because of `break` on `isInvalid` a few lines above.
I wonder if our error recovery would be improved by changing line 3134 to just

```if (DS.hasTypeSpecifier())```

(which would likewise render the `break` here unreachable given the current rules enforced by `SetTypeSpecType`).


================
Comment at: clang/lib/Parse/ParseDecl.cpp:3802
     if (DiagID != diag::err_bool_redeclaration)
-      ConsumeToken();
+      ConsumeAnyToken();
 
----------------
Maybe add a comment here saying we can get here with an annotation token after an error?


https://reviews.llvm.org/D44449