[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Aaron Ballman via Phabricator via cfe-commits]

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