[PATCH] D156277: [Parser][ObjC] Stop parsing on eof

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Parse/ParseObjc.cpp:749
+      if (!Tok.is(tok::eof))
+        ConsumeToken();
       break;
----------------
rjmccall wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > danix800 wrote:
> > > > tbaeder wrote:
> > > > > Why is there a `ConsumeToken()` call at all here? The token is already being consumed in line 729.
> > > > Didn't notice this, thanks for reminding!
> > > I have the same question as @tbaeder -- what token is this intending to consume? CC @rjmccall for Obj-C expertise
> > OH! This is consuming the identifier for the implementation/interface name itself. e.g.,
> > ```
> > @interface Frobble
> > ```
> > The consume on line 709 gets the `@`, the consume on line 729 gets the `interface`, and the consume on line 749 is getting the `Frobble`. That makes sense to me now.
> > 
> I don't think any language expertise is required here — just seems like a straightforward bug on an error path that's probably not exercised all that often.  Maybe somebody moved the `ConsumeToken` and forgot to fix this case or something.
What concerns me about this fix is that we don't typically check whether the token is EOF or not before consuming; that's usually an anti-pattern, isn't it? Wouldn't it make sense for this to use `SkipUntil(tok::identifier)` instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156277/new/

https://reviews.llvm.org/D156277