aaron.ballman added a comment.
Thank you for the patch! It should also have some test cases associated with
it. I'd recommend adding a test to the `AST` directory that do AST dumps where
you test the line and column information directly.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:238-239
GNUAttributeLoc = Tok.getLocation();
+ Attrs.Range.setBegin(GNUAttributeLoc);
ParseGNUAttributes(Attrs);
goto Retry;
----------------
This seems wrong -- it may not be the first attribute in the statement (this is
in `ParseStatementOrDeclarationAfterAttributes()`), so there may already be
`[[]]` attributes that have been parsed, which this will overwrite. You would
need to see if the start range is invalid and only set this in that case.
However, wouldn't it make more sense for `ParseGNUAttributes()` to accept a
`ParsedAttributesWithRange` and properly set the range there? (This may have a
larger impact though, because that function gets called in a lot more code
paths than this switch case does. I suspect we have a fair amount of
improvements we could make with attribute source ranges.)
Repository:
rG LLVM Github Monorepo
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
