I noticed that this is basically always unused. And if it is needed, the end of the attribute range can be used.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thank you for working on this! We've (very) slowly been working towards all of the attribute parsing functions taking a ParsedAttributesWithRange so that we don't lose this information in the AST, and this is a good step in the right direction towards that.
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
820–821 | This is a case where we're regressing functionality -- we used to track the actual end location here, but because this doesn't take a ParsedAttributesWithRange, the end location is lost. Can we rework this to take a ParsedAttributesWithRange instead? | |
clang/lib/Parse/ParseDeclCXX.cpp | ||
4661–4662 | Heh, same concern here as above. | |
clang/lib/Parse/ParseStmt.cpp | ||
1121–1122 | It seems like this could be re-flowed to 80-col? |
Yes, that's what I'm working on. There is a very peculiar test failure in clang/test/SemaOpenCL/address-spaces.cl however, so I can't switch everything over to ParsedAttributesWithRange (and do a rename) at once.
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
820–821 | Pretty sure yes. I already have that change locally anyway, I'll add it to this patch and run the tests. | |
clang/lib/Parse/ParseDeclCXX.cpp | ||
4661–4662 | Also same as above :) |
LGTM aside from two formatting nits, thank you for the cleanup!
clang/lib/Parse/ParsePragma.cpp | ||
---|---|---|
1373–1374 | Probably worth reformatting this before you land. |
This is a case where we're regressing functionality -- we used to track the actual end location here, but because this doesn't take a ParsedAttributesWithRange, the end location is lost. Can we rework this to take a ParsedAttributesWithRange instead?