[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D75844#1951915 <https://reviews.llvm.org/D75844#1951915>, @tbaeder wrote:

> Sorry for taking so long but it seems like I've went down a rabbit hole a bit. My previous patch sets the range in `parseGNUAttributes()` unconditionally, but that seems to trigger cases such as
>
>   // FixItLoc = possible correct location for the attributes
>   void ProhibitAttributes(ParsedAttributesWithRange &Attrs,
>                           SourceLocation FixItLoc = SourceLocation()) {
>     if (Attrs.Range.isInvalid())
>       return;
>     DiagnoseProhibitedAttributes(Attrs.Range, FixItLoc);
>     Attrs.clear();
>   }
>   
>
> in Parser.h. Because now the attributes have a valid range, clang emits lots of errors where it previously did not.
>
> Do you have a suggestion of what to do here, should I rather go back to a more local fix for the issue?


I think we rely on an invalid attribute range in quite a few places. IIRC, part of what makes this hard is a construct like: `[[]] int i;` where there is an attribute list but no actual attributes. We need to be able to prohibit the presence of an attribute list in some places (even if there are no attribute supplied). So we use the attribute range source location because looking for attribute is insufficient. (Note, this isn't specific to C++-style attributes, so it's a general mechanism we use across attribute styles).

That said, if `parseGNUAttributes()` only sets the range after having parsed the `__attribute__` keyword, I wouldn't expect any additional diagnostics when prohibiting attributes, so it's not clear to me what's going on.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75844/new/

https://reviews.llvm.org/D75844