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

aaron.ballman added a comment. In D75844#2677457 , @brooksmoses wrote: > FWIW, this now causes Clang to produce an error on this code, when it didn't > before: > > using namespace::foo __attribute__((deprecated("message"))); > > I discussed this with Ri

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

brooksmoses added a comment. FWIW, this now causes Clang to produce an error on this code, when it didn't before: using namespace::foo __attribute__((deprecated("message"))); I discussed this with Richard Smith, who points out that GCC does not accept this and it's not permitted according to

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

aaron.ballman added a comment. In D75844#2668095 , @thakis wrote: > The test fails on macOS: http://45.33.8.238/macm1/6821/step_6.txt > > Please take a look and revert for now if it takes a while to fix. I've committed a speculative fix in 241d42c38226e46

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

thakis added a comment. The test fails on macOS: http://45.33.8.238/macm1/6821/step_6.txt Please take a look and revert for now if it takes a while to fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844

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

aaron.ballman added a comment. Thank you for the additional test case, this LGTM again. 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://l

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

tbaeder added a comment. I compiled libc++ with a clang that has this patch applied and didn't run into more problems. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 ___ cfe-commits mailing list cfe-

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

tbaeder updated this revision to Diff 335016. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/ParsedAttr.h clang/lib/Parse/ParseDecl.cpp clang/lib/Parse/ParseDeclCXX.cpp

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

aaron.ballman added inline comments. Comment at: clang/test/Parser/cxx0x-attributes.cpp:377 + template friend class[[]] vector2; // expected-error {{an attribute list cannot appear here}} + template friend class [[clang::__type_visib

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

tbaeder updated this revision to Diff 334954. tbaeder added a comment. Herald added a subscriber: jdoerfert. A little earlier than expected. Happy Easter :P CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 Files: clang/include/clang/Parse/Pars

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

tbaeder added a comment. Yep. I'll try to come up with a test and a fix next week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 ___ cfe-commits mailing list cf

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

aaron.ballman added a comment. In D75844#2664119 , @tbaeder wrote: > Damn. Reverted again for the time being. The libc++ build seems to fail and I > won't have time to look into that this week. :( Thank you for the quick revert! From looking at the test

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

tbaeder added a comment. Damn. Reverted again for the time being. The libc++ build seems to fail and I won't have time to look into that this week. :( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 __

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1ea9fa8c507e: [clang][parser] Set source ranges for GNU-style attributes (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 ___ cfe-commits mailing list

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

tbaeder updated this revision to Diff 334668. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/ParsedAttr.h clang/lib/Parse/ParseDecl.cpp clang/test/AST/sourceranges.cpp c

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

aaron.ballman added a comment. Thank you for picking this back up! Comment at: clang/include/clang/Parse/Parser.h:2708 + bool MaybeParseGNUAttributes(ParsedAttributes &attrs, SourceLocation *endLoc = nullptr, aaron.ballman wro

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

tbaeder updated this revision to Diff 334605. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/new/ https://reviews.llvm.org/D75844 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/ParsedAttr.h clang/lib/Parse/ParseDecl.cpp clang/test/AST/sourceranges.cpp c

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

tbaeder updated this revision to Diff 334604. tbaeder added a comment. I know it's been a while, but here's an update on that patch. Now that I've got https://reviews.llvm.org/D97362 and https://reviews.llvm.org/D97371 pushed, this is a much simpler change and does not break any of the existin

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

aaron.ballman added a comment. In 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 `Pa

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

tbaeder added a comment. 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. `MaybePar

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

aaron.ballman added a comment. In 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

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

tbaeder added a comment. 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 Prohibi

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

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:110-114 +case 26: + return 0; +__attribute__((fallthrough)); // expected-warning{{fallthrough annotation in unreachable code}} +case 27: + break; ---

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

tbaeder marked an inline comment as done. tbaeder added a comment. Thanks, I'll take a look at the tests. Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:110-114 +case 26: + return 0; +__attribute__((fallthrough)); // expected-warning{{fallthrough an

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

aaron.ballman added a reviewer: rsmith. aaron.ballman added a subscriber: rsmith. aaron.ballman added a comment. I think this is a reasonable approach (including moving `ParsedAttributesWithRange` into `Sema`), but we should have test coverage for the places where we're now using a range where w

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

tbaeder updated this revision to Diff 249574. tbaeder added a comment. Updated according to Aaron's comments. I'm not sure about changing the `DeclSpec` member and moving `ParsedAttributesWithRange` to ParsedAttr.h. Tests included. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75844/ne

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

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.

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

tbaeder created this revision. tbaeder added reviewers: tstellar, serge-sans-paille. tbaeder added a project: clang. Herald added a subscriber: cfe-commits. Otherwise the source range of the passed-in ParsedAttributesWithRange ends up being invalid and the resulting error messages rather useless.