[PATCH] D155641: [-Wunsafe-buffer-usage] Do not assert that function parameters have names.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I nitpicked to comments, but other than that LGTM, thanks!



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1890-1891
+      // A parameter of a function definition has no name.
+      // FIXME: We should create a name for the parameter as part of the fix-it.
+      // Go through declarations of the function and look for a name?
+      if (!Parm->getIdentifier())
----------------
There doesn't have to be a name anywhere, and if it's not there in the definition this usually means the parameter is unused. If it's unused, we won't ever fix it, and this also means that there's no need to give it a name. So I suspect that in the typical case the correct behavior is to just preserve the anonymous parameter as-is.

This is just my usual argument: I think the fixit should simply take the part that doesn't need fixing from the original code, and include it textually. In this case this would mean duplicating the text rather than simply not touching it, but that's probably still more precise than writing code from scratch.


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp:160
+// CHECK-NOT: fix-it:{{.*}}:
+void parmHasNoName(int *p, int *) { // cannot fix the function because there is one parameter has no name.
+  p[5] = 5;
----------------
There's no fundamental reason we can't fix this, we just didn't have time yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155641