[-Wunsafe-buffer-usage] Add T* -> span<T> Fix-Its for function parameters

[ziqingluo-90]

Summary

Generate fix-its for function parameters. Currently, the analyzer fixes one parameter at a time.
Fix-its for a function parameter includes:

1. Fix the parameter declaration of the definition, result in a new overload of the function. We call the function with the original signature the old overload.
2. For any other existing declaration of the old overload, mark it with the [[unsafe_buffer_usage]] attribute and generate a new overload declaration next to it.
3. Creates a new definition for the old overload, which is simply defined by a call to the new overload.

Example,

void foo(int *p);

void foo(int *p) {
   int x = p[5];
}

will be like the following after applying fix-its

[[unsafe_buffer_usage]] void foo(int * p);
void foo(std::span<int> p);

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

[jkorous]

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

10

Oh, yes, this is fun. I hope I will eventually convince FileCheck this C++ attributes are not substitution blocks 😅

https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-string-substitution-blocks

[jkorous]

• rebased
• moved compatibility decl/def below original

still see crash in tests

[jkorous]

• fixed all crashes in our tests
• added cases of functions where we should not emit Fix-Its for parameters

[jkorous]

Started working on conflict detection for added compatibility overload.

[jkorous]

Note: One thing that I have little confidence the current patch handles correctly would be qualified function names. Definitely need tests.

[jkorous]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1745

We should not use the attribute directly - instead we should have a macro that gates it with __has_cpp_attribute.
https://clang.llvm.org/docs/LanguageExtensions.html#has-cpp-attribute

[jkorous]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1745

Also, we need to think about how do we vend the macro.

[jkorous]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1726

Sorry, the example below is unrelated. What I meant is that we shouldn't for example use \n on Windows.
EDIT: We also realized during an offline discussion that we should have our tests reflect that (somehow).

[jkorous]

As we discussed offline - we should add tests that our overload conflict detection would notice overloads that are declared below the function that's being analyzed.
Since our analysis callback is invoked after a body function has been parsed (but before the whole TU is parsed) it is not immediately clear if all function declarations in the TU will already be represented in the AST at that point.

[ziqingluo-90]

Taking care of this patch on behalf of @jkorous

[ziqingluo-90]

Still work in progress (but comments are very welcomed) ---there are several problems need to be solved:

• To conservatively detect any overload of the function whose parameter is being fixed. Thus far, the analysis cannot see any overload declared after the analyzing function.
• Fix-its conflict. Insertions of #include<span> and [[clang::unsafe_buffer_usage]] can conflict if both are inserted at the beginning of a file.
• There are many unsupported cases.
• Do we really want to fix parameters one at a time?
• As the fix-its become more complicated, tests now are harder to read.

[jkorous]

I just discovered my original prototype had yet another bug - parameters that use "array syntax" would get spanified but their name would be missing.

E. g.
void foo(int param[]) {
would get transformed to (notice the missing param name:
[clang::unsafe_buffer_usage] void foo(std::span<int>) {

I am wondering what is the source range of the parameter type in this case - the original code likely expects parameter type to be left of parameter name which might or might not be true in case.

It is possible that @ziqingluo-90 has fixed the bug in the meantime but I don't see a test - can we add one just to be sure?

It is likely that Transformer might solve this problem out of the box - maybe we should use it for this patch after all.

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

64

Since you mention main in the tests...
We can't change its signature. Not emitting Fix-Its (and not emitting the note to change param type) might be sufficient for now.
Long-term we might want a special Fix-It for argv - maybe construct std::span argv{argv_raw, argc}; local variable but that should be done in another patch.
Can you please add a test that we don't emit Fix-It for argv?

[jkorous]

I found one more bug.

The prototype mishandles const qualifier of the parameter type which means it transforms const int* to const span<const int> which is incorrect. That would correspond to const int* const.

Can we add a test for this case?

[ziqingluo-90]

Fix issues in transforming parameter declarations of the form const T * var and T * var[].
Add test for overload declarations appearing after the analyzing function with unsafe parameters.
Add more tests for unsupported cases so that we don't forget.

[jkorous]

I just realized one more problem we have to address with this patch - default arguments.
ParmVarDecl::getDefaultArg should be able to tell us if in any of the declarations of the function there is a default argument for the parameter we want to transform. Then we need to figure out if we can fix it/how do we fix it and we shouldn't emit the fixit if we can't fix the default argument. This problem is pretty similar to local variable initialization and we should try to reuse the solution.

clang/lib/Analysis/UnsafeBufferUsage.cpp

1702

This TODO will hopefully no longer be true after we land.
https://reviews.llvm.org/D146342

1710

IMO we should wrap this in some macro to make it user-friendlier.

Something like:

#ifndef UNSAFE_BUFFER_USAGE
#if __has_cpp_attribute(clang::unsafe_buffer_usage)
#define UNSAFE_BUFFER_USAGE [[clang::unsafe_buffer_usage]]
#else
#define UNSAFE_BUFFER_USAGE 
#endif
#endif

so the code (from our fix-its or written by the users) can look like this:

UNSAFE_BUFFER_USAGE void unsafe_foo(int* ptr, unsigned size);

...and we need to figure where exactly should the macro live (or if there should be multiple definitions, etc.).

I would love to hear @NoQ's thoughts on this topic.

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1710

That's, uh, interesting. We want users to use a macro of this kind and we need the macro to be defined, just like we want the users to use span and we want <span> to be included. But unlike #include <span>, we *really* don't want users to define the macro manually every time they need it. Instead we want it defined manually in some centralized place, and have the fixit machine pick it up from there.

So I think the fixit machine should never suggest *definining* the macro. That's the opposite of what we want.

On the other hand, there's a moderately famous precedent of compiler picking up an existing macro, namely the attribute [[fallthrough]]:

• example: https://godbolt.org/z/vvcW76cPE
• implementation: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0/clang/lib/Sema/AnalysisBasedWarnings.cpp#L1224)

So I think we can try to build something similar.

[NoQ]

In D143048#4256855, @jkorous wrote:

I just realized one more problem we have to address with this patch - default arguments.
ParmVarDecl::getDefaultArg should be able to tell us if in any of the declarations of the function there is a default argument for the parameter we want to transform. Then we need to figure out if we can fix it/how do we fix it and we shouldn't emit the fixit if we can't fix the default argument. This problem is pretty similar to local variable initialization and we should try to reuse the solution.

Oh right.

The problem with default arguments is that their AST lives outside of function body. It's the job of the *caller* to invoke this code. So that's a separate isolated piece of code that our matchers need to traverse together with the function's body.

I recommend giving up for the purposes of this patch (just make sure we don't try to fix such code) and see if we can address this later.

Which reminds me, we also need to handle CXXCtorInitializers correctly (or at least give up when we see them) - they're yet another piece of code that lives outside of function body, and they may access function parameters.

[ziqingluo-90]

To conservatively give up on fixing function parameters if they have default arguments.

The following improvement will be added by follow-up patches:

• Improve the insertion of [[unsafe_buffer_usage]]
• Fix function parameters with default arguments
• Group fix-its of multiple parameters

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1700–1701

I suspect that it's always going to be "the whole TU", given that Sema is currently at the end of TU parsing (because that's where we put our analysis).

I recommend adding some tests with namespaces and other situations when functions are nested into something. Esp. when the out-of-line definition may be lexically available outside of the scope, eg.

namespace N {
  void foo();
}

void N::foo() {
  // implementation
}

where the compatibility overload has to go into the right namespace and the implementation of the compatibility overload has to be correctly prefixed with N::.

1710

I think we have to go with raw [[clang::unsafe_buffer_usage]] for now. We can implement macro detection later, but we shouldn't insert preprocessor directives ourselves, because that's absolutely not how we want the final code to look.

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

27

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

11

Is there a semicolon at the end?

Also, do we want to preserve the parameter name p here, given that it's available on the respective redeclaration?

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

21

Why, what's wrong with it?

[ziqingluo-90]

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

11

No. There is no semicolon. The idea is that the fix-it tries to append a new function decl A; to a function decl B; but there is no obvious way to get the end location of B;. We only have the end location of B. So the fix-it inserts ;A to the end of B.

11

I did think about whether preserving the parameter name is necessary and didn't find a strong reason to do so.
A minor reason could be that if contracts are introduced in C++ one day, parameters in declarations need to have names so that contracts can refer to them.

I will add names to fix-its. Also I will be happy to hear any other benefits of doing that.

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

21

For const int * const x (or int * const x), we transform it to const std::span<const int> x (or const std::span<int> x). The left-most const are missing in the fix-its.

[ziqingluo-90]

Fix issues in overload lookup. Thanks to @NoQ 's comment.
Fix issues in handling qualified function or parameter names.
Add parameter names back for inserted overload declarations if the names exist in the original function declaration.

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1700–1701

This is very valuable comment that points out a big hole in my patch. Now it should be fixed. Thank you.

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1705–1707

Wait, did we decide to emit the #if check at every function after all?

I'd much rather temporarily emit the raw attribute without the check, than temporarily emit an #if, because the raw attribute is desired at least in some cases, but #if next to the function is never desired.

1718

Need to test this with various anonymous functions:

• constructors/destructors of anonymous classes? https://godbolt.org/z/876d9MY1v
• overloaded operators, most importantly lambda call operators?

1786–1787

What happens if these are in different FileIDs? Do we end up with a pair of pointers into unrelated buffers? (Eg., macro expansions, or #include in the middle of the function declaration.)

1889

Does this work correctly if the pointee type is an anonymous struct? It probably does (given that anonymous structs can't be used directly, and typedefs aren't skipped here).

I suspect it's more reliable to copy-paste the original spelling of the type from the source code than to rely on pretty-printing, but it could have other problems (eg., there's a typedef that covers the *).

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

11

Parameter name could be a valuable hint to the user about the purpose of the parameter. The user doesn't necessarily see the implementation, and it's not like we're copying over the comments before the function. So out of the few ways the caller can figure out what to put there, this could be the most important one.

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

21

Oh, right, yes, makes sense in both cases!

This doesn't really lead to broken code and it doesn't really sacrifice much const-correctness because const in this position applies only to the parameter's stack storage, which people don't usually overwrite anyway, given that it reliably disappears at end of function. In particular, it's completely immaterial to the caller.

(It could matter when the class does something non-trivial, like have weird global side effects in overloaded assignment operator, so if it's const it could act as a guarantee that these side effects don't happen no matter what the function does; but nobody writes code like this, and std::span specifically is definitely not written like this.)

But I totally agree that in the spirit of preserving maximum similarity to the original code, it's valuable to preserve const here.

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1889

We should definitely add a test for this even if it's already working correctly, because people love their .getCanonicalType() and I can easily see somebody adding it to the code later for some unrelated reason. They need to know that typedefs can't be dropped here.

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1705–1707

oh yes, I should have made the raw attribute the default case here. More advanced cases of dealing with attributes are addressed in this patch.

1718

These cases are not supported in this patch. (see the guard at lines 1358--1366).

1786–1787

This is a good point! I should have just obtained text representation of each part (i.e., return type and function name) separately.

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

21

Yes and thanks for the extra note.

Do you think const could also matter for pass-by-reference parameters? e.g.,
void f(int * const &p) -> void f(const std::span<int> &p)

I will add some tests for such cases

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1889

I hadn't thought of anonymous structs. They can be a problem.
But I think the problem is that we cannot getPointeeType() from a pointer-to an anonymous type.
For example,

typedef struct {int x;} * ANON_PTR;
void foo(ANON_PTR p) {
  ... p[5];
}

One of the generated fix-its looks like this
std::span<struct (unnamed struct at clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp:7:9)> q

Actually, getCanonicalType() is not a problem in the cases that we can handle. For example,

typedef struct {int x;} ANON_S;
typedef ANON_S * ANON_PTR;

void foo(ANON_PTR p) {  ... }

getCanonicalType() returns ANON_S for the pointee type of the type of p. If ANON_S is defined by a named struct, getCanonicalType() returns the named struct.

[ziqingluo-90]

Address comments and made the following changes to the patch:

1. Stop using SourceManager::getCharacterData to obtain source file texts. Because it is possible that the start and end pointer point to different buffers. Instead, a helper function getRangeText is created.
2. Stop using TypePrinter. Calling getAsString on a QualType uses the printer under the hood. The printer does not necessarily print a type t in the exact same text form as t appearing in the source file. Especially when macros or typedefs are used. We prefer to generate texts for types without any change to their original literal forms. A helper function getPointeeTypeText is created to generate text for types using TypeLocs to obtain source ranges of types. However, there are a few limitations:
   • TypeLoc does not provide source ranges of qualifiers. (There is a QualifiedTypeLoc that looks like an outlier to the rest of the TypeLoc sub-classes and seems fishy to me). We have to generate qualifier texts using a printer separately.
   • Currently, we cannot obtain the text of a pointee type from a parameter declaration, if the declarator is too complicated. It now only supports two forms of parameter declarations: T identifier or T identifier[], where T is any type.
   • If a pointee type is unnamed or has unnamed sub-types, we cannot guarantee that we can refer to that type. Although there are many cases where we still can refer to such an unnamed type, we give up for now.

[NoQ]

Hi, looks great to me now. It looks like it's at a point where there could be more cornercases for us to tackle, but the rough idea looks correct and we can address the rest of them incrementally. Great job!!

A few microscopic nitpicks but generally I think this is ok to land.

clang/lib/Analysis/UnsafeBufferUsage.cpp

1747

I'm a big fan of Present Simple all the way, unless there's a really good reason.

1785–1786

Slightly more C++-y this way.

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

47–49

[t-rasmud]

LGTM.

clang/lib/Analysis/UnsafeBufferUsage.cpp

1880

Minor nit: Changing function name to createOverloadsForFixedParams would need to reflect here as well.

clang/lib/Analysis/UnsafeBufferUsage.cpp

UPCAddressofArraySubscriptGadget::getFixits(const Strategy &S) const {
   case Strategy::Kind::Iterator:
   case Strategy::Kind::Array:
   case Strategy::Kind::Vector:
     llvm_unreachable("unsupported strategies for FixableGadgets");
   }
   return std::nullopt; // something went wrong, no fix-it
 }
 
+// FIXME: this function should be customizable through format
+static StringRef getEndOfLine() {
+  static const char *const EOL = "\n";
+  return EOL;
+}
+
 // Return the text representation of the given `APInt Val`:
 static std::string getAPIntText(APInt Val) {
   SmallVector<char> Txt;
   Val.toString(Txt, 10, true);
   // APInt::toString does not add '\0' to the end of the string for us:
   Txt.push_back('\0');
   return Txt.data();
 }
static std::optional<StringRef> getExprText(const Expr *E,
   if (LastCharLoc)
     return Lexer::getSourceText(
         CharSourceRange::getCharRange(E->getBeginLoc(), *LastCharLoc), SM,
         LangOpts);
 
   return std::nullopt;
 }
 
+// Returns the literal text in `SourceRange SR`, if `SR` is a valid range.
+static std::optional<StringRef> getRangeText(SourceRange SR,
+                                             const SourceManager &SM,
+                                             const LangOptions &LangOpts) {
+  bool Invalid = false;
+  CharSourceRange CSR = CharSourceRange::getCharRange(SR.getBegin(), SR.getEnd());
+  StringRef Text = Lexer::getSourceText(CSR, SM, LangOpts, &Invalid);
+
+  if (!Invalid)
+    return Text;
+  return std::nullopt;
+}
+
+// Returns the text of the pointee type of `T` from a `VarDecl` of a pointer
+// type. The text is obtained through from `TypeLoc`s.  Since `TypeLoc` does not
+// have source ranges of qualifiers ( The `QualifiedTypeLoc` looks hacky too me
+// :( ), `Qualifiers` of the pointee type is returned separately through the
+// output parameter `QualifiersToAppend`.
+static std::optional<std::string>
+getPointeeTypeText(const ParmVarDecl *VD, const SourceManager &SM,
+                   const LangOptions &LangOpts,
+                   std::optional<Qualifiers> *QualifiersToAppend) {
+  QualType Ty = VD->getType();
+  QualType PteTy;
+
+  assert(Ty->isPointerType() && !Ty->isFunctionPointerType() &&
+         "Expecting a VarDecl of type of pointer to object type");
+  PteTy = Ty->getPointeeType();
+  if (PteTy->hasUnnamedOrLocalType())
+    // If the pointee type is unnamed, we can't refer to it
+    return std::nullopt;
+
+  TypeLoc TyLoc = VD->getTypeSourceInfo()->getTypeLoc();
+  TypeLoc PteTyLoc = TyLoc.getUnqualifiedLoc().getNextTypeLoc();
+  SourceLocation VDNameStartLoc = VD->getLocation();
+
+  if (!SM.isBeforeInTranslationUnit(PteTyLoc.getSourceRange().getEnd(),
+                                    VDNameStartLoc)) {
+    // We only deal with the cases where the source text of the pointee type
+    // appears on the left-hand side of the variable identifier completely,
+    // including the following forms:
+    // `T ident`,
+    // `T ident[]`, where `T` is any type.
+    // Examples of excluded cases are `T (*ident)[]` or `T ident[][n]`.
+    return std::nullopt;
+  }
+  if (PteTy.hasQualifiers()) {
+    // TypeLoc does not provide source ranges for qualifiers (it says it's
+    // intentional but seems fishy to me), so we cannot get the full text
+    // `PteTy` via source ranges.
+    *QualifiersToAppend = PteTy.getQualifiers();
+  }
+
+  // Note that TypeLoc.getEndLoc() returns the begin location of the last token:
+  SourceRange CSR{
+      PteTyLoc.getBeginLoc(),
+      Lexer::getLocForEndOfToken(PteTyLoc.getEndLoc(), 0, SM, LangOpts)};
+
+  return getRangeText(CSR, SM, LangOpts)->str();
+}
+
+// Returns the text of the name (with qualifiers) of a `FunctionDecl`.
+static std::optional<StringRef> getFunNameText(const FunctionDecl *FD,
+                                               const SourceManager &SM,
+                                               const LangOptions &LangOpts) {
+  SourceLocation BeginLoc = FD->getQualifier()
+                                ? FD->getQualifierLoc().getBeginLoc()
+                                : FD->getNameInfo().getBeginLoc();
+  // Note that `FD->getNameInfo().getEndLoc()` returns the begin location of the
+  // last token:
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+      FD->getNameInfo().getEndLoc(), 0, SM, LangOpts);
+  SourceRange NameRange{BeginLoc, EndLoc};
+
+  return getRangeText(NameRange, SM, LangOpts);
+}
+
 std::optional<FixItList>
 DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
   const VarDecl *VD = dyn_cast<VarDecl>(BaseDeclRefExpr->getDecl());
 
   if (VD && s.lookup(VD) == Strategy::Kind::Span) {
     ASTContext &Ctx = VD->getASTContext();
     // std::span can't represent elements before its begin()
     if (auto ConstVal = Offset->getIntegerConstantExpr(Ctx))
static FixItList fixVarDeclWithSpan(const VarDecl *D, const ASTContext &Ctx,
   if (!ReplacementLastLoc)
     return {};
 
   FixIts.push_back(FixItHint::CreateReplacement(
       SourceRange{D->getBeginLoc(), *ReplacementLastLoc}, OS.str()));
   return FixIts;
 }
 
+static bool hasConflictingOverload(const FunctionDecl *FD) {
+  return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult();
+}
+
+// Returns the text representation of clang::unsafe_buffer_usage attribute.
+static std::string getUnsafeBufferUsageAttributeText() {
+  static const char *const RawAttr = "[[clang::unsafe_buffer_usage]]";
+  std::stringstream SS;
+  SS << RawAttr << getEndOfLine().str();
+  return SS.str();
+}
+
+// For a `FunDecl`, one of whose `ParmVarDecl`s is being changed to have a new
+// type, this function produces fix-its to make the change self-contained.  Let
+// 'F' be the entity defined by the original `FunDecl` and "NewF" be the entity
+// defined by the `FunDecl` after the change to the parameter.  Fix-its produced
+// by this function are
+//   1. Add the `[[clang::unsafe_buffer_usage]]` attribute to each declaration
+//   of 'F';
+//   2. Create a declaration of "NewF" next to each declaration of `F`;
+//   3. Create a definition of "F" (as its' original definition is now belongs
+//      to "NewF") next to its original definition.  The body of the creating
+//      definition calls to "NewF".
+//
+// Example:
+//
+// void f(int *p);  // original declaration
+// void f(int *p) { // original definition
+//    p[5];
+// }
+//
+// To change the parameter `p` to be of `std::span<int>` type, we
+// also add overloads:
+//
+// [[clang::unsafe_buffer_usage]] void f(int *p); // original decl
+// void f(std::span<int> p);                      // added overload decl
+// void f(std::span<int> p) {     // original def where param is changed
+//    p[5];
+// }
+// [[clang::unsafe_buffer_usage]] void f(int *p) {  // added def
+//   return f(std::span(p, <# size #>));
+// }
+//
+// The actual fix-its may contain more details, e.g., the attribute may be guard
+// by a macro
+//   #if __has_cpp_attribute(clang::unsafe_buffer_usage)
+//   [[clang::unsafe_buffer_usage]]
+//   #endif
+//
+// `NewTypeText` is the string representation of the new type, to which the
+// parameter indexed by `ParmIdx` is being changed.
+static std::optional<FixItList>
+createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
+                                const FunctionDecl *FD, const ASTContext &Ctx,
+                                UnsafeBufferUsageHandler &Handler) {
+  // FIXME: need to make this conflict checking better:
+  if (hasConflictingOverload(FD))
+    return std::nullopt;
+
+  const SourceManager &SM = Ctx.getSourceManager();
+  const LangOptions &LangOpts = Ctx.getLangOpts();
+  // FIXME Respect indentation of the original code.
+
+  // A lambda that creates the text representation of a function declaration
+  // with the new type signature:
+  const auto NewOverloadSignatureCreator =
+      [&SM, &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
+                       StringRef NewTypeText) -> std::optional<std::string> {
+    std::stringstream SS;
+
+    SS << ";";
+    SS << getEndOfLine().str();
+    // Append: ret-type func-name "("
+    if (auto Prefix = getRangeText(
+            SourceRange(FD->getBeginLoc(), (*FD->param_begin())->getBeginLoc()),
+            SM, LangOpts))
+      SS << Prefix->str();
+    else
+      return std::nullopt; // give up
+    // Append: parameter-type-list
+    const unsigned NumParms = FD->getNumParams();
+
+    for (unsigned i = 0; i < NumParms; i++) {
+      const ParmVarDecl *Parm = FD->getParamDecl(i);
+
+      if (Parm->isImplicit())
+        continue;
+      if (i == ParmIdx) {
+        SS << NewTypeText.str();
+        // print parameter name if provided:
+        if (IdentifierInfo * II = Parm->getIdentifier())
+          SS << " " << II->getName().str();
+      } else if (auto ParmTypeText =
+                     getRangeText(Parm->getSourceRange(), SM, LangOpts)) {
+        // print the whole `Parm` without modification:
+        SS << ParmTypeText->str();
+      } else
+        return std::nullopt; // something wrong, give up
+      if (i != NumParms - 1)
+        SS << ", ";
+    }
+    SS << ")";
+    return SS.str();
+  };
+
+  // A lambda that creates the text representation of a function definition with
+  // the original signature:
+  const auto OldOverloadDefCreator =
+      [&Handler, &SM,
+       &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
+                  StringRef NewTypeText) -> std::optional<std::string> {
+    std::stringstream SS;
+
+    SS << getEndOfLine().str();
+    // Append: attr-name ret-type func-name "(" param-list ")" "{"
+    if (auto FDPrefix = getRangeText(
+            SourceRange(FD->getBeginLoc(), FD->getBody()->getBeginLoc()), SM,
+            LangOpts))
+      SS << getUnsafeBufferUsageAttributeText() << FDPrefix->str() << "{";
+    else
+      return std::nullopt;
+    // Append: "return" func-name "("
+    if (auto FunQualName = getFunNameText(FD, SM, LangOpts))
+      SS << "return " << FunQualName->str() << "(";
+    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");
+      if (i == ParmIdx)
+        // This is our spanified paramter!
+        SS << NewTypeText.str() << "(" << Parm->getIdentifier()->getName().str() << ", "
+           << Handler.getUserFillPlaceHolder("size") << ")";
+      else
+        SS << Parm->getIdentifier()->getName().str();
+      if (i != NumParms - 1)
+        SS << ", ";
+    }
+    // finish call and the body
+    SS << ");}" << getEndOfLine().str();
+    // FIXME: 80-char line formatting?
+    return SS.str();
+  };
+
+  FixItList FixIts{};
+  for (FunctionDecl *FReDecl : FD->redecls()) {
+    std::optional<SourceLocation> Loc = getPastLoc(FReDecl, SM, LangOpts);
+
+    if (!Loc)
+      return {};
+    if (FReDecl->isThisDeclarationADefinition()) {
+      assert(FReDecl == FD && "inconsistent function definition");
+      // Inserts a definition with the old signature to the end of
+      // `FReDecl`:
+      if (auto OldOverloadDef =
+              OldOverloadDefCreator(FReDecl, ParmIdx, NewTyText))
+        FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *OldOverloadDef));
+      else
+        return {}; // give up
+    } else {
+      // Adds the unsafe-buffer attribute (if not already there) to `FReDecl`:
+      if (!FReDecl->hasAttr<UnsafeBufferUsageAttr>()) {
+        FixIts.emplace_back(FixItHint::CreateInsertion(
+            FReDecl->getBeginLoc(), getUnsafeBufferUsageAttributeText()));
+      }
+      // Inserts a declaration with the new signature to the end of `FReDecl`:
+      if (auto NewOverloadDecl =
+              NewOverloadSignatureCreator(FReDecl, ParmIdx, NewTyText))
+        FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *NewOverloadDecl));
+      else
+        return {};
+    }
+  }
+  return FixIts;
+}
+
+// To fix a `ParmVarDecl` to be of `std::span` type.  In addition, creating a
+// new overload of the function so that the change is self-contained (see
+// `createOverloadsForFixedParams`).
+static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx,
+                                  UnsafeBufferUsageHandler &Handler) {
+  if (PVD->hasDefaultArg())
+    // FIXME: generate fix-its for default values:
+    return {};
+  assert(PVD->getType()->isPointerType());
+  auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
+
+  if (!FD)
+    return {};
+
+  std::optional<Qualifiers> PteTyQualifiers = std::nullopt;
+  std::optional<std::string> PteTyText = getPointeeTypeText(
+      PVD, Ctx.getSourceManager(), Ctx.getLangOpts(), &PteTyQualifiers);
+
+  if (!PteTyText)
+    return {};
+
+  std::optional<StringRef> PVDNameText = PVD->getIdentifier()->getName();
+
+  if (!PVDNameText)
+    return {};
+
+  std::string SpanOpen = "std::span<";
+  std::string SpanClose = ">";
+  std::string SpanTyText;
+  std::stringstream SS;
+
+  SS << SpanOpen << *PteTyText;
+  // Append qualifiers to span element type:
+  if (PteTyQualifiers)
+    SS << " " << PteTyQualifiers->getAsString();
+  SS << SpanClose;
+  // Save the Span type text:
+  SpanTyText = SS.str();
+  // Append qualifiers to the type of the parameter:
+  if (PVD->getType().hasQualifiers())
+    SS << " " << PVD->getType().getQualifiers().getAsString();
+  // Append parameter's name:
+  SS << " " << PVDNameText->str();
+
+  FixItList Fixes;
+  unsigned ParmIdx = 0;
+
+  Fixes.push_back(
+      FixItHint::CreateReplacement(PVD->getSourceRange(), SS.str()));
+  for (auto *ParmIter : FD->parameters()) {
+    if (PVD == ParmIter)
+      break;
+    ParmIdx++;
+  }
+  if (ParmIdx < FD->getNumParams())
+    if (auto OverloadFix = createOverloadsForFixedParams(ParmIdx, SpanTyText,
+                                                           FD, Ctx, Handler)) {
+      Fixes.append(*OverloadFix);
+      return Fixes;
+    }
+  return {};
+}
+
 static FixItList fixVariableWithSpan(const VarDecl *VD,
                                      const DeclUseTracker &Tracker,
                                      const ASTContext &Ctx,
                                      UnsafeBufferUsageHandler &Handler) {
   const DeclStmt *DS = Tracker.lookupDecl(VD);
   assert(DS && "Fixing non-local variables not implemented yet!");
   if (!DS->isSingleDecl()) {
     // FIXME: to support handling multiple `VarDecl`s in a single `DeclStmt`
     return {};
   }
   // Currently DS is an unused variable but we'll need it when
   // non-single decls are implemented, where the pointee type name
   // and the '*' are spread around the place.
   (void)DS;
 
   // FIXME: handle cases where DS has multiple declarations
   return fixVarDeclWithSpan(VD, Ctx, Handler.getUserFillPlaceHolder());
 }
 
-static FixItList fixVariable(const VarDecl *VD, Strategy::Kind K,
-                             const DeclUseTracker &Tracker,
-                             const ASTContext &Ctx,
-                             UnsafeBufferUsageHandler &Handler) {
+// TODO: we should be consistent to use `std::nullopt` to represent no-fix due
+// to any unexpected problem.
+static FixItList
+fixVariable(const VarDecl *VD, Strategy::Kind K,
+            /* The function decl under analysis */ const Decl *D,
+            const DeclUseTracker &Tracker, const ASTContext &Ctx,
+  if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
+    auto *FD = dyn_cast<clang::FunctionDecl>(PVD->getDeclContext());
+    if (!FD || FD != D)
+      // `FD != D` means that `PVD` belongs to a function that is not being
+      // analyzed currently.  Thus `FD` may not be complete.
+      return {};
+
+    // TODO If function has a try block we can't change params unless we check
+    // also its catch block for their use.
+    // FIXME We might support static class methods, some select methods,
+    // operators and possibly lamdas.
+    if (FD->isMain() || FD->isConstexpr() ||
+        FD->getTemplatedKind() != FunctionDecl::TemplatedKind::TK_NonTemplate ||
+        FD->isVariadic() ||
+        // also covers call-operator of lamdas
+        isa<CXXMethodDecl>(FD) ||
+        // skip when the function body is a try-block
+        (FD->hasBody() && isa<CXXTryStmt>(FD->getBody())) ||
+        FD->isOverloadedOperator())
+      return {}; // TODO test all these cases
+  }
+
   switch (K) {
   case Strategy::Kind::Span: {
-    if (VD->getType()->isPointerType() && VD->isLocalVarDecl())
+    if (VD->getType()->isPointerType()) {
+      if (const auto *PVD = dyn_cast<ParmVarDecl>(VD))
+        return fixParamWithSpan(PVD, Ctx, Handler);
+
+      if (VD->isLocalVarDecl())
         return fixVariableWithSpan(VD, Tracker, Ctx, Handler);
+    }
     return {};
   }
   case Strategy::Kind::Iterator:
   case Strategy::Kind::Array:
   case Strategy::Kind::Vector:
     llvm_unreachable("Strategy not implemented yet!");
   case Strategy::Kind::Wontfix:
     llvm_unreachable("Invalid strategy!");
if (!Fixits) {
       return true;
     }
   }
   return false;
 }
 
 static std::map<const VarDecl *, FixItList>
 getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
-        const DeclUseTracker &Tracker, const ASTContext &Ctx,
-        UnsafeBufferUsageHandler &Handler,
-        const DefMapTy &VarGrpMap) {
+    const ASTContext &Ctx,
+          /* The function decl under analysis */ const Decl *D,
+    const DeclUseTracker &Tracker, UnsafeBufferUsageHandler &Handler,
+    const DefMapTy &VarGrpMap) {
   std::map<const VarDecl *, FixItList> FixItsForVariable;
   for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) {
-    const Strategy::Kind ReplacementTypeForVD = S.lookup(VD);
     FixItsForVariable[VD] =
-        fixVariable(VD, ReplacementTypeForVD, Tracker, Ctx, Handler);
+        fixVariable(VD, S.lookup(VD), D, Tracker, Ctx, Handler);
     // If we fail to produce Fix-It for the declaration we have to skip the
     // variable entirely.
     if (FixItsForVariable[VD].empty()) {
       FixItsForVariable.erase(VD);
       continue;
     }
     bool ImpossibleToFix = false;
     llvm::SmallVector<FixItHint, 16> FixItsForVD;
for (auto VD : FixItsForVariable) {
     if (VarGroupForVD != VarGrpMap.end()) {
       for (const VarDecl * Var : VarGroupForVD->second) {
         if (Var == VD.first) {
           continue;
         }
         
         FixItList GroupFix;
         if (FixItsForVariable.find(Var) == FixItsForVariable.end()) {
-          GroupFix = fixVariable(Var, ReplacementTypeForVD, Tracker,
-                                           Var->getASTContext(), Handler);
+          GroupFix = fixVariable(Var, ReplacementTypeForVD, D,
+                                 Tracker, Var->getASTContext(), Handler);
         } else {
           GroupFix = FixItsForVariable[Var];
         }
         
         for (auto Fix : GroupFix) {
           FixItsForVariable[VD.first].push_back(Fix);
         }
       }
void clang::checkUnsafeBufferUsage(const Decl *D,
   FixablesForAllVars = groupFixablesByVar(std::move(FixableGadgets));
 
   std::map<const VarDecl *, FixItList> FixItsForVariableGroup;
   DefMapTy VariableGroupsMap{};
 
   // Filter out non-local vars and vars with unclaimed DeclRefExpr-s.
   for (auto it = FixablesForAllVars.byVar.cbegin();
        it != FixablesForAllVars.byVar.cend();) {
-    // FIXME: Support ParmVarDecl as well.
-    if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) {
+      // FIXME: need to deal with global variables later
+      if ((!it->first->isLocalVarDecl() && !isa<ParmVarDecl>(it->first)) ||
+          Tracker.hasUnclaimedUses(it->first)) {
       it = FixablesForAllVars.byVar.erase(it);
     } else {
       ++it;
     }
   }
 
   llvm::SmallVector<const VarDecl *, 16> UnsafeVars;
   for (const auto &[VD, ignore] : FixablesForAllVars.byVar)
if (VisitedVars.find(Var) == VisitedVars.end()) {
         if (UnsafeOps.byVar.find(V) != UnsafeOps.byVar.end()) {
           VariableGroupsMap[V] = VarGroup;
         }
       }
     }
   }
 
   Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
-  FixItsForVariableGroup = getFixIts(FixablesForAllVars, NaiveStrategy, Tracker,
-                                D->getASTContext(), Handler, VariableGroupsMap);
+
+  FixItsForVariableGroup =
+      getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D,
+                Tracker, Handler, VariableGroupsMap);
 
   // FIXME Detect overlapping FixIts.
 
   for (const auto &G : UnsafeOps.noVar) {
     Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false);
   }
 
   for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) {

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

+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fcxx-exceptions  -fsafe-buffer-usage-suggestions -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fcxx-exceptions -fdiagnostics-parseable-fixits  -fsafe-buffer-usage-suggestions %s 2>&1 | FileCheck %s
+
+// We do not fix parameters of the `main` function
+
+// CHECK-NOT: fix-it:
+
+// main function
+int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe pointer used for buffer access}}
+  char tmp;
+  tmp = argv[5][5];                // expected-note{{used in buffer access here}} \
+              expected-warning{{unsafe buffer access}}
+}

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

+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions -include %s %s 2>&1 | FileCheck %s
+
+// We cannot deal with overload conflicts for now so NO fix-it to
+// function parameters will be emitted if there are overloads for that
+// function.
+
+#ifndef INCLUDE_ME
+#define INCLUDE_ME
+
+void baz();
+
+#else
+
+
+void foo(int *p, int * q);
+
+void foo(int *p);
+
+void foo(int *p) {
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  int tmp;
+  tmp = p[5];
+}
+
+// an overload declaration of `bar(int *)` appears after it
+void bar(int *p) {
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  int tmp;
+  tmp = p[5];
+}
+
+void bar();
+
+// an overload declaration of `baz(int)` appears is included
+void baz(int *p) {
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  int tmp;
+  tmp = p[5];
+}
+
+namespace NS {
+  // `NS::foo` is a distinct function from `foo`, so it has no
+  // overload and shall be fixed.
+  void foo(int *p) {
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:18}:"std::span<int> p"
+    int tmp;
+    tmp = p[5];
+  }
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid foo(int *p) {return foo(std::span<int>(p, <# size #>));}\n"
+
+  // Similarly, `NS::bar` is distinct from `bar`:
+  void bar(int *p);
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:3}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid bar(std::span<int> p)"
+} // end of namespace NS
+
+// This is the implementation of `NS::bar`, which shall be fixed.
+void NS::bar(int *p) {
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:20}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS::bar(int *p) {return NS::bar(std::span<int>(p, <# size #>));}\n"
+
+namespace NESTED {
+  void alpha(int);
+  void beta(int *, int *);
+
+  namespace INNER {
+    // `NESTED::INNER::alpha` is distinct from `NESTED::alpha`, so it
+    // has no overload and shall be fixed.
+    void alpha(int *p) {
+      // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:22}:"std::span<int> p"
+      int tmp;
+      tmp = p[5];
+    }
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:6}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid alpha(int *p) {return alpha(std::span<int>(p, <# size #>));}\n"
+  }
+}
+
+namespace NESTED {
+  // There is an `NESTED::beta(int *, int *)` declared above, so this
+  // unsafe function will not be fixed.
+  void beta(int *p) {
+    // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]:
+    int tmp;
+    tmp = p[5];
+  }
+
+  namespace INNER {
+    void delta(int);  // No fix for `NESTED::INNER::delta`
+    void delta(int*);
+  }
+}
+
+// There is an `NESTED::beta(int *)` declared above, so this unsafe
+// function will not be fixed.
+void NESTED::beta(int *p, int *q) {
+  // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]:
+  int tmp;
+  tmp = p[5];
+  tmp = q[5];
+}
+
+void NESTED::INNER::delta(int * p) {
+  // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]:
+  int tmp;
+  tmp = p[5];
+}
+
+
+#endif

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

+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions %s 2>&1 | FileCheck %s
+
+namespace NS1 {
+  void foo(int *);
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+  // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:18-[[@LINE-2]]:18}:";\nvoid foo(std::span<int>)"
+  namespace NS2 {
+    void foo(int *);
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+    // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:";\nvoid foo(std::span<int>)"
+    namespace NS3 {
+      void foo(int *);
+      // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:7-[[@LINE-1]]:7}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+      // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:22-[[@LINE-2]]:22}:";\nvoid foo(std::span<int>)"
+    }
+  }
+
+  typedef int MyType;
+}
+
+void NS1::foo(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:15-[[@LINE-1]]:21}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS1::foo(int *p) {return NS1::foo(std::span<int>(p, <# size #>));}\n"
+
+void NS1::NS2::foo(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:20-[[@LINE-1]]:26}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS1::NS2::foo(int *p) {return NS1::NS2::foo(std::span<int>(p, <# size #>));}\n"
+
+void NS1::NS2::NS3::foo(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:31}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS1::NS2::NS3::foo(int *p) {return NS1::NS2::NS3::foo(std::span<int>(p, <# size #>));}\n"
+
+
+void f(NS1::MyType * x) {
+  // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:8-[[@LINE-1]]:23}:"std::span<NS1::MyType> x"
+  NS1::MyType tmp;
+  tmp = x[5];
+}
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid f(NS1::MyType * x) {return f(std::span<NS1::MyType>(x, <# size #>));}\n"

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

+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions -include %s %s 2>&1 | FileCheck %s
+
+// TODO test if there's not a single character in the file after a decl or def
+
+#ifndef INCLUDE_ME
+#define INCLUDE_ME
+
+void simple(int *p);
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:";\nvoid simple(std::span<int> p)"
+
+#else
+
+void simple(int *);
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid simple(std::span<int>)"
+
+void simple(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:13-[[@LINE-1]]:19}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid simple(int *p) {return simple(std::span<int>(p, <# size #>));}\n"
+
+
+void twoParms(int *p, int * q) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:15-[[@LINE-1]]:21}:"std::span<int> p"
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:23-[[@LINE-2]]:30}:"std::span<int> q"
+  int tmp;
+  tmp = p[5] + q[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid twoParms(int *p, int * q) {return twoParms(std::span<int>(p, <# size #>), q);}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:2-[[@LINE-2]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid twoParms(int *p, int * q) {return twoParms(p, std::span<int>(q, <# size #>));}\n"
+
+void ptrToConst(const int * x) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:30}:"std::span<int const> x"
+  int tmp = x[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid ptrToConst(const int * x) {return ptrToConst(std::span<int const>(x, <# size #>));}\n"
+
+// The followings test cases where multiple FileIDs maybe involved
+// when the analyzer loads characters from source files.
+
+#define FUN_NAME(x) _##x##_
+
+// The analyzer reads `void FUNNAME(macro_defined_name)(` from the
+// source file.  The MACRO and this source file have different
+// FileIDs.
+void FUN_NAME(macro_defined_name)(int * x) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:35-[[@LINE-1]]:42}:"std::span<int> x"
+  int tmp = x[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid FUN_NAME(macro_defined_name)(int * x) {return FUN_NAME(macro_defined_name)(std::span<int>(x, <# size #>));}\n"
+
+
+// The followings test various type specifiers
+namespace {
+  void simpleSpecifier(unsigned long long int *p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:24-[[@LINE-1]]:49}:"std::span<unsigned long long int> p"
+    auto tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid simpleSpecifier(unsigned long long int *p) {return simpleSpecifier(std::span<unsigned long long int>(p, <# size #>));}\n"
+
+  void attrParm([[maybe_unused]] int * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:34-[[@LINE-1]]:41}:"std::span<int> p"
+    int tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid attrParm({{\[}}{{\[}}maybe_unused{{\]}}{{\]}} int * p) {return attrParm(std::span<int>(p, <# size #>));}\n"
+
+  using T = unsigned long long int;
+
+  void usingTypenameSpecifier(T * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:31-[[@LINE-1]]:36}:"std::span<T> p"
+    int tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid usingTypenameSpecifier(T * p) {return usingTypenameSpecifier(std::span<T>(p, <# size #>));}\n"
+
+  typedef unsigned long long int T2;
+
+  void typedefSpecifier(T2 * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:31}:"std::span<T2> p"
+    int tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid typedefSpecifier(T2 * p) {return typedefSpecifier(std::span<T2>(p, <# size #>));}\n"
+
+  class SomeClass {
+  } C;
+
+  void classTypeSpecifier(const class SomeClass * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:27-[[@LINE-1]]:52}:"std::span<class SomeClass const> p"
+    if (++p) {}
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid classTypeSpecifier(const class SomeClass * p) {return classTypeSpecifier(std::span<class SomeClass const>(p, <# size #>));}\n"
+
+  struct {
+    // anon
+  } ANON_S;
+
+  struct MyStruct {
+    // namned
+  } NAMED_S;
+
+  // FIXME: `decltype(ANON_S)` represents an unnamed type but it can
+  // be referred as "`decltype(ANON_S)`", so the analysis should
+  // fix-it.
+  void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,
+                         decltype(NAMED_S) ** rr) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:26-[[@LINE-2]]:41}:"std::span<decltype(C)> p"
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:65-[[@LINE-3]]:86}:"std::span<decltype(NAMED_S)> r"
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:26-[[@LINE-3]]:49}:"std::span<decltype(NAMED_S) *> rr"
+    if (++p) {}
+    if (++q) {}
+    if (++r) {}
+    if (++rr) {}
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(std::span<decltype(C)>(p, <# size #>), q, r, rr);}\n
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, std::span<decltype(NAMED_S)>(r, <# size #>), rr);}\n"
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:4-[[@LINE-3]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, r, std::span<decltype(NAMED_S) *>(rr, <# size #>));}\n"
+
+#define MACRO_TYPE(T) long T
+
+  void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:18-[[@LINE-1]]:46}:"std::span<unsigned MACRO_TYPE(int)> p"
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:48-[[@LINE-2]]:77}:"std::span<unsigned MACRO_TYPE(long)> q"
+    int tmp = p[5];
+    tmp = q[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(std::span<unsigned MACRO_TYPE(int)>(p, <# size #>), q);}\n"
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(p, std::span<unsigned MACRO_TYPE(long)>(q, <# size #>));}\n"
+}
+
+// The followings test various declarators:
+void decayedArray(int a[]) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:19-[[@LINE-1]]:26}:"std::span<int> a"
+  int tmp;
+  tmp = a[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decayedArray(int a[]) {return decayedArray(std::span<int>(a, <# size #>));}\n"
+
+void decayedArrayOfArray(int a[10][10]) {
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+  if (++a){}
+}
+
+void complexDeclarator(int * (*a[10])[10]) {
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+  if (++a){}
+}
+
+// Make sure we do not generate fixes for the following cases:
+
+#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){}
+}
+
+#endif

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

+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fcxx-exceptions -fsafe-buffer-usage-suggestions -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fcxx-exceptions -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions %s 2>&1 | FileCheck %s
+
+void const_ptr(int * const x) { // expected-warning{{'x' is an unsafe pointer used for buffer access}} expected-note{{change type of 'x' to 'std::span' to preserve bounds information}}
+  // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:29}:"std::span<int> const x"
+  int tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid const_ptr(int * const x) {return const_ptr(std::span<int>(x, <# size #>));}\n"
+
+void const_ptr_to_const(const int * const x) {// expected-warning{{'x' is an unsafe pointer used for buffer access}} expected-note{{change type of 'x' to 'std::span' to preserve bounds information}}
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:44}:"std::span<int const> const x"
+  int tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid const_ptr_to_const(const int * const x) {return const_ptr_to_const(std::span<int const>(x, <# size #>));}\n"
+
+typedef struct {int x;} NAMED_UNNAMED_STRUCT; // an unnamed struct type named by a typedef
+typedef struct {int x;} * PTR_TO_ANON;        // pointer to an unnamed struct
+typedef NAMED_UNNAMED_STRUCT * PTR_TO_NAMED;  // pointer to a named type
+
+// We can fix a pointer to a named type
+void namedPointeeType(NAMED_UNNAMED_STRUCT * p) {  // expected-warning{{'p' is an unsafe pointer used for buffer access}}\ expected-note{{change type of 'p' to 'std::span' to preserve bounds information}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:23-[[@LINE-1]]:47}:"std::span<NAMED_UNNAMED_STRUCT> p"
+  if (++p) {  // expected-note{{used in pointer arithmetic here}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+  }
+}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid namedPointeeType(NAMED_UNNAMED_STRUCT * p) {return namedPointeeType(std::span<NAMED_UNNAMED_STRUCT>(p, <# size #>));}\n"
+
+// We CANNOT fix a pointer to an unnamed type
+// CHECK-NOT: fix-it:
+void unnamedPointeeType(PTR_TO_ANON p) {  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  if (++p) {  // expected-note{{used in pointer arithmetic here}}
+  }
+}
+
+// We do not fix parameters participating unsafe operations for the
+// following functions/methods or function-like expressions:
+
+// CHECK-NOT: fix-it:
+class A {
+  // constructor & descructor
+  A(int * p) {  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+    int tmp;
+    tmp = p[5]; // expected-note{{used in buffer access here}}
+  }
+
+  // class member methods
+  void foo(int *p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+    int tmp;
+    tmp = p[5];      // expected-note{{used in buffer access here}}
+  }
+
+  // overload operator
+  int operator+(int * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+    int tmp;
+    tmp = p[5];            // expected-note{{used in buffer access here}}
+    return tmp;
+  }
+};
+
+// lambdas
+void foo() {
+  auto Lamb = [&](int *p) // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+    -> int {
+    int tmp;
+    tmp = p[5];           // expected-note{{used in buffer access here}}
+    return tmp;
+  };
+}
+
+// template
+template<typename T>
+void template_foo(T * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  T tmp;
+  tmp = p[5];              // expected-note{{used in buffer access here}}
+}
+
+void instantiate_template_foo() {
+  int * p;
+  template_foo(p);        // FIXME expected note {{in instantiation of function template specialization 'template_foo<int>' requested here}}
+}
+
+// variadic function
+void vararg_foo(int * p...) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  int tmp;
+  tmp = p[5];                 // expected-note{{used in buffer access here}}
+}
+
+// constexpr functions
+constexpr int constexpr_foo(int * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  return p[5];                         // expected-note{{used in buffer access here}}
+}
+
+// function body is a try-block
+void fn_with_try_block(int* p)    // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  try {
+    int tmp;
+
+    if (p == nullptr)
+      throw 42;
+    tmp = p[5];                   // expected-note{{used in buffer access here}}
+  }
+  catch (int) {
+    *p = 0;
+  }
+
+// The following two unsupported cases are not specific to
+// parm-fixits. Adding them here in case they get forgotten.
+void isArrayDecayToPointerUPC(int a[][10], int (*b)[10]) {
+// expected-warning@-1{{'a' is an unsafe pointer used for buffer access}}
+// expected-warning@-2{{'b' is an unsafe pointer used for buffer access}}
+  int tmp;
+
+  tmp = a[5][5] + b[5][5];  // expected-warning2{{unsafe buffer access}}  expected-note2{{used in buffer access here}}
+}
+
+// parameter having default values:
+void parmWithDefaultValue(int * x = 0) {
+  // expected-warning@-1{{'x' is an unsafe pointer used for buffer access}}
+  int tmp;
+  tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+
+void parmWithDefaultValueDecl(int * x = 0);
+
+void parmWithDefaultValueDecl(int * x) {
+  // expected-warning@-1{{'x' is an unsafe pointer used for buffer access}}
+  int tmp;
+  tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+

clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

void testIncrement(char *p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   p++; // expected-note{{used in pointer arithmetic here}}
   --p; // expected-note{{used in pointer arithmetic here}}
   p--; // expected-note{{used in pointer arithmetic here}}
 }
 
 void * voidPtrCall(void);
 char * charPtrCall(void);
 
-void testArraySubscripts(int *p, int **pp) {
+void testArraySubscripts(int *p, int **pp) { // expected-note{{change type of 'pp' to 'std::span' to preserve bounds information}}
 // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}}
 // expected-warning@-2{{'pp' is an unsafe pointer used for buffer access}}
   foo(p[1],             // expected-note{{used in buffer access here}}
       pp[1][1],         // expected-note{{used in buffer access here}}
                         // expected-warning@-1{{unsafe buffer access}}
       1[1[pp]],         // expected-note{{used in buffer access here}}
                         // expected-warning@-1{{unsafe buffer access}}
       1[pp][1]          // expected-note{{used in buffer access here}}
foo(ap3[1][1]); // expected-note{{used in buffer access here}}
                   // expected-warning@-1{{unsafe buffer access}}
 
   auto ap4 = *pp; // expected-warning{{'ap4' is an unsafe pointer used for buffer access}} \
            expected-note{{change type of 'ap4' to 'std::span' to preserve bounds information}}
 
   foo(ap4[1]);    // expected-note{{used in buffer access here}}
 }
 
+void testUnevaluatedContext(int * p) {// no-warning
+  foo(sizeof(p[1]),             // no-warning
+      sizeof(decltype(p[1])));  // no-warning
+}
+
+// expected-note@+1{{change type of 'a' to 'std::span' to preserve bounds information}}
 void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) {
   // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}}
   // expected-warning@-2{{'q' is an unsafe pointer used for buffer access}}
   // expected-warning@-3{{'a' is an unsafe pointer used for buffer access}}
   // expected-warning@-4{{'b' is an unsafe pointer used for buffer access}}
 
   foo(p[1], 1[p], p[-1],    // expected-note3{{used in buffer access here}}
       q[1], 1[q], q[-1],    // expected-note3{{used in buffer access here}}
template<typename T> void fArr(T t[]) {
   // expected-warning@-1{{'t' is an unsafe pointer used for buffer access}}
   foo(t[1]);    // expected-note{{used in buffer access here}}
   T ar[8];      // expected-warning{{'ar' is an unsafe buffer that does not perform bounds checks}}
   foo(ar[5]);   // expected-note{{used in buffer access here}}
 }
 
 template void fArr<int>(int t[]); // FIXME: expected note {{in instantiation of}}
 
-int testReturn(int t[]) {
+int testReturn(int t[]) {// expected-note{{change type of 't' to 'std::span' to preserve bounds information}}
   // expected-warning@-1{{'t' is an unsafe pointer used for buffer access}}
   return t[1]; // expected-note{{used in buffer access here}}
 }
 
 int testArrayAccesses(int n) {
     // auto deduced array type
     int cArr[2][3] = {{1, 2, 3}, {4, 5, 6}};
     // expected-warning@-1{{'cArr' is an unsafe buffer that does not perform bounds checks}}