cor3ntin added inline comments.

================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1397
+        for (LateParsedAttribute *Attr : LateParsedAttrs) {
+          ParseLambdaLexedAttribute(*Attr, Attributes, D);
+          delete Attr;
----------------
aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > I think you should add an assert before this call to ensure that all 
> > > attributes are GNU ones.
> > I'm not sure that gains much? 
> > In the future if late parsing is implemented for `__declspec` this would 
> > just work , unless we add an assert. I'd be fine adding it though.
> Because `ParseLambdaLexedAttribute()` specifically parses GNU attribute args 
> and not declspec ones, I think that function should be renamed. Once that 
> function is renamed to say GNU explicitly, then there's a question of what 
> happens when we hand it late parsed declspec attribute. Which is why I was 
> suggesting a comment above to mention that declspec attributes don't late 
> parse anyway.
> 
> Another way to handle this that's more general would be to change 
> `ParseLambdaLexedAttribute()` to parse different args depending on the syntax 
> of the attribute (then it can keep its generic name).
Okay, after actually reading the code, we can't actually assert on that. 

`LateParsedAttribute` is really `LateParsedGNUAttribute`. And maybe we want to 
rename that after this patch. It can never contain anything else, and because 
`__attribute__` was eaten, we don't really have anything to check for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119136

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

Reply via email to