[PATCH] D153059: [-Wunsafe-buffer-usage][WIP] Group parameter fix-its

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, t-rasmud, jkorous, malavikasamak.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For a function `F` whose parameters need to be fixed,  we group fix-its of its' parameters together so that either all of the parameters get fixed or none of them gets fixed.

The reason for doing so is to avoid generating low-quality fix-its.   For example,

  void f(int * p, int * q) {
     int i = p[5];
     int j = q[5];
  }

We would like to fix both parameters `p` and `q` with `std::span`.  If we do not group them together, they will be fixed in two steps: 1) fixing `p` first; and then 2) fixing `q`.   Step 1 yields a new overload of `f`:

  void f(std::span<int> p, int *q) {  ...  }
  [[unsafe_buffer_usage]] void f(int *p, int *q) {return f(std::span(p, <# size #>), q);}
  }

The new overload of `f` is, however, still an unsafe function.  Then we apply step 2 and have:

  void f(std::span<int> p, std::span<int> q) {  ...  }
  [[unsafe_buffer_usage]] void f(int *p, int *q) {return f(std::span<int>(p, <# size #>), q);}
  [[unsafe_buffer_usage]] void f(std::span<int> p, int *q) {return f(p, std::span<int>(q, <# size #>));}

The "intermediate" overload `void f(std::span<int>, int *)` stays there but usually is not useful.  If there are more than two parameters need to be fixed,  there will be more such useless "intermediate" overloads.

With this patch, `p` and `q` will be fixed together:  either selecting `p` or `q` to fix will result in a safe function `void f(std::span<int> p, std::span<int> q` in one step.

The implementation is based on this patch <https://reviews.llvm.org/D145739>, which uses a directed graph to group variables that depend on each other.   To group parameters,  I explicitly add directed edges between all parameters that need fix so that they form a ring.   As being a ring in the directed graph, no such parameter will be missed regardless of where the search in the graph starts, as long as at least one such parameter is reachable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153059

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153059.531834.patch
Type: text/x-patch
Size: 46506 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230615/0dacb443/attachment-0001.bin>