benhamilton added inline comments.

================
Comment at: lib/Format/TokenAnnotator.cpp:155
+           Next->startsSequence(tok::identifier, tok::l_square,
+                                tok::numeric_constant, tok::r_square,
+                                tok::r_paren, tok::l_paren))) {
----------------
djasper wrote:
> benhamilton wrote:
> > benhamilton wrote:
> > > djasper wrote:
> > > > benhamilton wrote:
> > > > > djasper wrote:
> > > > > > This seems suspect. Does it have to be a numeric_constant?
> > > > > Probably not, any constexpr would do, I suspect. What's the best way 
> > > > > to parse that?
> > > > I think this is the same answer for both of your questions. If what you 
> > > > are trying to prevent "FOO(^)" to be parsed as a block, wouldn't it be 
> > > > enough to look for whether there is a "(" after the ")" or even only 
> > > > after "(^)", everything else is already correct IIUC? That would get 
> > > > you out of need to parse the specifics here, which will be hard.
> > > > 
> > > > Or thinking about it another way. Previously, every "(^" would be 
> > > > parsed as an ObjC block. There seems to be only a really rare corner 
> > > > case in which it isn't (macros). Thus, I'd just try to detect that 
> > > > corner case. Instead you are completely inverting the defaults 
> > > > (defaulting to "^" is not a block) and then try to exactly parse ObjC 
> > > > where there might be many cases and edge cases that you won't even 
> > > > think of now.
> > > Hmm. Well, it's not just `FOO(^);` that isn't a block:
> > > 
> > > ```
> > > #define FOO(X) operator X
> > > 
> > > SomeType FOO(^)(int x, const SomeType& y) { ... }
> > > ```
> > > 
> > > Obviously we can't get this perfect without a pre-processor, but it seems 
> > > like our best bet is to only assign mark `TT_ObjCBlockLParen` when we are 
> > > sure the syntax is a valid block type or block variable.
> > I tried the suggestion to only treat `(^)(` as a block type, but it appears 
> > this is the primary place where we set `TT_ObjCBlockLParen`, so I think we 
> > really do need to handle the other cases here.
> I don't follow your logic. I'd like you to slowly change this as opposed to 
> completely going the opposite way.
> 
> So currently, the only know real-live problem is "FOO(^);". So address this 
> somehow, but still default/error to recognizing too much stuff as a block.
> 
> Have you actually seen
> 
>   SomeType FOO(^)(int x, const SomeType& y) { ... }
> 
> in real code?
I haven't seen that example, but I have seen `FOO(^, OtherParam);`.

I'm trying to change the parser to handle this now without explicitly parsing 
all block types, but it's tricky. The following should be ObjC block types:

```
(^)(int, char);
(^foo)(int, char);
(^foo[10])(int, char);
(^foo[kNum])(int, char);
(^foo[(kNum + kOtherNum)])(int, char);
```

but the following should not:

```
FOO(^);
FOO(^, int, char);
```

I'll make it work, it's just easy to make a mistake.


Repository:
  rC Clang

https://reviews.llvm.org/D43906



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to