aaron.ballman added inline comments.
================ Comment at: clang/lib/Parse/ParseDecl.cpp:1618 + + if (Tok && (*Tok).is(tok::l_square)) { + Diag(Attrs.Range.getBegin(), diag::err_attributes_not_allowed) << Attrs.Range; ---------------- tbaeder wrote: > aaron.ballman wrote: > > This will incorrectly classify an empty MS attribute `[]` as being a > > prohibited `[[]]` attribute. I think we need something like: > > ``` > > if (Tok && Tok->is(tok::l_square)) { > > SourceLocation NextTokLoc = > > Lexer::findLocationAfterToken(Attrs.Range.getBegin(), Tok.getKind(), SM, > > getLangOpts(), true); > > auto NextTok = Lexer::findNextToken(NextTokLoc, SM, getLangOpts()); > > if (NextTok && NextTok->is(tok::l_square)) { > > ... > > } > > } > > ``` > > Also, I think it should use `DiagID` rather than hard-coding the diagnostic > > to use. > The `findNextToken()` here returns the second `[` in `[[]]`, so would return > the `]` for `[]`. The code you proposed doesn't work because the second > `findNextToken()` returns the first `]`, so not a `tok::l_square`. I know > your code makes more sense since we're looking for `[[`, but I couldn't find > a way to get the first `[` from the Lexer. Hrm, that surprises me -- we have the correct range when printing diagnostics, such as https://godbolt.org/z/r8rTKhY7f. What happens if you use `Lexer::getRawToken()` on `Attrs.Range.getBegin()`? Does that find the first `[` or the second one? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97362/new/ https://reviews.llvm.org/D97362 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits