[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:17-19
+// Because the analysis revolves around variables and their types, we'll need to
+// track uses of variables (aka DeclRefExprs).
+using DeclUseList = SmallVector<const DeclRefExpr *, 1>;
----------------
aaron.ballman wrote:
> Do we have to care about data member accesses (which would be a `MemberExpr` and not a `DeclRefExpr`)?
> 
> Also, because this shows up in interfaces, it should probably be a `SmallVectorImpl` so the size doesn't matter.
> Do we have to care about data member accesses (which would be a `MemberExpr` and not a `DeclRefExpr`)?

Unlikely. Auto-fixing member variables without breaking source or binary compatibility is next to impossible.

> Also, because this shows up in interfaces, it should probably be a SmallVectorImpl so the size doesn't matter.

This doesn't seem to work on return types, which is where these classes show up in my case (`SmallVectorImpl` has deleted copy-move constructors so it's only suitable for passing by reference).


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:266-271
+    for (const Decl *D: DS->decls()) {
+      if (const auto *VD = dyn_cast<VarDecl>(D)) {
+        assert(Defs.count(VD) == 0 && "Definition already discovered!");
+        Defs[VD] = DS;
+      }
+    }
----------------
aaron.ballman wrote:
> Use a `specific_decl_iterator<VarDecl>` instead?
Hmm `DeclStmt` doesn't seem to provide a neat accessor, would you like me to add it?


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

https://reviews.llvm.org/D138253