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

2021-04-08 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-08 Thread Brooks Moses via Phabricator via cfe-commits
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

2021-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-04 Thread Nico Weber via Phabricator via cfe-commits
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

2021-04-03 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-02 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-02 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-02 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-02 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
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

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
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

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
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

2020-06-16 Thread Aaron Ballman via Phabricator via cfe-commits
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

2020-06-05 Thread Timm Bäder via Phabricator via cfe-commits
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

2020-04-08 Thread Aaron Ballman via Phabricator via cfe-commits
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

2020-03-31 Thread Timm Bäder via Phabricator via cfe-commits
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

2020-03-23 Thread Aaron Ballman via Phabricator via cfe-commits
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

2020-03-23 Thread Timm Bäder via Phabricator via cfe-commits
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

2020-03-14 Thread Aaron Ballman via Phabricator via cfe-commits
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

2020-03-11 Thread Timm Bäder via Phabricator via cfe-commits
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

2020-03-09 Thread 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.

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

2020-03-09 Thread Timm Bäder via Phabricator via cfe-commits
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.