[PATCH] D157441: [-Wunsafe-buffer-usage] Use `Strategy` to determine whether to fix a parameter

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added inline comments.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1624
     // Inserts the [0]
-    std::optional<SourceLocation> EndOfOperand =
-        getEndCharLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts());
-    if (EndOfOperand) {
+    if (auto LocPastOperand =
+            getPastLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts())) {
----------------
NFC.  Just noticed that `getPastLoc` is the better function to call here.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2719
+  Strategy NaiveStrategy = getNaiveStrategy(
+      llvm::make_range(VisitedVars.begin(), VisitedVars.end()));
   VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap, GrpsUnionForParms);
----------------
`VisitedVars` are the set of variables on the computed graph.  The previous `UnsafeVars` is a superset of it, including safe variables that have `FixableGadget`s discovered.  We do not want to assign strategies other than `Won't fix` to those safe variables.


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp:31
 
-  b = a;
+  b = a;    // CHECK-NOT: fix-it:{{.*}}:{[[@LINE]]
   b[5] = 5; // expected-note{{used in buffer access here}}
----------------
Both `b` and `a` will be changed to have `std::span` type.  So this assignment needs no fix-it.     Same reason applies to similar changes in the rest of this test file.


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp:83
+  x = p;  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE]]
+  y = x;  // CHECK: fix-it:{{.*}}:{[[@LINE]]:8-[[@LINE]]:8}:".data()"
   // `x` needs to be fixed so does the pointer assigned to `x`, i.e.,`p`
----------------
`y` does not have to be changed to a safe-type while `x` does.  So we need to fix this assignment.   Same reason applies to similar changes in the rest of this test file.


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp:29
+  int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+	          expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' to 'std::span' to propagate bounds information between them}}
   p[5] = 10;  // expected-note{{used in buffer access here}}
----------------
We now support to fix ` int *r = q`.  So all the unsafe variables can be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157441