aaron.ballman added inline comments.
================
Comment at: clang/lib/Parse/ParseTentative.cpp:773
+ while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+ if (Tok.is(tok::l_square)) {
----------------
rsmith wrote:
> aaron.ballman wrote:
> > Do we need to also skip other keyword attributes as well as `alignas`? For
> > instance, `tok::kw__Noreturn` or `tok::kw__Alignas`
> `_Alignas` and `_Noreturn` are their own kinds of //decl-specifier//s, not
> attributes. In particular, they're not syntactically valid in any of the
> places that call this (after a `*` or a tag keyword). It looks like we're
> missing support for them, but we should recognize them in
> `isCXXDeclarationSpecifier`, not here -- but I think that's unrelated to this
> patch.
Okay, that's a good point. Thanks!
================
Comment at: clang/lib/Parse/ParseTentative.cpp:789
+ ConsumeParen();
+ if (!SkipUntil(tok::r_paren))
+ return false;
----------------
rsmith wrote:
> aaron.ballman wrote:
> > If we're skipping `__attribute__(())`, this looks to miss the second
> > closing right paren. Also, this logic won't work if we need to skip
> > `_Noreturn`, which has no parens.
> >
> > This branch is missing test coverage.
> Because `SkipUntil` handles nested parens, this does the right thing for
> `__attribute__((...))`. Agreed on test coverage :)
I'll update the docs for `SkipUntil()`, because they don't suggest that this
magic takes place. :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74643/new/
https://reviews.llvm.org/D74643
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
