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

[Aaron Ballman via Phabricator via cfe-commits]

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