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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:236
+  using UseSetTy = SmallSet<const DeclRefExpr *, 16>;
+  using DefMapTy = DenseMap<const VarDecl *, const DeclStmt *>;
+
----------------
NoQ wrote:
> NoQ wrote:
> > This extra payload wasn't advertised but it's very useful because it's hard to jump from `VarDecl` to `DeclStmt` without it, and we need to do such jump because our analysis starts from unsafe *uses* of a variable, which only give us a `VarDecl`.
> Another way to look at this, is to notice that currently `DeclStmt` of the variable isn't a `Gadget` on its own, and ask whether it *should* be a `Gadget`.
> 
> I currently believe that yes, it should be a `Gadget`, because it can potentially be a "multi-variable" `Gadget` if, for example, the initializer-expression is itself another raw pointer `DeclRefExpr`. So there needs to be a way to communicate that the fix for the variable declaration may depend on `Strategy` for more than one variable, and that the `Strategy` itself should be chosen carefully, considering that choices for different variables may interact this way. And from the point of view of the *other* variable, this definitely needs to be a `Gadget`, it's no different from any other `Gadget`.
> 
> So, this part needs some more work.
What about `DeclStmt`s that declare multiple variables, some safe some unsafe. Do you plan to generate fixits in that case exploding the DeclStmt? 


Repository:
  rC Clang

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

https://reviews.llvm.org/D138253