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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
