[-Wunsafe-buffer-usage] Use `Strategy` to determine whether to fix a parameter

[ziqingluo-90]

Summary

• Use Strategy to determine whether to fix a parameter.
• Fix the Strategy construction so that only variables on the graph are assigned the std::span strategy
• Extend PointerAssignmentGadget and PointerInitGadget to generate fix-its for cases where the left-hand side has won't fix strategy and the right-hand side has std::span strategy.

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1637–1638

NFC. Just noticed that getPastLoc is the better function to call here.

2743

VisitedVars are the set of variables on the computed graph. The previous UnsafeVars is a superset of it, including safe variables that have FixableGadgets discovered. We do not want to assign strategies other than Won't fix to those safe variables.

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

30

Both b and a will be changed to have std::span type. So this assignment needs no fix-it. Same reason applies to similar changes in the rest of this test file.

80

y does not have to be changed to a safe-type while x does. So we need to fix this assignment. Same reason applies to similar changes in the rest of this test file.

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp

29

We now support to fix int *r = q. So all the unsafe variables can be fixed.

[NoQ]

Ooo nice!!

Extend PointerAssignmentGadget and PointerInitGadget to generate fix-its for cases where the left-hand side has won't fix strategy and the right-hand side has std::span strategy.

Hmm, is this intertwined with other changes, or is this completely separate? Also my head appears to be blank; did we reach any conclusion about the nature of "single" RHS-es and usefulness of annotating them as such? Which could be an approach that would encourage people to propagate more bounds in the backwards direction by default.

clang/lib/Analysis/UnsafeBufferUsage.cpp

1573

MatchFinder::MatchResult.Context is what you're probably looking for, so it's already kinda passed in. You can stash it in a member variable in the base class Gadget and have it readily available everywhere, i.e.

 class Gadget {
 public:
-  Gadget(Kind K) : K(K) {}
+  Gadget(Kind K, const MatchResult &Result) : K(K), Context(*Result.Context) {
+    assert(Context);
+  }

 private:
   Kind K;
+  const ASTContext &Context;
 };

 class FixableGadget : public Gadget {
 public:
-  FixableGadget(Kind K) : Gadget(K) {}
+  FixableGadget(Kind K, const MatchResult &Result) : Gadget(K, Result) {}
 };

 class PointerAssignmentGadget {
   PointerAssignmentGadget(const MatchResult &Result)
-      : FixableGadget(Kind::PointerAssignment),
+      : FixableGadget(Kind::PointerAssignment, Result),
     PtrLHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerAssignLHSTag)),
     PtrRHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerAssignRHSTag)) {}
 };

(the change in individual gadget classes is really tiny!)

2712–2738

This entire loop can probably go away as soon as we stop relying on FixablesForAllVars and treat groups separately.

Then we can probably even have different Strategy objects for different groups, so getNaiveStrategy() could also be invoked per-group?

2753

I guess the next step is to call this function separately for each group?

[ziqingluo-90]

Extend PointerAssignmentGadget and PointerInitGadget to generate fix-its for cases where the left-hand side
has won't fix strategy and the right-hand side has std::span strategy.

Hmm, is this intertwined with other changes, or is this completely separate? Also my head appears to be blank;

The two FixableGadgets previously gave incorrect fix-its for the cases where only RHS need fix. After the change made to Strategy, they returned std::nullopt as the case was not implemented.
So to avoid regression, I made the extension to the two Gadgets.

did we reach any conclusion about the nature of "single" RHS-es and usefulness of annotating them as such?
Which could be an approach that would encourage people to propagate more bounds in the backwards direction by default.

Can you elaborate the question a bit more?

[t-rasmud]

clang/lib/Analysis/UnsafeBufferUsage.cpp

2743

Would it make more sense (make the code more readable) if we renamed UnsafeVars to say, FixableVars?

[NoQ]

In D157441#4575220, @ziqingluo-90 wrote:

Extend PointerAssignmentGadget and PointerInitGadget to generate fix-its for cases where the left-hand side
has won't fix strategy and the right-hand side has std::span strategy.

Hmm, is this intertwined with other changes, or is this completely separate? Also my head appears to be blank;

The two FixableGadgets previously gave incorrect fix-its for the cases where only RHS need fix. After the change made to Strategy, they returned std::nullopt as the case was not implemented.
So to avoid regression, I made the extension to the two Gadgets.

did we reach any conclusion about the nature of "single" RHS-es and usefulness of annotating them as such?
Which could be an approach that would encourage people to propagate more bounds in the backwards direction by default.

Can you elaborate the question a bit more?

If previously we were emitting an incorrect fixit and now we're not emitting any fixit, then I think it's a very good improvement, not a regression 🤔

Uhh, I meant LHSes, sorry 😅

Anyway, the point was that it's hard to tell whether .data() is the right fix, so I think we wanted to hold off implementing it, because we probably want to do something better instead in most cases. Like, implement a strategy discovery procedure that would give us a better idea of how exactly is the LHS pointer used safely. Then we might emit different fixes based on that. So I suspect that it should be a separate effort, so it probably makes sense to separate it into a different patch and think more about it.

[ziqingluo-90]

In our offline discussion, we realized that p = q.data() is not always a correct fix to p = q for cases where only q is an unsafe pointer. It depends on whether p is being dereferenced later. So I removed my changes to PointerAssignmentGadget and PointerInitGadget.

Note there are many tests adjusted due to the fact that now p = q cannot be fixed if only q is unsafe (previously incorrect fix-it was provided, so it is not a regression).

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

2587

We cannot fix reference type variable declarations for now.
Besides, it seems that reference type is invisible to Exprs. For example,

void foo(int * &p) {
      p[5];  // the type of the DRE `p` is a pointer-to-int instead of a reference
}

So we ignore such variables explicitly here.

2743

Oh, I just realized that UnsafeVars is now an unused variable. So let's remove it. FixableVars would be a better name for sure, since it is in fact the key set of FixablesForAllVars.byVar.

2743

To correct myself, VisitedVars are the variables In the graph but it may not be a subset of "fixable variables". VisitedVars may contain variables that is not fixable, i.e., variables having no FixableGadget associated with. We should leave those unfixable variables being WON'T FIX.

[NoQ]

Looks amazing now. Thanks for splitting it up and fighting hard against complexity!

clang/lib/Analysis/UnsafeBufferUsage.cpp

2587

Ooo you're one of today's lucky 10,000! (More like today's 0.01, I doubt there are many C++ programmers who are aware of this quirk, it's a very niche compiler gotcha.)

That's right, expressions in C++ can't have reference types. They have "value kinds" instead (lvalue, rvalue, etc) which are more fundamental (not even necessarily C++-specific). As far as I understand, from language formalism point of view, references aren't values per se; they're just names given to other values (glvalues, to be exact). There's no separate operation of "reference dereference" in C++ formalism; "undereferenced reference" expressions don't exist in the first place.

So in this example:

int x = 5;
int &y = x;
int a = x + y;

not only the type of DeclRefExpr y is int, but also the *value* of the DeclRefExpr y is the glvalue named x. And the value of DeclRefExpr x is *also* the glvalue named x. From the point of view of C++ formalism, there's no separate location in memory named y. The declaration y simply declares a different way to refer to x.

This formalism is very different from pointers, where in

int x = 5;
int *y = &x;
int a = x + *y;

you're allowed to assume that there's a separate memory location named y where the address of memory location named x is written. You can further take the address of that location and put it into a third location named w by writing int **z = &y;. You can do that indefinitely, and then roll it all back with sufficient amount of *s. You can also reassign your pointers, eg. y = &a would overwrite the data at location y.

You cannot do any of that with references. You cannot reassign a reference. You cannot take a reference to a reference; that's the same as a reference to the original lvalue, both simply become names given to that actual value. Similarly, the language doesn't let you take an address of the location in which the reference is stored; it doesn't even allow you to assume that such location exists.

Sure you can "convert a reference to a pointer" with the help of the operator &, but that's just taking address of an lvalue it refers to. This doesn't mean that the reference itself "is" an address, it just means that the lvalue itself is technically still just an address. References can often be implemented as pointers by the codegen, eg. reference-type fields in classes or reference-type return values are probably just pointers "under the hood". Conversely, in case of pointers the actual location for y and for z could have also been optimized away by the optimizer, causing the pointers to "act like references" "under the hood": from CodeGen perspective they're just a different way to refer to x. But the purpose of this formalism isn't to describe what happens under the hood; it's just to define what operations are allowed on which expressions. And for that very simple and limited purpose, references don't act like values, and expressions of reference type don't really make sense, which is what we observe in the AST.

(None of this has anything to do with the patch, I just hope this helps you approach this problem in the future.)

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

2587

Thank you for explaining this! You just provided me a brand new view on C++ references . "expressions of reference type don't really make sense" could have confused me but now it makes perfect sense with your explanation.

clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h

public:
   /// Returns the set of variables (including `Var`) that need to be fixed
   /// together in one step.
   ///
   /// `Var` must be a variable that needs fix (so it must be in a group).
   /// `HasParm` is an optional argument that will be set to true if the set of
   /// variables, where `Var` is in, contains parameters.
   virtual VarGrpRef getGroupOfVar(const VarDecl *Var,
                                   bool *HasParm = nullptr) const =0;
+
+  /// Returns the non-empty group of variables that include parameters of the
+  /// analyzing function, if such a group exists.  An empty group, otherwise.
+  virtual VarGrpRef getGroupOfParms() const =0;
 };
 
 /// The interface that lets the caller handle unsafe buffer usage analysis
 /// results by overriding this class's handle... methods.
 class UnsafeBufferUsageHandler {
 #ifndef NDEBUG
 public:
   // A self-debugging facility that you can use to notify the user when

clang/lib/Analysis/UnsafeBufferUsage.cpp

DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
 
   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))
       if (ConstVal->isNegative())
         return std::nullopt;
 
     // note that the expr may (oddly) has multiple layers of parens
     // example:
     //   *((..(pointer + 123)..))
     // goal:
     //   pointer[123]
     // Fix-It:
     //   remove '*('
     //   replace ' + ' with '['
     //   replace ')' with ']'
PointerDereferenceGadget::getFixits(const Strategy &S) const {
   case Strategy::Kind::Span: {
     ASTContext &Ctx = VD->getASTContext();
     SourceManager &SM = Ctx.getSourceManager();
     // Required changes: *(ptr); => (ptr[0]); and *ptr; => ptr[0]
     // Deletes the *operand
     CharSourceRange derefRange = clang::CharSourceRange::getCharRange(
         Op->getBeginLoc(), Op->getBeginLoc().getLocWithOffset(1));
     // Inserts the [0]
-    std::optional<SourceLocation> EndOfOperand =
-        getEndCharLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts());
-    if (EndOfOperand) {
+    if (auto LocPastOperand =
+            getPastLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts())) {
       return FixItList{{FixItHint::CreateRemoval(derefRange),
-                        FixItHint::CreateInsertion(
-                            (*EndOfOperand).getLocWithOffset(1), "[0]")}};
+      FixItHint::CreateInsertion(*LocPastOperand, "[0]")}};
     }
     break;
   }
   case Strategy::Kind::Iterator:
   case Strategy::Kind::Array:
   case Strategy::Kind::Vector:
     llvm_unreachable("Strategy not implemented yet!");
   case Strategy::Kind::Wontfix:

 // 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
-//
-// `ParmsMask` is an array of size of `FD->getNumParams()` such
-// that `ParmsMask[i]` is true iff the `i`-th parameter will be fixed with some
-// strategy.
 static std::optional<FixItList>
-createOverloadsForFixedParams(const std::vector<bool> &ParmsMask, const Strategy &S,
-                              const FunctionDecl *FD, const ASTContext &Ctx,
+createOverloadsForFixedParams(const Strategy &S, 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();
   const unsigned NumParms = FD->getNumParams();
   std::vector<std::string> NewTysTexts(NumParms);
+  std::vector<bool> ParmsMask(NumParms, false);
+  bool AtLeastOneParmToFix = false;
 
   for (unsigned i = 0; i < NumParms; i++) {
-    if (!ParmsMask[i])
+    const ParmVarDecl *PVD = FD->getParamDecl(i);
+
+    if (S.lookup(PVD) == Strategy::Kind::Wontfix)
       continue;
+    if (S.lookup(PVD) != Strategy::Kind::Span)
+      // Not supported, not suppose to happen:
+      return std::nullopt;
 
     std::optional<Qualifiers> PteTyQuals = std::nullopt;
     std::optional<std::string> PteTyText =
-        getPointeeTypeText(FD->getParamDecl(i), SM, LangOpts, &PteTyQuals);
+        getPointeeTypeText(PVD, SM, LangOpts, &PteTyQuals);
 
     if (!PteTyText)
       // something wrong in obtaining the text of the pointee type, give up
       return std::nullopt;
     // FIXME: whether we should create std::span type depends on the Strategy.
     NewTysTexts[i] = getSpanTypeText(*PteTyText, PteTyQuals);
+    ParmsMask[i] = true;
+    AtLeastOneParmToFix = true;
   }
+  if (!AtLeastOneParmToFix)
+    // No need to create function overloads:
+    return {};
   // FIXME Respect indentation of the original code.
 
   // A lambda that creates the text representation of a function declaration
   // with the new type signatures:
   const auto NewOverloadSignatureCreator =
       [&SM, &LangOpts, &NewTysTexts,
        &ParmsMask](const FunctionDecl *FD) -> std::optional<std::string> {
     std::stringstream SS;

 // NOTE: In case `D`'s parameters will be changed but bounds-safe function
 // overloads cannot created, the whole group that contains the parameters will
 // be erased from `FixItsForVariable`.
 static FixItList createFunctionOverloadsForParms(
     std::map<const VarDecl *, FixItList> &FixItsForVariable /* mutable */,
     const VariableGroupsManager &VarGrpMgr, const FunctionDecl *FD,
     const Strategy &S, ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) {
   FixItList FixItsSharedByParms{};
-  std::vector<bool> ParmsNeedFixMask(FD->getNumParams(), false);
-  const VarDecl *FirstParmNeedsFix = nullptr;
 
-  for (unsigned i = 0; i < FD->getNumParams(); i++)
-    if (FixItsForVariable.count(FD->getParamDecl(i))) {
-      ParmsNeedFixMask[i] = true;
-      FirstParmNeedsFix = FD->getParamDecl(i);
-    }
-  if (FirstParmNeedsFix) {
-    // In case at least one parameter needs to be fixed:
-    std::optional<FixItList> OverloadFixes =
-        createOverloadsForFixedParams(ParmsNeedFixMask, S, FD, Ctx, Handler);
+      createOverloadsForFixedParams(S, FD, Ctx, Handler);
 
-    if (OverloadFixes) {
-      FixItsSharedByParms.append(*OverloadFixes);
-    } else {
-      // Something wrong in generating `OverloadFixes`, need to remove the
-      // whole group, where parameters are in, from `FixItsForVariable` (Note
-      // that all parameters should be in the same group):
-      for (auto *Member : VarGrpMgr.getGroupOfVar(FirstParmNeedsFix))
-        FixItsForVariable.erase(Member);
-    }
-  }
+    for (auto *Member : VarGrpMgr.getGroupOfParms())
   return FixItsSharedByParms;
 }
 
 // Constructs self-contained fix-its for each variable in `FixablesForAllVars`.
 static std::map<const VarDecl *, FixItList>
 getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
     ASTContext &Ctx,
           /* The function decl under analysis */ const Decl *D,
for (auto Iter = FinalFixItsForVariable.begin();
     if (overlapWithMacro(Iter->second) ||
         clang::internal::anyConflict(Iter->second, Ctx.getSourceManager())) {
       Iter = FinalFixItsForVariable.erase(Iter);
     } else
       Iter++;
   return FinalFixItsForVariable;
 }
 
+template <typename VarDeclIterTy>
 static Strategy
-getNaiveStrategy(const llvm::SmallVectorImpl<const VarDecl *> &UnsafeVars) {
+getNaiveStrategy(llvm::iterator_range<VarDeclIterTy> UnsafeVars) {
   Strategy S;
   for (const VarDecl *VD : UnsafeVars) {
     S.set(VD, Strategy::Kind::Span);
   }
   return S;
 }
 
 //  Manages variable groups:
if (HasParm)
       *HasParm = false;
 
     auto It = VarGrpMap.find(Var);
 
     if (It == VarGrpMap.end())
       return std::nullopt;
     return Groups[It->second];
   }
+
+  VarGrpRef getGroupOfParms() const override {
+    return GrpsUnionForParms.getArrayRef();
+  }
 };
 
 void clang::checkUnsafeBufferUsage(const Decl *D,
                                    UnsafeBufferUsageHandler &Handler,
                                    bool EmitSuggestions) {
 #ifndef NDEBUG
   Handler.clearDebugNotes();
 #endif
for (auto it = FixablesForAllVars.byVar.cbegin();
       if ((!it->first->isLocalVarDecl() && !isa<ParmVarDecl>(it->first))) {
 #ifndef NDEBUG
           Handler.addDebugNoteForVar(
               it->first, it->first->getBeginLoc(),
               ("failed to produce fixit for '" + it->first->getNameAsString() +
                "' : neither local nor a parameter"));
 #endif
         it = FixablesForAllVars.byVar.erase(it);
+      } else if (it->first->getType().getCanonicalType()->isReferenceType()) {
+#ifndef NDEBUG
+        Handler.addDebugNoteForVar(it->first, it->first->getBeginLoc(),
+                                   ("failed to produce fixit for '" +
+                                    it->first->getNameAsString() +
+                                    "' : has a reference type"));
+#endif
+        it = FixablesForAllVars.byVar.erase(it);
       } else if (Tracker.hasUnclaimedUses(it->first)) {
 #ifndef NDEBUG
         auto AllUnclaimed = Tracker.getUnclaimedUses(it->first);
         for (auto UnclaimedDRE : AllUnclaimed) {
           Handler.addDebugNoteForVar(
               it->first, UnclaimedDRE->getBeginLoc(),
                                      ("failed to produce fixit for '" + it->first->getNameAsString() +
                                       "' : has an unclaimed use"));
           }
 #endif
         it = FixablesForAllVars.byVar.erase(it);
       } else if (it->first->isInitCapture()) {
 #ifndef NDEBUG
         Handler.addDebugNoteForVar(
             it->first, it->first->getBeginLoc(),
                                    ("failed to produce fixit for '" + it->first->getNameAsString() +
                                     "' : init capture"));
 #endif
         it = FixablesForAllVars.byVar.erase(it);
       }else {
       ++it;
     }
   }
 
-  llvm::SmallVector<const VarDecl *, 16> UnsafeVars;
-  for (const auto &[VD, ignore] : FixablesForAllVars.byVar)
-    UnsafeVars.push_back(VD);
-
   // Fixpoint iteration for pointer assignments
   using DepMapTy = DenseMap<const VarDecl *, llvm::SetVector<const VarDecl *>>;
   DepMapTy DependenciesMap{};
   DepMapTy PtrAssignmentGraph{};
 
   for (auto it : FixablesForAllVars.byVar) {
     for (const FixableGadget *fixable : it.second) {
       std::optional<std::pair<const VarDecl *, const VarDecl *>> ImplPair =
if (VisitedVars.find(Var) == VisitedVars.end()) {
         if (!HasParm && isParameterOf(V, D))
           HasParm = true;
       }
       if (HasParm)
         GrpsUnionForParms.insert(VarGroup.begin(), VarGroup.end());
     }
   }
 
   // Remove a `FixableGadget` if the associated variable is not in the graph
   // computed above.  We do not want to generate fix-its for such variables,
   // since they are neither warned nor reachable from a warned one.
   //
   // Note a variable is not warned if it is not directly used in any unsafe
   // operation. A variable `v` is NOT reachable from an unsafe variable, if it
   // does not exist another variable `u` such that `u` is warned and fixing `u`
   // (transitively) implicates fixing `v`.
   //
   // For example,
   // ```
   // void f(int * p) {
   //   int * a = p; *p = 0;
   // }
   // ```
   // `*p = 0` is a fixable gadget associated with a variable `p` that is neither
   // warned nor reachable from a warned one.  If we add `a[5] = 0` to the end of
   // the function above, `p` becomes reachable from a warned variable.
   for (auto I = FixablesForAllVars.byVar.begin();
        I != FixablesForAllVars.byVar.end();) {
     // Note `VisitedVars` contain all the variables in the graph:
     if (!VisitedVars.count((*I).first)) {
       // no such var in graph:
       I = FixablesForAllVars.byVar.erase(I);
     } else
       ++I;
   }
 
-  Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
+  // We assign strategies to variables that are 1) in the graph and 2) can be
+  // fixed. Other variables have the default "Won't fix" strategy.
+  Strategy NaiveStrategy = getNaiveStrategy(llvm::make_filter_range(
+      VisitedVars, [&FixablesForAllVars](const VarDecl *V) {
+        // If a warned variable has no "Fixable", it is considered unfixable:
+        return FixablesForAllVars.byVar.count(V);
+      }));
   VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap, GrpsUnionForParms);
 
   if (isa<NamedDecl>(D))
     // The only case where `D` is not a `NamedDecl` is when `D` is a
     // `BlockDecl`. Let's not fix variables in blocks for now
     FixItsForVariableGroup =
         getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D,
                   Tracker, Handler, VarGrpMgr);
 
   for (const auto &G : UnsafeOps.noVar) {
     Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false);
   }
 
   for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) {
     auto FixItsIt = FixItsForVariableGroup.find(VD);

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

void parmsSingleton(int *p) { //expected-warning{{'p' is an unsafe pointer used for buffer access}} \
   // CHECK: fix-it:{{.*}}:{[[@LINE+3]]:3-[[@LINE+3]]:12}:"std::span<int> a"
   // CHECK: fix-it:{{.*}}:{[[@LINE+2]]:13-[[@LINE+2]]:13}:"{"
   // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:24-[[@LINE+1]]:24}:", 10}"
   int * a = new int[10];
   // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> b"
   int * b; //expected-warning{{'b' is an unsafe pointer used for buffer access}} \
        expected-note{{change type of 'b' to 'std::span' to preserve bounds information, and change 'a' to 'std::span' to propagate bounds information between them}}
 
   b = a;
   b[5] = 5; // expected-note{{used in buffer access here}}
   p[5] = 5; // expected-note{{used in buffer access here}}
 }
 
 
 // Parameters other than `r` all will be fixed
 // CHECK: fix-it:{{.*}}:{[[@LINE+15]]:24-[[@LINE+15]]:30}:"std::span<int> p"
 // CHECK  fix-it:{{.*}}:{[[@LINE+14]]:32-[[@LINE+14]]:39}:"std::span<int *> q"

 }
 
 void * multiParmAllFix(int *p, int **q, int a[], int * r);
 // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
 // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:58-[[@LINE-2]]:58}:";\nvoid * multiParmAllFix(std::span<int> p, std::span<int *> q, std::span<int> a, int * r)"
 
 // Fixing local variables implicates fixing parameters
 void  multiParmLocalAllFix(int *p, int * r) {
-  // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:28-[[@LINE-1]]:34}:"std::span<int> p"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:36-[[@LINE-2]]:43}:"std::span<int> r"
-  // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> x"
-  int * 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, and change 'p', 'z', and 'r' to safe types to make function 'multiParmLocalAllFix' bounds-safe}}
-  // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> z"
-  int * z; // expected-warning{{'z' is an unsafe pointer used for buffer access}} \
-              expected-note{{change type of 'z' to 'std::span' to preserve bounds information, and change 'x', 'p', and 'r' to safe types to make function 'multiParmLocalAllFix' bounds-safe}}
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]:
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]:
+  int * x; // expected-warning{{'x' is an unsafe pointer used for buffer access}}
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]:
+  int * z; // expected-warning{{'z' is an unsafe pointer used for buffer access}}
   int * y;
 
   x = p;
-  y = x;
+  y = x; // FIXME: we do not fix `y = x` here as the `.data()` fix-it is not generally correct
   // `x` needs to be fixed so does the pointer assigned to `x`, i.e.,`p`
   x[5] = 5; // expected-note{{used in buffer access here}}
   z = r;
   // `z` needs to be fixed so does the pointer assigned to `z`, i.e.,`r`
   z[5] = 5; // expected-note{{used in buffer access here}}
   // Since `p` and `r` are parameters need to be fixed together,
   // fixing `x` involves fixing all `p`, `r` and `z`. Similar for
   // fixing `z`.
 }
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void  multiParmLocalAllFix(int *p, int * r) {return multiParmLocalAllFix(std::span<int>(p, <# size #>), std::span<int>(r, <# size #>));}\n"
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]:
 
 
 // Fixing parameters implicates fixing local variables
-// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:29-[[@LINE+2]]:35}:"std::span<int> p"
-// CHECK: fix-it:{{.*}}:{[[@LINE+1]]:37-[[@LINE+1]]:44}:"std::span<int> r"
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]:
 void  multiParmLocalAllFix2(int *p, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
-                                                  expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'x', 'r', and 'z' to safe types to make function 'multiParmLocalAllFix2' bounds-safe}} \
-                                                  expected-warning{{'r' is an unsafe pointer used for buffer access}} \
-                    expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'x', and 'z' to safe types to make function 'multiParmLocalAllFix2' bounds-safe}}
+                                                  expected-warning{{'r' is an unsafe pointer used for buffer access}}
   int * x = new int[10];
-  // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> x"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]:
   int * z = new int[10];
-  // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> z"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
-  // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]:
   int * y;
 
   p = x;
-  y = x;
+  y = x;    // FIXME: we do not fix `y = x` here as the `.data()` fix-it is not generally correct
   p[5] = 5; // expected-note{{used in buffer access here}}
   r = z;
   r[5] = 5; // expected-note{{used in buffer access here}}
 }
-// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void  multiParmLocalAllFix2(int *p, int * r) {return multiParmLocalAllFix2(std::span<int>(p, <# size #>), std::span<int>(r, <# size #>));}\n"
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]:
 
 
 // No fix emitted for any of the parameter since parameter `r` cannot be fixed
 // CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]
 void noneParmFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
                    expected-warning{{'q' is an unsafe pointer used for buffer access}} \
                    expected-warning{{'r' is an unsafe pointer used for buffer access}}
   int tmp = p[5]; // expected-note{{used in buffer access here}}

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-fixits-test.cpp

 // RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions %s 2>&1 | FileCheck %s
 
 void foo1a() {
   int *r = new int[7];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *p = new int[4];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 4}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   p = r;
   int tmp = p[9];
   int *q;
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
-  q = r;
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  q = r;  // FIXME: we do not fix `q = r` here as the `.data()` fix-it is not generally correct
 }
 
 void foo1b() {
   int *r = new int[7];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
   int *p = new int[4];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 4}"
   p = r;
   int tmp = p[9];
   int *q = new int[4];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 4}"
   q = r;
   tmp = q[9];
 }
 
 void foo1c() {
   int *r = new int[7];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *p = new int[4];
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
-  p = r;
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  p = r;  // FIXME: we do not fix `p = r` here as the `.data()` fix-it is not generally correct
   int tmp = r[9];
   int *q;
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   q = r;
   tmp = q[9];
 }
 
 void foo2a() {
   int *r = new int[7];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
   int *p = new int[5];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 5}"
   int *q = new int[4];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 4}"
   p = q;
   int tmp = p[8];
   q = r;
 }
 
 void foo2b() {
   int *r = new int[7];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *p = new int[5];
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 5}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *q = new int[4];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 4}"
-  p = q;
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  p = q;  // FIXME: we do not fix `p = q` here as the `.data()` fix-it is not generally correct
   int tmp = q[8];
   q = r;
 }
 
 void foo2c() {
   int *r = new int[7];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
void foo2c() {
   p = q;
   int tmp = p[8];
   q = r;
   tmp = q[8];
 }
 
 void foo3a() {
   int *r = new int[7];
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *p = new int[5];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 5}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *q = new int[4];
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
-  q = p;
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  q = p;  // FIXME: we do not fix `q = p` here as the `.data()` fix-it is not generally correct
   int tmp = p[8];
   q = r;
 }
 
 void foo3b() {
   int *r = new int[10];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init-fixits.cpp

void rhs_span3() {
   p[5] = 10;
   int *r = q;
 }
 
 void test_grouping() {
   int *z = new int[8];
   int tmp;
   int *y = new int[10];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> y"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   tmp = y[5];
 
   int *x = new int[10];
-  x = y;
+  x = y; // FIXME: we do not fix `x = y` here as the `.data()` fix-it is not generally correct
 
   int *w = z;
 }
 
 void test_crash() {
   int *r = new int[8];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp

void rhs_span2() {
   int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' to 'std::span' to propagate bounds information between them}}
   p[5] = 10;  // expected-note{{used in buffer access here}}
 }
 
 // FIXME: Suggest fixits for p, q, and r since span a valid fixit for r.
 void rhs_span3() {
   int *q = new int[6];
   int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   p[5] = 10;  // expected-note{{used in buffer access here}}
-  int *r = q;
+  int *r = q; // FIXME: we do not fix `int *r = q` here as the `.data()` fix-it is not generally correct
 }
 
 void test_grouping() {
   int *z = new int[8];
   int tmp;
-  int *y = new int[10];  // expected-warning{{'y' is an unsafe pointer used for buffer access}} expected-note{{change type of 'y' to 'std::span' to preserve bounds information}}
+  int *y = new int[10];  // expected-warning{{'y' is an unsafe pointer used for buffer access}}
   tmp = y[5]; // expected-note{{used in buffer access here}}
 
   int *x = new int[10];
-  x = y;
+  x = y;      // FIXME: we do not fix `x = y` here as the `.data()` fix-it is not generally correct
 
   int *w = z;
 }
 
 void test_crash() {
   int *r = new int[8];
   int *q = r;
   int *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, and change 'q' and 'r' to 'std::span' to propagate bounds information between them}}
   p = q;
   int tmp = p[9];  // expected-note{{used in buffer access here}}
 }

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp

 // RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
 // RUN:            -fsafe-buffer-usage-suggestions \
 // RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void bar(int * param) {}
 
 void foo1a() {
   int *r = new int[7];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int *p = new int[4];
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 4}"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   p = r;
   int tmp = p[9];
   int *q;
-  q = r;
+  q = r; // FIXME: we do not fix `q = r` here as the `.data()` fix-it is not generally correct
 }
 
 void uuc_if_body() {
   int *r = new int[7];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
   int *p = new int[4];

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp

 // RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions -verify %s
 void bar(int * param) {}
 
 void foo1a() {
   int *r = new int[7];
-  int *p = new int[4];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them}}
+  int *p = new int[4];  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   p = r;
   int tmp = p[9];  // expected-note{{used in buffer access here}}
   int *q;
-  q = r;
+  q = r;           // FIXME: we do not fix `q = r` here as the `.data()` fix-it is not generally correct
 }
 
 void uuc_if_body() {
   int *r = new int[7];
   int *p = new int[4];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} // expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them}}
   if (true)
     p = r;
   p[5] = 4;  // expected-note{{used in buffer access here}}

clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp

void local_assign_both_span() {
   tmp = q[4];  // expected-note{{used in buffer access here}}
 
   q = p;
 }
 
 void local_assign_rhs_span() {
   int tmp;
   int* p = new int[10];
-  int* q = new int[10];  // expected-warning{{'q' is an unsafe pointer used for buffer access}} expected-note{{change type of 'q' to 'std::span' to preserve bounds information}}
+  int* q = new int[10];  // expected-warning{{'q' is an unsafe pointer used for buffer access}}
   tmp = q[4];  // expected-note{{used in buffer access here}}
-  p = q;
+  p = q;  // FIXME: we do not fix `p = q` here as the `.data()` fix-it is not generally correct
 }
 
 void local_assign_no_span() {
   int tmp;
   int* p = new int[10];
   int* q = new int[10];
   p = q;
 }
void lhs_span_multi_assign() {
   int *b = a;
   int *c = b;
   int *d = c;  // expected-warning{{'d' is an unsafe pointer used for buffer access}} expected-note{{change type of 'd' to 'std::span' to preserve bounds information, and change 'c', 'b', and 'a' to 'std::span' to propagate bounds information between them}}
   int tmp = d[2];  // expected-note{{used in buffer access here}}
 }
 
 void rhs_span() {
   int *x = new int[3];
-  int *y;  // expected-warning{{'y' is an unsafe pointer used for buffer access}} expected-note{{change type of 'y' to 'std::span' to preserve bounds information}}
+  int *y;  // expected-warning{{'y' is an unsafe pointer used for buffer access}}
   y[5] = 10;  // expected-note{{used in buffer access here}}
 
-  x = y;
+  x = y; // FIXME: we do not fix `x = y` here as the `.data()` fix-it is not generally correct
 }
 
 void rhs_span1() {
   int *q = new int[12];
   int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'r' to 'std::span' to propagate bounds information between them}}
   p[5] = 10;  // expected-note{{used in buffer access here}}
   int *r = q;  // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p' and 'q' to 'std::span' to propagate bounds information between them}}
   r[10] = 5;  // expected-note{{used in buffer access here}}
 }
 
 void rhs_span2() {
   int *q = new int[6];
-  int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   p[5] = 10;  // expected-note{{used in buffer access here}}
-  int *r = q;
+  int *r = q; // FIXME: we do not fix `int *r = q` here as the `.data()` fix-it is not generally correct
 }
 
 void test_grouping() {
   int *z = new int[8];
   int tmp;
-  int *y = new int[10];  // expected-warning{{'y' is an unsafe pointer used for buffer access}} expected-note{{change type of 'y' to 'std::span' to preserve bounds information}}
+  int *y = new int[10];  // expected-warning{{'y' is an unsafe pointer used for buffer access}}
   tmp = y[5]; // expected-note{{used in buffer access here}}
 
   int *x = new int[10];
-  x = y;
+  x = y;      // FIXME: we do not fix `x = y` here as the `.data()` fix-it is not generally correct
 
   int *w = z;
 }
 
 void test_grouping1() {
   int tmp;
-  int *y = new int[10];  // expected-warning{{'y' is an unsafe pointer used for buffer access}} expected-note{{change type of 'y' to 'std::span' to preserve bounds information}}
+  int *y = new int[10];  // expected-warning{{'y' is an unsafe pointer used for buffer access}}
   tmp = y[5];  // expected-note{{used in buffer access here}}
   int *x = new int[10];
-  x = y;
+  x = y;       // FIXME: we do not fix `x = y` here as the `.data()` fix-it is not generally correct
 
-  int *w = new int[10];  // expected-warning{{'w' is an unsafe pointer used for buffer access}} expected-note{{change type of 'w' to 'std::span' to preserve bounds information}}
+  int *w = new int[10];  // expected-warning{{'w' is an unsafe pointer used for buffer access}}
   tmp = w[5];  // expected-note{{used in buffer access here}}
   int *z = new int[10];
-  z = w;
+  z = w;       // FIXME: we do not fix `z = w` here as the `.data()` fix-it is not generally correct
 }
 
 void foo1a() {
   int *r = new int[7];
-  int *p = new int[4];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them}}
+  int *p = new int[4];  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   p = r;
   int tmp = p[9];  // expected-note{{used in buffer access here}}
   int *q;
-  q = r;
+  q = r;  // FIXME: we do not fix `q = r` here as the `.data()` fix-it is not generally correct
 }
 
 void foo1b() {
   int *r = new int[7];
   int *p = new int[4];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'r' and 'q' to 'std::span' to propagate bounds information between them}}
   p = r;
   int tmp = p[9];  // expected-note{{used in buffer access here}}
   int *q;  // expected-warning{{'q' is an unsafe pointer used for buffer access}} expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p' and 'r' to 'std::span' to propagate bounds information between them}}
   q = r;
   tmp = q[9];  // expected-note{{used in buffer access here}}
 }
 
 void foo1c() {
-  int *r = new int[7];  // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'q' to 'std::span' to propagate bounds information between them}}
+  int *r = new int[7];  // expected-warning{{'r' is an unsafe pointer used for buffer access}}
   int *p = new int[4];
-  p = r;
+  p = r;   // FIXME: we do not fix `p = r` here as the `.data()` fix-it is not generally correct
   int tmp = r[9];  // expected-note{{used in buffer access here}}
-  int *q;  // expected-warning{{'q' is an unsafe pointer used for buffer access}} expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them}}
-  q = r;
+  int *q;  // expected-warning{{'q' is an unsafe pointer used for buffer access}}
+  q = r;   // FIXME: we do not fix `q = r` here as the `.data()` fix-it is not generally correct
   tmp = q[9];  // expected-note{{used in buffer access here}}
 }
 
 void foo2a() {
   int *r = new int[7];
   int *p = new int[5];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'r' to 'std::span' to propagate bounds information between them}}
   int *q = new int[4];
   p = q;
   int tmp = p[8];  // expected-note{{used in buffer access here}}
   q = r;
 }
 
 void foo2b() {
   int *r = new int[7];
   int *p = new int[5];
-  int *q = new int[4];  // expected-warning{{'q' is an unsafe pointer used for buffer access}} expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them}}
-  p = q;
+  int *q = new int[4];  // expected-warning{{'q' is an unsafe pointer used for buffer access}}
+  p = q;           // FIXME: we do not fix `p = q` here as the `.data()` fix-it is not generally correct
   int tmp = q[8];  // expected-note{{used in buffer access here}}
   q = r;
 }
 
 void foo2c() {
   int *r = new int[7];
   int *p = new int[5];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'r' to 'std::span' to propagate bounds information between them}}
   int *q = new int[4];  // expected-warning{{'q' is an unsafe pointer used for buffer access}} expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p' and 'r' to 'std::span' to propagate bounds information between them}}
   p = q;
   int tmp = p[8];  // expected-note{{used in buffer access here}}
   q = r;
   tmp = q[8];  // expected-note{{used in buffer access here}}
 }
 
 void foo3a() {
   int *r = new int[7];
-  int *p = new int[5];  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note{{change type of 'p' to 'std::span' to preserve bounds information}}
+  int *p = new int[5];  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   int *q = new int[4];
-  q = p;
+  q = p;           // FIXME: we do not fix `q = p` here as the `.data()` fix-it is not generally correct
   int tmp = p[8];  // expected-note{{used in buffer access here}}
   q = r;
 }
 
 void foo3b() {
   int *r = new int[7];
   int *p = new int[5];
   int *q = new int[4];  // expected-warning{{'q' is an unsafe pointer used for buffer access}} //expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'r' and 'p' to 'std::span' to propagate bounds information between them}}

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

 // operation on `c` is not suppressed. Only variable `c` will be fixed
 // then.
 void suppressedVarInGroup3() {
   int * a;
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int * b;
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
   int * c;
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> c"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
 
   c[5] = 5;
 #pragma clang unsafe_buffer_usage begin
   b[5] = 5;
 #pragma clang unsafe_buffer_usage end
   a = b;
   b = c;
+// FIXME: we do not fix `a = b` and `b = c` because the `.data()`  fix-it is not generally correct.
 }
 
 // The implication edges are: `a` -> `b` -> `c` -> `a`.
 // The unsafe operation on `b` is supressed, while the unsafe
 // operation on `c` is not suppressed. Since the implication graph
 // forms a cycle, all variables will be fixed.
 void suppressedVarInGroup4() {
   int * a;