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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
