aaron.ballman added inline comments.
================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1397
+ for (LateParsedAttribute *Attr : LateParsedAttrs) {
+ ParseLambdaLexedAttribute(*Attr, Attributes, D);
+ delete Attr;
----------------
cor3ntin wrote:
> 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.
Oh, well TIL. :-D
Yeah, don't bother with the assert then. Any probably don't bother with the
rename I suggested either.
(Someday) we'll either rename `LateParsedAttribute` to be more correct, or
we'll extend it to support more syntaxes and keep the same name. Either way,
the interfaces in your patch will be sufficiently clear.
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
