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

[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 AvailabilityAttr::getAvailability(ASTContext &C) const {
+  return CheckAvailability(C, this, nullptr, VersionTuple());
----------------
This is just novel enough that I'd appreciate a comment about the declaration being in Attr.td so it's more obvious where to find this.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:6431
+  // be null.
+  auto *VD = dyn_cast<ValueDecl>(&ND);
+  if (VD && VD->isUsed(false) && VD->isWeak() &&
----------------
Can the pointers here all be marked `const` or does that cause issues?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:6435-6436
+    Attr *WeakA = nullptr;
+    for (Attr *A : VD->getAttrs()) {
+      if (!isa<WeakAttr, WeakRefAttr, WeakImportAttr, AvailabilityAttr>(A))
+        continue;
----------------
Ah, it's too bad that `Decl::specific_attrs()` doesn't accept a pack of attributes...


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18288
 
-  if (PrevDecl) {
-    PrevDecl->addAttr(WeakAttr::CreateImplicit(Context, PragmaLoc, AttributeCommonInfo::AS_Pragma));
+  if (NamedDecl *PrevDecl =
+          LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName)) {
----------------
Same request for `const` here as above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89212/new/

https://reviews.llvm.org/D89212