steakhal added inline comments.
================
Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:30
+ /// of primitive fixits (individual insertions/removals/replacements).
+ using FixItList = llvm::SmallVector<FixItHint, 2>;
+
----------------
Unless we have a well-formed idea of how many elements we are going to have, we
should probably omit it.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:83
+ /// Returns the list of pointer-type variables on which this gadget performs
+ /// its operation. Typically there's only one variable. This isn't a list
+ /// of all DeclRefExprs in the gadget's AST!
----------------
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:90
+ /// returns an empty list if no fixes are necessary.
+ virtual Optional<FixItList> getFixits(const Strategy &) const {
+ return None;
----------------
I wonder if we should prefer `std::optional` as we are c++17.
IDK if it was ever covered on the community forum.
I'm just leaving this here without expecting any action.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:231-233
+// An auxiliary tracking facility for the fixit analysis. It helps connect
+// declarations to its and make sure we've covered all uses with our analysis
+// before we try to fix the declaration.
----------------
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:259-262
+ // FIXME: Can this be less linear? Maybe maintain a map from VDs to DREs?
+ return any_of(*Uses, [VD](const DeclRefExpr *DRE) {
+ return DRE->getDecl()->getCanonicalDecl() == VD->getCanonicalDecl();
+ });
----------------
Maybe `Uses` should refer to the canonical decls. If that would be the case we
could just call `.contains(VD)` here.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:408
for (const auto &G : Gadgets) {
- Handler.handleUnsafeOperation(G->getBaseStmt());
+ DeclUseList DREs = G->getClaimedVarUseSites();
+
----------------
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:428-430
+ for (const auto &Item : Map) {
+ const VarDecl *VD = Item.first;
+ const std::vector<const Gadget *> &VDGadgets = Item.second;
----------------
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
