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

Reply via email to