[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:6435-6436 +Attr *WeakA = nullptr; +for (Attr *A : VD->getAttrs()) { + if (!isa(A)) +continue; rsmith wrote: > aaron.ballman wrote: > > Ah, it's too bad that `Decl::s

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 298268. rsmith marked 3 inline comments as done. rsmith added a comment. - Addressing review feedback from @aaron.ballman. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89212/new/ https://reviews.llvm.org/D89212

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done. rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:6435-6436 +Attr *WeakA = nullptr; +for (Attr *A : VD->getAttrs()) { + if (!isa(A)) +continue; aaron.ballman wrote: > Ah, it's too b

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D89212#2326771 , @rjmccall wrote: > Patch looks good to me. I think we're running some internal tests; do you > want to wait for those? More testing and validation would be nice; I don't think this patch is urgent so I'm happ

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-14 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 aside from some very small nits (feel free to ignore any that don't make sense to you). Comment at: clang/lib/AST/DeclBase.cpp:622 +AvailabilityResult A

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Patch looks good to me. I think we're running some internal tests; do you want to wait for those? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89212/new/ https://reviews.llvm.org/D89212

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 297767. rsmith added a comment. - Don't warn if we see a weak definition for a declaration that's already Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89212/new/ https://reviews.llvm.org/D89212 Files: clang/

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. It does seem necessary to distinguish weak definitions from weak non-definitions, but that's completely reasonable — those two cases essentially act like totally different attributes that happen to be written with the same spelling. If we acknowledge that, I think tha

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-11 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D89212#2324127 , @rjmccall wrote: > This seems like a good idea in the abstract; we'll need to figure out what > the practical consequences are. Looks like it triggers warnings in Abseil at least; there, the code looks like th

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. This seems like a good idea in the abstract; we'll need to figure out what the practical consequences are. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89212/new/ https://reviews.llvm.org/D89212

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

2020-10-11 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added reviewers: rjmccall, aaron.ballman. Herald added a project: clang. rsmith requested review of this revision. Such code will not work in general; we might have already used the non-weakness, for example when constant-evaluating an address comparison Thisor