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

[ziqingluo-90]

Summary

It is possible that a function parameter does not have a name even in a function definition.
So we should not assert that function parameters always have names.

This patch lets the analysis give up on generating fix-its in cases where a function parameter of a definition has no name.
Later we can come up with a better solution, e.g., generating a name for the parameter.

[NoQ]

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

clang/lib/Analysis/UnsafeBufferUsage.cpp

1889–1890

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.

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp

160

There's no fundamental reason we can't fix this, we just didn't have time yet.

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1889–1890

I think you are right. I'll update the FIXME comment before land this patch.

clang/lib/Analysis/UnsafeBufferUsage.cpp

else
       return std::nullopt;
 
     // Append: arg-list
     const unsigned NumParms = FD->getNumParams();
     for (unsigned i = 0; i < NumParms; i++) {
       const ParmVarDecl *Parm = FD->getParamDecl(i);
 
       if (Parm->isImplicit())
-        continue;
-      assert(Parm->getIdentifier() &&
-             "A parameter of a function definition has no name");
+        continue;      
+      // FIXME: If a parameter has no name, it is unused in the
+      // definition. So we could just leave it as it is.
+      if (!Parm->getIdentifier()) 
+  // If a parameter of a function definition has no name:
+        return std::nullopt;
       if (i == ParmIdx)
         // This is our spanified paramter!
         SS << NewTypeText.str() << "(" << Parm->getIdentifier()->getName().str() << ", "
            << getUserFillPlaceHolder("size") << ")";
       else
         SS << Parm->getIdentifier()->getName().str();
       if (i != NumParms - 1)
         SS << ", ";

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp

 
 #define MACRO_NAME MyName
 
 void macroIdentifier(int *MACRO_NAME) { // The fix-it ends with a macro. It will be discarded due to overlap with macros.
   // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
   if (++MyName){}
 }
 
+// CHECK-NOT: fix-it:{{.*}}:
+void parmHasNoName(int *p, int *) { // cannot fix the function because there is one parameter has no name.
+  p[5] = 5;
+}
+
 #endif