danix800 added inline comments.
================
Comment at: clang/lib/Parse/ParseObjc.cpp:749
+ if (!Tok.is(tok::eof))
+ ConsumeToken();
break;
----------------
rjmccall wrote:
> aaron.ballman wrote:
> > 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?
> Okay, so now I can bring a little language expertise to bear. :)
>
> We're in the middle of parsing an ObjC block (e.g. `@interface`), and we see
> `@interface` or `@implementation`, which starts a new block. You can never
> nest these ObjC blocks, so the parser is reasonably assuming that the second
> `@keyword` is an attempt to start a new block and the user just forgot to
> terminate the last block with `@end`. Unfortunately, the actual recovery
> done by the parser doesn't seem to match the diagnostic and the fixit — it's
> trying to swallow `@interface Foo` (or whatever) and then continue the loop
> as if it were part of the current block, which is definitely not the right
> thing to do.
>
> The right way to recover here is to act like we actually saw `@end` and break
> out of the loop, leaving `Tok` on the `@` so that the parser will pick up
> parsing `@interface` normally after we return. To do that, we just need to
> get the ObjC keyword by peeking at the next token instead of consuming.
>
> Also, we should take this recovery path on every `@` keyword that's only
> allowed at the top level (so `@class`, `@compatibility_alias`, `@interface`,
> `@implementation`, and `@protocol`).
It's really great to learn things here! I don't know two much about ObjC. I
seached google trying to find some standard or specs for ObjC but only docs
like tutorials teaching how to use it can be found, so I might not be able to
give a good enough fix for this issue. I'll give it a try though.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156277/new/
https://reviews.llvm.org/D156277
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
