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

Wed Jul 12 14:49:11 PDT 2023

ziqingluo-90 added a comment.

- a major change to the patch:  Parameters are now grouped after the grouping of all variables.  The groups that contain parameters of the function will form a union group.

- changes to the grouping algorithm: Groups will reside in the vector `Groups` and being accessed through their indexes so that we do not have to copy groups.  The `GrpsUnionForParms` is the union group over groups containing parameters.  It holds a separate copy. These data structures are wrapped in `VariableGroupsManager`, which returns the proper group given any variable that needs fix.

- add a new diagnostic note kind:  `diag::note_unsafe_buffer_variable_fixit_together`. It will replace `diag::note_unsafe_buffer_variable_fixit_group` when we do not want to explain how a group of variables gets formed.

- add a "Comparator" to `FixableGadgetSets` and replace some `std::set`s with `llvm::SetVector`s for deterministic diagnostic messages.

================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2168
+  // itself:
+  std::string listVariableGroupAsString(
+      const VarDecl *VD, const ArrayRef<const VarDecl *> &VarGroupForVD) const {
----------------
The printing process was extracted as a method with some refactoring.

================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp:251
+}
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
----------------
t-rasmud wrote:
> Does this patch handle virtual methods? Ideally we'd like the fixed methods to have the same subtyping relations as the original method. Does it make sense to have test cases with virtual methods as part of this patch?
Sorry for the late response.  
That's a good point.  Virtual methods could get things more complicated.  This is one of the reasons that we do not fix any member functions at this point.

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

https://reviews.llvm.org/D153059