aaron.ballman added a comment. In D75844#2076260 <https://reviews.llvm.org/D75844#2076260>, @tbaeder wrote:
> I'm looking at this again and I am not sure how it is meant to work. For > example in `Parser::parseClassSpecifier` in `ParseDeclCXX.cpp`. > Here `attrs` is a local variable of type `ParsedAttributesWithRange`, > already before my patch. `attrs` is then passed to > > 1. `MaybeParseGNUAttributes` > 2. `MaybeParseMicrosoftDeclSpecs` > 3. `ParseMicrosoftInheritanceClassAttributes` > 4. `MaybeParseCXX11Attributes` > > and later `parseClassSpecifier` calls `ProhibitAttributes(attrs)` a few > times. `ProhibitAttributes` in turn will not do anything if the given attrs > have an invalid (i.e. unset) range. So, how could they ever have a valid > range set? All the four functions above only take a `ParsedAttributes`, no > range. > > This is one of the cases that now (that `MaybeParseGNUAttributes` sets the > range of the given `attrs` if it really parses attributes) generates errors > in various test cases. For example in > `clang/test/AST/ast-print-record-decl.c`: File > /home/tbaeder/llvm-project/clang/test/AST/ast-print-record-decl.c Line 209: > an attribute list cannot appear here > > Am I missing something? I don't think you're missing anything -- the state of the system is currently inconsistent. From (possibly faulty) memory, we previously only tracked the minimum of location information for attribute source locations and some parts of the system relied on invalid source locations to convey meaning that it shouldn't have. This is compounded by the fact that the different attribute syntaxes (gnu, C++, declspec, etc) have been worked on at different times with different assumptions about source locations, but are all generalized in the same parsed attributes data structure. Now that you're starting to more consistently track source location information across syntaxes, you're hitting some fallout from those inconsistencies. Hopefully there's not so much fallout that it cannot be handled though (if there is, please speak up and we can try to explore other options). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
