[PATCH] D156189: [-Wunsafe-buffer-usage] Refactor to let local variable fix-its and parameter fix-its share common code

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

LGTM! I have minor suggestions for comments.



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1791-1794
 // For a `VarDecl` of the form `T  * var (= Init)?`, this
-// function generates a fix-it for the declaration, which re-declares `var` to
-// be of `span<T>` type and transforms the initializer, if present, to a span
-// constructor---`span<T> var {Init, Extent}`, where `Extent` may need the user
-// to fill in.
+// function generates fix-its that
+//  1) replaces `T * var` with `std::span<T> var`; and
+//  2) changes `Init` accordingly to a span constructor, if it exists.
----------------



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1828
+  if (!IdentText)
+    return {};
+  // Fix the initializer if it exists:
----------------
Should we start adding debug notes to all these early-returns? Or are they already covered by a catch-all debug note down the line?


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp:20-23
+  // We do not fix the following declaration. Because if the
+  // definition of `Int_ptr_t` gets changed, the fixed code becomes
+  // incorrect and may NOT be noticed.
   Int_ptr_t x = new int[10];
----------------



================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp:41-44
+  // We do not fix the following declaration. Because if the
+  // definition of `Int_ptr_t` gets changed, the fixed code becomes
+  // incorrect and may NOT be noticed.
   auto p = new int[10];
----------------



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

https://reviews.llvm.org/D156189