[clang] [-Wunsafe-buffer-usage] Introduce std::array fixits (PR #80084)

[via cfe-commits]

https://github.com/jkorous-apple updated https://github.com/llvm/llvm-project/pull/80084

>From 463a9904c1ae85fbdc0bd6029c6effea3fb16ea6 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Tue, 23 Jan 2024 16:16:10 -0800
Subject: [PATCH 01/13] [-Wunsafe-buffer-usage] Move Strategy class to the
 header

It needs to be used from handleUnsafeVariableGroup function.
---
 .../Analysis/Analyses/UnsafeBufferUsage.h     |  37 +++
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 238 ++++++++----------
 2 files changed, 140 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index aca1ad998822c..622c094f5b1eb 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -42,6 +42,43 @@ class VariableGroupsManager {
   virtual VarGrpRef getGroupOfParms() const =0;
 };
 
+// FixitStrategy is a map from variables to the way we plan to emit fixes for
+// these variables. It is figured out gradually by trying different fixes
+// for different variables depending on gadgets in which these variables
+// participate.
+class FixitStrategy {
+public:
+  enum class Kind {
+    Wontfix,  // We don't plan to emit a fixit for this variable.
+    Span,     // We recommend replacing the variable with std::span.
+    Iterator, // We recommend replacing the variable with std::span::iterator.
+    Array,    // We recommend replacing the variable with std::array.
+    Vector    // We recommend replacing the variable with std::vector.
+  };
+
+private:
+  using MapTy = llvm::DenseMap<const VarDecl *, Kind>;
+
+  MapTy Map;
+
+public:
+  FixitStrategy() = default;
+  FixitStrategy(const FixitStrategy &) = delete; // Let's avoid copies.
+  FixitStrategy &operator=(const FixitStrategy &) = delete;
+  FixitStrategy(FixitStrategy &&) = default;
+  FixitStrategy &operator=(FixitStrategy &&) = default;
+
+  void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
+
+  Kind lookup(const VarDecl *VD) const {
+    auto I = Map.find(VD);
+    if (I == Map.end())
+      return Kind::Wontfix;
+
+    return I->second;
+  }
+};
+
 /// The interface that lets the caller handle unsafe buffer usage analysis
 /// results by overriding this class's handle... methods.
 class UnsafeBufferUsageHandler {
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 823cd2a7b9969..924d677786275 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -407,9 +407,6 @@ using DeclUseList = SmallVector<const DeclRefExpr *, 1>;
 
 // Convenience typedef.
 using FixItList = SmallVector<FixItHint, 4>;
-
-// Defined below.
-class Strategy;
 } // namespace
 
 namespace {
@@ -486,7 +483,7 @@ class FixableGadget : public Gadget {
   /// Returns a fixit that would fix the current gadget according to
   /// the current strategy. Returns std::nullopt if the fix cannot be produced;
   /// returns an empty list if no fixes are necessary.
-  virtual std::optional<FixItList> getFixits(const Strategy &) const {
+  virtual std::optional<FixItList> getFixits(const FixitStrategy &) const {
     return std::nullopt;
   }
 
@@ -737,7 +734,8 @@ class PointerInitGadget : public FixableGadget {
     return stmt(PtrInitStmt);
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 
   virtual const Stmt *getBaseStmt() const override {
     // FIXME: This needs to be the entire DeclStmt, assuming that this method
@@ -789,7 +787,8 @@ class PointerAssignmentGadget : public FixableGadget {
     return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 
   virtual const Stmt *getBaseStmt() const override {
     // FIXME: This should be the binary operator, assuming that this method
@@ -892,7 +891,8 @@ class ULCArraySubscriptGadget : public FixableGadget {
     return expr(isInUnspecifiedLvalueContext(Target));
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 
   virtual const Stmt *getBaseStmt() const override { return Node; }
 
@@ -932,7 +932,8 @@ class UPCStandalonePointerGadget : public FixableGadget {
     return stmt(isInUnspecifiedPointerContext(target));
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 
   virtual const Stmt *getBaseStmt() const override { return Node; }
 
@@ -976,7 +977,8 @@ class PointerDereferenceGadget : public FixableGadget {
 
   virtual const Stmt *getBaseStmt() const final { return Op; }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 };
 
 // Represents expressions of the form `&DRE[any]` in the Unspecified Pointer
@@ -1009,7 +1011,8 @@ class UPCAddressofArraySubscriptGadget : public FixableGadget {
             .bind(UPCAddressofArraySubscriptTag)))));
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &) const override;
 
   virtual const Stmt *getBaseStmt() const override { return Node; }
 
@@ -1088,46 +1091,6 @@ class DeclUseTracker {
 };
 } // namespace
 
-namespace {
-// Strategy is a map from variables to the way we plan to emit fixes for
-// these variables. It is figured out gradually by trying different fixes
-// for different variables depending on gadgets in which these variables
-// participate.
-class Strategy {
-public:
-  enum class Kind {
-    Wontfix,  // We don't plan to emit a fixit for this variable.
-    Span,     // We recommend replacing the variable with std::span.
-    Iterator, // We recommend replacing the variable with std::span::iterator.
-    Array,    // We recommend replacing the variable with std::array.
-    Vector    // We recommend replacing the variable with std::vector.
-  };
-
-private:
-  using MapTy = llvm::DenseMap<const VarDecl *, Kind>;
-
-  MapTy Map;
-
-public:
-  Strategy() = default;
-  Strategy(const Strategy &) = delete; // Let's avoid copies.
-  Strategy &operator=(const Strategy &) = delete;
-  Strategy(Strategy &&) = default;
-  Strategy &operator=(Strategy &&) = default;
-
-  void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
-
-  Kind lookup(const VarDecl *VD) const {
-    auto I = Map.find(VD);
-    if (I == Map.end())
-      return Kind::Wontfix;
-
-    return I->second;
-  }
-};
-} // namespace
-
-
 // Representing a pointer type expression of the form `++Ptr` in an Unspecified
 // Pointer Context (UPC):
 class UPCPreIncrementGadget : public FixableGadget {
@@ -1159,7 +1122,8 @@ class UPCPreIncrementGadget : public FixableGadget {
 										  ).bind(UPCPreIncrementTag)))));
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 
   virtual const Stmt *getBaseStmt() const override { return Node; }
 
@@ -1204,7 +1168,8 @@ class UUCAddAssignGadget : public FixableGadget {
     // clang-format on
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &S) const override;
 
   virtual const Stmt *getBaseStmt() const override { return Node; }
 
@@ -1254,7 +1219,8 @@ class DerefSimplePtrArithFixableGadget : public FixableGadget {
     // clang-format on
   }
 
-  virtual std::optional<FixItList> getFixits(const Strategy &s) const final;
+  virtual std::optional<FixItList>
+  getFixits(const FixitStrategy &s) const final;
 
   // TODO remove this method from FixableGadget interface
   virtual const Stmt *getBaseStmt() const final { return nullptr; }
@@ -1464,38 +1430,38 @@ bool clang::internal::anyConflict(const SmallVectorImpl<FixItHint> &FixIts,
 }
 
 std::optional<FixItList>
-PointerAssignmentGadget::getFixits(const Strategy &S) const {
+PointerAssignmentGadget::getFixits(const FixitStrategy &S) const {
   const auto *LeftVD = cast<VarDecl>(PtrLHS->getDecl());
   const auto *RightVD = cast<VarDecl>(PtrRHS->getDecl());
   switch (S.lookup(LeftVD)) {
-    case Strategy::Kind::Span:
-      if (S.lookup(RightVD) == Strategy::Kind::Span)
-        return FixItList{};
-      return std::nullopt;
-    case Strategy::Kind::Wontfix:
-      return std::nullopt;
-    case Strategy::Kind::Iterator:
-    case Strategy::Kind::Array:
-    case Strategy::Kind::Vector:
-      llvm_unreachable("unsupported strategies for FixableGadgets");
+  case FixitStrategy::Kind::Span:
+    if (S.lookup(RightVD) == FixitStrategy::Kind::Span)
+      return FixItList{};
+    return std::nullopt;
+  case FixitStrategy::Kind::Wontfix:
+    return std::nullopt;
+  case FixitStrategy::Kind::Iterator:
+  case FixitStrategy::Kind::Array:
+  case FixitStrategy::Kind::Vector:
+    llvm_unreachable("unsupported strategies for FixableGadgets");
   }
   return std::nullopt;
 }
 
 std::optional<FixItList>
-PointerInitGadget::getFixits(const Strategy &S) const {
+PointerInitGadget::getFixits(const FixitStrategy &S) const {
   const auto *LeftVD = PtrInitLHS;
   const auto *RightVD = cast<VarDecl>(PtrInitRHS->getDecl());
   switch (S.lookup(LeftVD)) {
-    case Strategy::Kind::Span:
-      if (S.lookup(RightVD) == Strategy::Kind::Span)
-        return FixItList{};
-      return std::nullopt;
-    case Strategy::Kind::Wontfix:
-      return std::nullopt;
-    case Strategy::Kind::Iterator:
-    case Strategy::Kind::Array:
-    case Strategy::Kind::Vector:
+  case FixitStrategy::Kind::Span:
+    if (S.lookup(RightVD) == FixitStrategy::Kind::Span)
+      return FixItList{};
+    return std::nullopt;
+  case FixitStrategy::Kind::Wontfix:
+    return std::nullopt;
+  case FixitStrategy::Kind::Iterator:
+  case FixitStrategy::Kind::Array:
+  case FixitStrategy::Kind::Vector:
     llvm_unreachable("unsupported strategies for FixableGadgets");
   }
   return std::nullopt;
@@ -1512,12 +1478,12 @@ static bool isNonNegativeIntegerExpr(const Expr *Expr, const VarDecl *VD,
 }
 
 std::optional<FixItList>
-ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
+ULCArraySubscriptGadget::getFixits(const FixitStrategy &S) const {
   if (const auto *DRE =
           dyn_cast<DeclRefExpr>(Node->getBase()->IgnoreImpCasts()))
     if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
       switch (S.lookup(VD)) {
-      case Strategy::Kind::Span: {
+      case FixitStrategy::Kind::Span: {
 
         // If the index has a negative constant value, we give up as no valid
         // fix-it can be generated:
@@ -1528,10 +1494,10 @@ ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
         // no-op is a good fix-it, otherwise
         return FixItList{};
       }
-      case Strategy::Kind::Wontfix:
-      case Strategy::Kind::Iterator:
-      case Strategy::Kind::Array:
-      case Strategy::Kind::Vector:
+      case FixitStrategy::Kind::Array:
+      case FixitStrategy::Kind::Wontfix:
+      case FixitStrategy::Kind::Iterator:
+      case FixitStrategy::Kind::Vector:
         llvm_unreachable("unsupported strategies for FixableGadgets");
       }
     }
@@ -1542,17 +1508,17 @@ static std::optional<FixItList> // forward declaration
 fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node);
 
 std::optional<FixItList>
-UPCAddressofArraySubscriptGadget::getFixits(const Strategy &S) const {
+UPCAddressofArraySubscriptGadget::getFixits(const FixitStrategy &S) const {
   auto DREs = getClaimedVarUseSites();
   const auto *VD = cast<VarDecl>(DREs.front()->getDecl());
 
   switch (S.lookup(VD)) {
-  case Strategy::Kind::Span:
+  case FixitStrategy::Kind::Span:
     return fixUPCAddressofArraySubscriptWithSpan(Node);
-  case Strategy::Kind::Wontfix:
-  case Strategy::Kind::Iterator:
-  case Strategy::Kind::Array:
-  case Strategy::Kind::Vector:
+  case FixitStrategy::Kind::Wontfix:
+  case FixitStrategy::Kind::Iterator:
+  case FixitStrategy::Kind::Array:
+  case FixitStrategy::Kind::Vector:
     llvm_unreachable("unsupported strategies for FixableGadgets");
   }
   return std::nullopt; // something went wrong, no fix-it
@@ -1803,10 +1769,10 @@ getSpanTypeText(StringRef EltTyText,
 }
 
 std::optional<FixItList>
-DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
+DerefSimplePtrArithFixableGadget::getFixits(const FixitStrategy &s) const {
   const VarDecl *VD = dyn_cast<VarDecl>(BaseDeclRefExpr->getDecl());
 
-  if (VD && s.lookup(VD) == Strategy::Kind::Span) {
+  if (VD && s.lookup(VD) == FixitStrategy::Kind::Span) {
     ASTContext &Ctx = VD->getASTContext();
     // std::span can't represent elements before its begin()
     if (auto ConstVal = Offset->getIntegerConstantExpr(Ctx))
@@ -1866,10 +1832,10 @@ DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
 }
 
 std::optional<FixItList>
-PointerDereferenceGadget::getFixits(const Strategy &S) const {
+PointerDereferenceGadget::getFixits(const FixitStrategy &S) const {
   const VarDecl *VD = cast<VarDecl>(BaseDeclRefExpr->getDecl());
   switch (S.lookup(VD)) {
-  case Strategy::Kind::Span: {
+  case FixitStrategy::Kind::Span: {
     ASTContext &Ctx = VD->getASTContext();
     SourceManager &SM = Ctx.getSourceManager();
     // Required changes: *(ptr); => (ptr[0]); and *ptr; => ptr[0]
@@ -1884,11 +1850,11 @@ PointerDereferenceGadget::getFixits(const Strategy &S) const {
     }
     break;
   }
-  case Strategy::Kind::Iterator:
-  case Strategy::Kind::Array:
-  case Strategy::Kind::Vector:
-    llvm_unreachable("Strategy not implemented yet!");
-  case Strategy::Kind::Wontfix:
+  case FixitStrategy::Kind::Iterator:
+  case FixitStrategy::Kind::Array:
+  case FixitStrategy::Kind::Vector:
+    llvm_unreachable("FixitStrategy not implemented yet!");
+  case FixitStrategy::Kind::Wontfix:
     llvm_unreachable("Invalid strategy!");
   }
 
@@ -1897,27 +1863,26 @@ PointerDereferenceGadget::getFixits(const Strategy &S) const {
 
 // Generates fix-its replacing an expression of the form UPC(DRE) with
 // `DRE.data()`
-std::optional<FixItList> UPCStandalonePointerGadget::getFixits(const Strategy &S)
-      const {
+std::optional<FixItList>
+UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const {
   const auto VD = cast<VarDecl>(Node->getDecl());
   switch (S.lookup(VD)) {
-    case Strategy::Kind::Span: {
-      ASTContext &Ctx = VD->getASTContext();
-      SourceManager &SM = Ctx.getSourceManager();
-      // Inserts the .data() after the DRE
-      std::optional<SourceLocation> EndOfOperand =
-          getPastLoc(Node, SM, Ctx.getLangOpts());
-
-      if (EndOfOperand)
-        return FixItList{{FixItHint::CreateInsertion(
-            *EndOfOperand, ".data()")}};
-      // FIXME: Points inside a macro expansion.
-      break;
+  case FixitStrategy::Kind::Span: {
+    ASTContext &Ctx = VD->getASTContext();
+    SourceManager &SM = Ctx.getSourceManager();
+    // Inserts the .data() after the DRE
+    std::optional<SourceLocation> EndOfOperand =
+        getPastLoc(Node, SM, Ctx.getLangOpts());
+
+    if (EndOfOperand)
+      return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
+    // FIXME: Points inside a macro expansion.
+    break;
     }
-    case Strategy::Kind::Wontfix:
-    case Strategy::Kind::Iterator:
-    case Strategy::Kind::Array:
-    case Strategy::Kind::Vector:
+    case FixitStrategy::Kind::Wontfix:
+    case FixitStrategy::Kind::Iterator:
+    case FixitStrategy::Kind::Array:
+    case FixitStrategy::Kind::Vector:
       llvm_unreachable("unsupported strategies for FixableGadgets");
   }
 
@@ -1962,14 +1927,14 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) {
 }
 
 std::optional<FixItList>
-UUCAddAssignGadget::getFixits(const Strategy &S) const {
+UUCAddAssignGadget::getFixits(const FixitStrategy &S) const {
   DeclUseList DREs = getClaimedVarUseSites();
 
   if (DREs.size() != 1)
     return std::nullopt; // In cases of `Ptr += n` where `Ptr` is not a DRE, we
                          // give up
   if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) {
-    if (S.lookup(VD) == Strategy::Kind::Span) {
+    if (S.lookup(VD) == FixitStrategy::Kind::Span) {
       FixItList Fixes;
 
       const Stmt *AddAssignNode = getBaseStmt();
@@ -2003,14 +1968,15 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
   return std::nullopt; // Not in the cases that we can handle for now, give up.
 }
 
-std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) const {
+std::optional<FixItList>
+UPCPreIncrementGadget::getFixits(const FixitStrategy &S) const {
   DeclUseList DREs = getClaimedVarUseSites();
 
   if (DREs.size() != 1)
     return std::nullopt; // In cases of `++Ptr` where `Ptr` is not a DRE, we
                          // give up
   if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) {
-    if (S.lookup(VD) == Strategy::Kind::Span) {
+    if (S.lookup(VD) == FixitStrategy::Kind::Span) {
       FixItList Fixes;
       std::stringstream SS;
       const Stmt *PreIncNode = getBaseStmt();
@@ -2261,7 +2227,7 @@ static bool hasConflictingOverload(const FunctionDecl *FD) {
 // }
 //
 static std::optional<FixItList>
-createOverloadsForFixedParams(const Strategy &S, const FunctionDecl *FD,
+createOverloadsForFixedParams(const FixitStrategy &S, const FunctionDecl *FD,
                               const ASTContext &Ctx,
                               UnsafeBufferUsageHandler &Handler) {
   // FIXME: need to make this conflict checking better:
@@ -2278,9 +2244,9 @@ createOverloadsForFixedParams(const Strategy &S, const FunctionDecl *FD,
   for (unsigned i = 0; i < NumParms; i++) {
     const ParmVarDecl *PVD = FD->getParamDecl(i);
 
-    if (S.lookup(PVD) == Strategy::Kind::Wontfix)
+    if (S.lookup(PVD) == FixitStrategy::Kind::Wontfix)
       continue;
-    if (S.lookup(PVD) != Strategy::Kind::Span)
+    if (S.lookup(PVD) != FixitStrategy::Kind::Span)
       // Not supported, not suppose to happen:
       return std::nullopt;
 
@@ -2291,7 +2257,8 @@ createOverloadsForFixedParams(const Strategy &S, const FunctionDecl *FD,
     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.
+    // FIXME: whether we should create std::span type depends on the
+    // FixitStrategy.
     NewTysTexts[i] = getSpanTypeText(*PteTyText, PteTyQuals);
     ParmsMask[i] = true;
     AtLeastOneParmToFix = true;
@@ -2498,7 +2465,7 @@ static FixItList fixVariableWithSpan(const VarDecl *VD,
 // 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,
+fixVariable(const VarDecl *VD, FixitStrategy::Kind K,
             /* The function decl under analysis */ const Decl *D,
             const DeclUseTracker &Tracker, ASTContext &Ctx,
             UnsafeBufferUsageHandler &Handler) {
@@ -2529,7 +2496,7 @@ fixVariable(const VarDecl *VD, Strategy::Kind K,
   }
 
   switch (K) {
-  case Strategy::Kind::Span: {
+  case FixitStrategy::Kind::Span: {
     if (VD->getType()->isPointerType()) {
       if (const auto *PVD = dyn_cast<ParmVarDecl>(VD))
         return fixParamWithSpan(PVD, Ctx, Handler);
@@ -2540,11 +2507,11 @@ fixVariable(const VarDecl *VD, Strategy::Kind K,
     DEBUG_NOTE_DECL_FAIL(VD, " : not a pointer");
     return {};
   }
-  case Strategy::Kind::Iterator:
-  case Strategy::Kind::Array:
-  case Strategy::Kind::Vector:
-    llvm_unreachable("Strategy not implemented yet!");
-  case Strategy::Kind::Wontfix:
+  case FixitStrategy::Kind::Array:
+  case FixitStrategy::Kind::Iterator:
+  case FixitStrategy::Kind::Vector:
+    llvm_unreachable("FixitStrategy not implemented yet!");
+  case FixitStrategy::Kind::Wontfix:
     llvm_unreachable("Invalid strategy!");
   }
   llvm_unreachable("Unknown strategy!");
@@ -2605,7 +2572,8 @@ static void eraseVarsForUnfixableGroupMates(
 static FixItList createFunctionOverloadsForParms(
     std::map<const VarDecl *, FixItList> &FixItsForVariable /* mutable */,
     const VariableGroupsManager &VarGrpMgr, const FunctionDecl *FD,
-    const Strategy &S, ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) {
+    const FixitStrategy &S, ASTContext &Ctx,
+    UnsafeBufferUsageHandler &Handler) {
   FixItList FixItsSharedByParms{};
 
   std::optional<FixItList> OverloadFixes =
@@ -2625,8 +2593,8 @@ static FixItList createFunctionOverloadsForParms(
 
 // Constructs self-contained fix-its for each variable in `FixablesForAllVars`.
 static std::map<const VarDecl *, FixItList>
-getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
-	  ASTContext &Ctx,
+getFixIts(FixableGadgetSets &FixablesForAllVars, const FixitStrategy &S,
+          ASTContext &Ctx,
           /* The function decl under analysis */ const Decl *D,
           const DeclUseTracker &Tracker, UnsafeBufferUsageHandler &Handler,
           const VariableGroupsManager &VarGrpMgr) {
@@ -2724,11 +2692,11 @@ getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
 }
 
 template <typename VarDeclIterTy>
-static Strategy
+static FixitStrategy
 getNaiveStrategy(llvm::iterator_range<VarDeclIterTy> UnsafeVars) {
-  Strategy S;
+  FixitStrategy S;
   for (const VarDecl *VD : UnsafeVars) {
-    S.set(VD, Strategy::Kind::Span);
+    S.set(VD, FixitStrategy::Kind::Span);
   }
   return S;
 }
@@ -3027,7 +2995,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
 
   // 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(
+  FixitStrategy 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);

>From 38255d1ebe412a770053bc132072d2fdb56af62b Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Tue, 23 Jan 2024 16:24:59 -0800
Subject: [PATCH 02/13] [-Wunsafe-buffer-usage] Implement fixits from const
 size array to std::array

Array subscript on a const size array is not bounds-checked. The idiomatic
replacement is std::array which is bounds-safe in hardened mode of libc++.

This commit extends the fixit-producing machine to consider std::array as a
transformation target type and teaches it to handle the array subscript on const
size arrays with a trivial (empty) fixit.

The transformation is straightforward for most cases:
T array[N]; => std::array<T, N> array;
but for array of const pointers it needs to be:
T* const array[N]; => const std::array<T*, N> array;

This case and other special cases are to be implemented in follow-up
patches.
---
 .../Analysis/Analyses/UnsafeBufferUsage.h     | 10 +-
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 85 ++++++++++++++++-
 clang/lib/Sema/AnalysisBasedWarnings.cpp      | 16 +++-
 .../warn-unsafe-buffer-usage-array.cpp        | 24 +++++
 .../warn-unsafe-buffer-usage-debug.cpp        | 12 +--
 ...fe-buffer-usage-fixits-local-var-array.cpp | 95 +++++++++++++++++++
 .../test/SemaCXX/warn-unsafe-buffer-usage.cpp |  5 +
 7 files changed, 230 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
 create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp

diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 622c094f5b1eb..2c9ebf3fb42d0 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -95,6 +95,8 @@ class UnsafeBufferUsageHandler {
 #endif
 
 public:
+  enum class TargetType { Span, Array };
+
   UnsafeBufferUsageHandler() = default;
   virtual ~UnsafeBufferUsageHandler() = default;
 
@@ -112,9 +114,11 @@ class UnsafeBufferUsageHandler {
   ///
   /// `D` is the declaration of the callable under analysis that owns `Variable`
   /// and all of its group mates.
-  virtual void handleUnsafeVariableGroup(const VarDecl *Variable,
-                                         const VariableGroupsManager &VarGrpMgr,
-                                         FixItList &&Fixes, const Decl *D) = 0;
+  virtual void
+  handleUnsafeVariableGroup(const VarDecl *Variable,
+                            const VariableGroupsManager &VarGrpMgr,
+                            FixItList &&Fixes, const Decl *D,
+                            const FixitStrategy &VarTargetTypes) = 0;
 
 #ifndef NDEBUG
 public:
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 924d677786275..6c99a23aeb9e8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1495,6 +1495,7 @@ ULCArraySubscriptGadget::getFixits(const FixitStrategy &S) const {
         return FixItList{};
       }
       case FixitStrategy::Kind::Array:
+        return FixItList{};
       case FixitStrategy::Kind::Wontfix:
       case FixitStrategy::Kind::Iterator:
       case FixitStrategy::Kind::Vector:
@@ -2462,6 +2463,71 @@ static FixItList fixVariableWithSpan(const VarDecl *VD,
   return fixLocalVarDeclWithSpan(VD, Ctx, getUserFillPlaceHolder(), Handler);
 }
 
+static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
+                                     UnsafeBufferUsageHandler &Handler) {
+  FixItList FixIts{};
+
+  if (auto CAT = dyn_cast<clang::ConstantArrayType>(D->getType())) {
+    const QualType &ArrayEltT = CAT->getElementType();
+    assert(!ArrayEltT.isNull() && "Trying to fix a non-array type variable!");
+    // Producing fix-it for variable declaration---make `D` to be of std::array
+    // type:
+    SmallString<32> Replacement;
+    raw_svector_ostream OS(Replacement);
+
+    // For most types the transformation is simple:
+    //   T foo[10]; => std::array<T, 10> foo;
+    // Cv-specifiers are straigtforward:
+    //   const T foo[10]; => std::array<const T, 10> foo;
+    // Pointers are straightforward:
+    //   T * foo[10]; => std::array<T *, 10> foo;
+    //
+    // However, for const pointers the transformation is different:
+    //   T * const foo[10]; => const std::array<T *, 10> foo;
+    if (ArrayEltT->isPointerType() && ArrayEltT.isConstQualified()) {
+      DEBUG_NOTE_DECL_FAIL(D, " : const size array of const pointers");
+      // FIXME: implement the support
+      // FIXME: bail if the const pointer is a typedef
+      return {};
+    }
+
+    std::optional<StringRef> IdentText =
+        getVarDeclIdentifierText(D, Ctx.getSourceManager(), Ctx.getLangOpts());
+
+    if (!IdentText) {
+      DEBUG_NOTE_DECL_FAIL(D, " : failed to locate the identifier");
+      return {};
+    }
+
+    OS << "std::array<" << ArrayEltT.getAsString() << ", "
+       << getAPIntText(CAT->getSize()) << "> " << IdentText->str();
+
+    FixIts.push_back(FixItHint::CreateReplacement(
+        SourceRange{D->getBeginLoc(), D->getTypeSpecEndLoc()}, OS.str()));
+  }
+
+  return FixIts;
+}
+
+static FixItList fixVariableWithArray(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 fixVarDeclWithArray(VD, Ctx, Handler);
+}
+
 // TODO: we should be consistent to use `std::nullopt` to represent no-fix due
 // to any unexpected problem.
 static FixItList
@@ -2507,7 +2573,13 @@ fixVariable(const VarDecl *VD, FixitStrategy::Kind K,
     DEBUG_NOTE_DECL_FAIL(VD, " : not a pointer");
     return {};
   }
-  case FixitStrategy::Kind::Array:
+  case FixitStrategy::Kind::Array: {
+    if (VD->isLocalVarDecl() && isa<clang::ConstantArrayType>(VD->getType()))
+      return fixVariableWithArray(VD, Tracker, Ctx, Handler);
+
+    DEBUG_NOTE_DECL_FAIL(VD, " : not a local const-size array");
+    return {};
+  }
   case FixitStrategy::Kind::Iterator:
   case FixitStrategy::Kind::Vector:
     llvm_unreachable("FixitStrategy not implemented yet!");
@@ -2696,7 +2768,10 @@ static FixitStrategy
 getNaiveStrategy(llvm::iterator_range<VarDeclIterTy> UnsafeVars) {
   FixitStrategy S;
   for (const VarDecl *VD : UnsafeVars) {
-    S.set(VD, FixitStrategy::Kind::Span);
+    if (isa<ConstantArrayType>(VD->getType()))
+      S.set(VD, FixitStrategy::Kind::Array);
+    else
+      S.set(VD, FixitStrategy::Kind::Span);
   }
   return S;
 }
@@ -3018,9 +3093,9 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
     auto FixItsIt = FixItsForVariableGroup.find(VD);
     Handler.handleUnsafeVariableGroup(VD, VarGrpMgr,
                                       FixItsIt != FixItsForVariableGroup.end()
-                                      ? std::move(FixItsIt->second)
-                                      : FixItList{},
-                                      D);
+                                          ? std::move(FixItsIt->second)
+                                          : FixItList{},
+                                      D, NaiveStrategy);
     for (const auto &G : WarningGadgets) {
       Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true,
                                     D->getASTContext());
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 78b9f324e1390..8239ba49429d3 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2297,7 +2297,8 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
 
   void handleUnsafeVariableGroup(const VarDecl *Variable,
                                  const VariableGroupsManager &VarGrpMgr,
-                                 FixItList &&Fixes, const Decl *D) override {
+                                 FixItList &&Fixes, const Decl *D,
+                                 const FixitStrategy &VarTargetTypes) override {
     assert(!SuggestSuggestions &&
            "Unsafe buffer usage fixits displayed without suggestions!");
     S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable)
@@ -2312,7 +2313,18 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
       // NOT explain how the variables are grouped as the reason is non-trivial
       // and irrelavant to users' experience:
       const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Variable, &BriefMsg);
-      unsigned FixItStrategy = 0; // For now we only have 'std::span' strategy
+      unsigned FixItStrategy = 0;
+      switch (VarTargetTypes.lookup(Variable)) {
+      case clang::FixitStrategy::Kind::Span:
+        FixItStrategy = 0;
+        break;
+      case clang::FixitStrategy::Kind::Array:
+        FixItStrategy = 1;
+        break;
+      default:
+        assert(false && "We support only std::span and std::array");
+      };
+
       const auto &FD =
           S.Diag(Variable->getLocation(),
                  BriefMsg ? diag::note_unsafe_buffer_variable_fixit_together
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
new file mode 100644
index 0000000000000..12fd5c69c78f0
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -verify %s
+
+// CHECK-NOT: [-Wunsafe-buffer-usage]
+
+
+void foo(unsigned idx) {
+  int buffer[10];         // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds}}
+                          // expected-note at -1{{change type of 'buffer' to 'std::array' to preserve bounds information}}
+  buffer[idx] = 0;        // expected-note{{used in buffer access here}}
+}
+
+int global_buffer[10];    // expected-warning{{'global_buffer' is an unsafe buffer that does not perform bounds}}
+void foo2(unsigned idx) {
+  global_buffer[idx] = 0;        // expected-note{{used in buffer access here}}
+}
+
+struct Foo {
+  int member_buffer[10];
+};
+void foo2(Foo& f, unsigned idx) {
+  f.member_buffer[idx] = 0; // expected-warning{{unsafe buffer access}}
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
index e08f70d97e391..518fafcda9d37 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
@@ -32,13 +32,11 @@ void foo() {
                         // debug-note{{safe buffers debug: gadget 'ULCArraySubscript' refused to produce a fix}}
 }
 
-void failed_decl() {
-  int a[10];  // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}} \
-              // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : not a pointer}}
-  
-  for (int i = 0; i < 10; i++) {
-    a[i] = i;  // expected-note{{used in buffer access here}}
-  }
+void failed_decl(const int* out, unsigned idx) {
+  const int* const a[10] = {nullptr};  // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}} \
+              // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : const size array of const pointers}}
+
+  out = a[idx];  // expected-note{{used in buffer access here}}
 }
 
 void failed_multiple_decl() {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
new file mode 100644
index 0000000000000..a14a88ac08636
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -0,0 +1,95 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+typedef int * Int_ptr_t;
+typedef int Int_t;
+
+void local_array(unsigned idx) {
+  int buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
+  buffer[idx] = 0;
+}
+
+void unsupported_multi_decl1(unsigned idx) {
+  int a, buffer[10];
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+  buffer[idx] = 0;
+}
+
+void unsupported_multi_decl2(unsigned idx) {
+  int buffer[10], b;
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+  buffer[idx] = 0;
+}
+
+void local_array_ptr_to_const(unsigned idx, const int*& a) {
+  const int * buffer[10] = {a};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:25}:"std::array<const int *, 10> buffer"
+  a = buffer[idx];
+}
+
+void local_array_const_ptr(unsigned idx, int*& a) {
+  int * const buffer[10] = {a};
+// FIXME: implement support
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+  a = buffer[idx];
+
+}
+
+void local_array_const_ptr_to_const(unsigned idx, const int*& a) {
+  const int * const buffer[10] = {a};
+// FIXME: implement support
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+  a = buffer[idx];
+
+}
+
+template<typename T>
+void local_array_in_template(unsigned idx) {
+  T buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+  buffer[idx] = 0;
+}
+// Instantiate the template function to force its analysis.
+template void local_array_in_template<int>(unsigned); // FIXME: expected note {{in instantiation of}}
+
+void macro_as_identifier(unsigned idx) {
+#define MY_BUFFER buffer
+  // Although fix-its include macros, the macros do not overlap with
+  // the bounds of the source range of these fix-its. So these fix-its
+  // are valid.
+
+  int MY_BUFFER[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:20}:"std::array<int, 10> MY_BUFFER"
+  MY_BUFFER[idx] = 0;
+
+#undef MY_BUFFER
+}
+
+void unsupported_fixit_overlapping_macro(unsigned idx) {
+#define MY_INT int
+  MY_INT buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+  buffer[idx] = 0;
+#undef MY_INT
+}
+
+void subscript_negative() {
+  int buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
+
+  // For constant-size arrays any negative index will lead to buffer underflow.
+  // std::array::operator[] has unsigned parameter so the value will be casted to unsigned.
+  // This will very likely be buffer overflow but hardened std::array catch these at runtime.
+  buffer[-5] = 0;
+}
+
+void subscript_signed(int signed_idx) {
+  int buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
+
+  // For constant-size arrays any negative index will lead to buffer underflow.
+  // std::array::operator[] has unsigned parameter so the value will be casted to unsigned.
+  // This will very likely be buffer overflow but hardened std::array catch these at runtime.
+  buffer[signed_idx] = 0;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index c5f0a9ef92937..a70d4bda45c72 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -61,6 +61,7 @@ void testArraySubscripts(int *p, int **pp) {
       );
 
     int a[10];          // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
+                        // expected-note at -1{{change type of 'a' to 'std::array' to preserve bounds information}}
     int b[10][10];      // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
 
   foo(a[1], 1[a],   // expected-note2{{used in buffer access here}}
@@ -174,6 +175,7 @@ auto file_scope_lambda = [](int *ptr) {
 void testLambdaCapture() {
   int a[10];              // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
   int b[10];              // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
+                          // expected-note at -1{{change type of 'b' to 'std::array' to preserve bounds information}}
   int c[10];
 
   auto Lam1 = [a]() {
@@ -191,7 +193,9 @@ void testLambdaCapture() {
 
 void testLambdaImplicitCapture() {
   int a[10];              // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
+                          // expected-note at -1{{change type of 'a' to 'std::array' to preserve bounds information}}
   int b[10];              // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
+                          // expected-note at -1{{change type of 'b' to 'std::array' to preserve bounds information}}
   
   auto Lam1 = [=]() {
     return a[1];           // expected-note{{used in buffer access here}}
@@ -344,6 +348,7 @@ template<typename T> void fArr(T t[]) {
   // expected-warning at -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}}
+                // expected-note at -1{{change type of 'ar' to 'std::array' to preserve bounds information}}
   foo(ar[5]);   // expected-note{{used in buffer access here}}
 }
 

>From 83e31ad1c55ffa7967e940da18c534bce5bcf8ef Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Wed, 24 Jan 2024 14:22:01 -0800
Subject: [PATCH 03/13] [-Wunsafe-buffer-usage] Don't assert Array strategy is
 unimplemented

---
 clang/lib/Analysis/UnsafeBufferUsage.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6c99a23aeb9e8..7b9edbd605650 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1442,6 +1442,7 @@ PointerAssignmentGadget::getFixits(const FixitStrategy &S) const {
     return std::nullopt;
   case FixitStrategy::Kind::Iterator:
   case FixitStrategy::Kind::Array:
+    return std::nullopt;
   case FixitStrategy::Kind::Vector:
     llvm_unreachable("unsupported strategies for FixableGadgets");
   }
@@ -1461,6 +1462,7 @@ PointerInitGadget::getFixits(const FixitStrategy &S) const {
     return std::nullopt;
   case FixitStrategy::Kind::Iterator:
   case FixitStrategy::Kind::Array:
+    return std::nullopt;
   case FixitStrategy::Kind::Vector:
     llvm_unreachable("unsupported strategies for FixableGadgets");
   }
@@ -1519,6 +1521,7 @@ UPCAddressofArraySubscriptGadget::getFixits(const FixitStrategy &S) const {
   case FixitStrategy::Kind::Wontfix:
   case FixitStrategy::Kind::Iterator:
   case FixitStrategy::Kind::Array:
+    return std::nullopt;
   case FixitStrategy::Kind::Vector:
     llvm_unreachable("unsupported strategies for FixableGadgets");
   }
@@ -1853,6 +1856,7 @@ PointerDereferenceGadget::getFixits(const FixitStrategy &S) const {
   }
   case FixitStrategy::Kind::Iterator:
   case FixitStrategy::Kind::Array:
+    return std::nullopt;
   case FixitStrategy::Kind::Vector:
     llvm_unreachable("FixitStrategy not implemented yet!");
   case FixitStrategy::Kind::Wontfix:
@@ -1883,6 +1887,7 @@ UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const {
     case FixitStrategy::Kind::Wontfix:
     case FixitStrategy::Kind::Iterator:
     case FixitStrategy::Kind::Array:
+      return std::nullopt;
     case FixitStrategy::Kind::Vector:
       llvm_unreachable("unsupported strategies for FixableGadgets");
   }

>From da47129c2987295afbc5432e4199bcbefaa115e7 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Fri, 26 Jan 2024 16:22:50 -0800
Subject: [PATCH 04/13] [-Wunsafe-buffer-usage] Preserve spelling of array size
 in std::array fixits

Fixits related to arrays generally change type from T [N] to
std::array<T, N>. N has to be constant but doesn't have to be an integer
literal. It can be for example a constant.

The fixits need to preserve the spelling of the array size.
The assumption is that not specifying the numerical value directly was a
conscious decision of the original author and it'll very likely still apply to
the transformed code.

E. g. for
int buffer[SIZE];

if `buffer` is unsafe it should become:
std::array<int, SIZE> buffer;

rather than:
std::array<int, 42> buffer;
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 23 ++++++++++---
 ...fe-buffer-usage-fixits-local-var-array.cpp | 32 +++++++++++++------
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 7b9edbd605650..9ad532aedfa20 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -2475,10 +2475,6 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
   if (auto CAT = dyn_cast<clang::ConstantArrayType>(D->getType())) {
     const QualType &ArrayEltT = CAT->getElementType();
     assert(!ArrayEltT.isNull() && "Trying to fix a non-array type variable!");
-    // Producing fix-it for variable declaration---make `D` to be of std::array
-    // type:
-    SmallString<32> Replacement;
-    raw_svector_ostream OS(Replacement);
 
     // For most types the transformation is simple:
     //   T foo[10]; => std::array<T, 10> foo;
@@ -2496,6 +2492,21 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
       return {};
     }
 
+    // Find the '[' token.
+    std::optional<Token> NextTok = Lexer::findNextToken(getVarDeclIdentifierLoc(D), Ctx.getSourceManager(), Ctx.getLangOpts());
+    while (NextTok && !NextTok->is(tok::l_square))
+      NextTok = Lexer::findNextToken(NextTok->getLocation(), Ctx.getSourceManager(), Ctx.getLangOpts());
+    if (!NextTok)
+      return {};
+    const SourceLocation LSqBracketLoc = NextTok->getLocation();
+
+    // Get the spelling of the array size as written in the source file (including macros, etc.).
+    auto MaybeArraySizeTxt = getRangeText({LSqBracketLoc.getLocWithOffset(1), D->getTypeSpecEndLoc()}, Ctx.getSourceManager(), Ctx.getLangOpts());
+    if (!MaybeArraySizeTxt)
+      return {};
+
+    const std::string ArraySizeTxt = MaybeArraySizeTxt->str();
+
     std::optional<StringRef> IdentText =
         getVarDeclIdentifierText(D, Ctx.getSourceManager(), Ctx.getLangOpts());
 
@@ -2504,8 +2515,10 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
       return {};
     }
 
+    SmallString<32> Replacement;
+    raw_svector_ostream OS(Replacement);
     OS << "std::array<" << ArrayEltT.getAsString() << ", "
-       << getAPIntText(CAT->getSize()) << "> " << IdentText->str();
+       << ArraySizeTxt << "> " << IdentText->str();
 
     FixIts.push_back(FixItHint::CreateReplacement(
         SourceRange{D->getBeginLoc(), D->getTypeSpecEndLoc()}, OS.str()));
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index a14a88ac08636..6f6b1a8aa5c66 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -10,6 +10,18 @@ void local_array(unsigned idx) {
   buffer[idx] = 0;
 }
 
+void weird_whitespace_in_declaration(unsigned idx) {
+  int      buffer_w   [       10 ] ;
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:35}:"std::array<int,        10 > buffer_w"
+  buffer_w[idx] = 0;
+}
+
+void weird_comments_in_declaration(unsigned idx) {
+  int   /* [ ] */   buffer_w  /* [ ] */ [ /* [ ] */ 10 /* [ ] */ ] ;
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int,  /* [ ] */ 10 /* [ ] */ > buffer_w"
+  buffer_w[idx] = 0;
+}
+
 void unsupported_multi_decl1(unsigned idx) {
   int a, buffer[10];
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
@@ -55,23 +67,25 @@ template void local_array_in_template<int>(unsigned); // FIXME: expected note {{
 
 void macro_as_identifier(unsigned idx) {
 #define MY_BUFFER buffer
-  // Although fix-its include macros, the macros do not overlap with
-  // the bounds of the source range of these fix-its. So these fix-its
-  // are valid.
-
   int MY_BUFFER[10];
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:20}:"std::array<int, 10> MY_BUFFER"
   MY_BUFFER[idx] = 0;
-
 #undef MY_BUFFER
 }
 
-void unsupported_fixit_overlapping_macro(unsigned idx) {
-#define MY_INT int
-  MY_INT buffer[10];
+void macro_as_size(unsigned idx) {
+#define MY_TEN 10
+  int buffer[MY_TEN];
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
   buffer[idx] = 0;
-#undef MY_INT
+#undef MY_TEN
+}
+
+void constant_as_size(unsigned idx) {
+  const unsigned my_const = 10;
+  int buffer[my_const];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:23}:"std::array<int, my_const> buffer"
+  buffer[idx] = 0;
 }
 
 void subscript_negative() {

>From e3c2483fa637ac5e0b16f137e658bae8561691b9 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Fri, 26 Jan 2024 16:52:41 -0800
Subject: [PATCH 05/13] [-Wunsafe-buffer-usage] Preserve array element type
 spelling in std::array fixits

In other words - don't use the type that the source code gets expanded
to in the AST as there's very likely a reason why it is represented via
a typedef or macro in the sourcode.
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 23 +++++++++++++++++--
 ...fe-buffer-usage-fixits-local-var-array.cpp | 19 +++++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 9ad532aedfa20..1a06eff32f039 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -12,6 +12,8 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/APSInt.h"
@@ -2492,8 +2494,25 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
       return {};
     }
 
+    const SourceLocation IdentifierLoc = getVarDeclIdentifierLoc(D);
+
+    // Get the spelling of the element type as written in the source file (including macros, etc.).
+    auto MaybeElemTypeTxt = getRangeText({D->getBeginLoc(), IdentifierLoc}, Ctx.getSourceManager(), Ctx.getLangOpts());
+    if (!MaybeElemTypeTxt)
+      return {};
+    std::string ElemTypeTxt = MaybeElemTypeTxt->str();
+    // Trim whitespace from the type spelling.
+    unsigned TrailingWhitespace = 0;
+    for (auto It = ElemTypeTxt.rbegin(); It < ElemTypeTxt.rend(); ++It) {
+      if (!isWhitespace(*It))
+        break;
+      ++TrailingWhitespace;
+    }
+    if (TrailingWhitespace > 0)
+      ElemTypeTxt.erase(ElemTypeTxt.length() - TrailingWhitespace);
+
     // Find the '[' token.
-    std::optional<Token> NextTok = Lexer::findNextToken(getVarDeclIdentifierLoc(D), Ctx.getSourceManager(), Ctx.getLangOpts());
+    std::optional<Token> NextTok = Lexer::findNextToken(IdentifierLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
     while (NextTok && !NextTok->is(tok::l_square))
       NextTok = Lexer::findNextToken(NextTok->getLocation(), Ctx.getSourceManager(), Ctx.getLangOpts());
     if (!NextTok)
@@ -2517,7 +2536,7 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
 
     SmallString<32> Replacement;
     raw_svector_ostream OS(Replacement);
-    OS << "std::array<" << ArrayEltT.getAsString() << ", "
+    OS << "std::array<" << ElemTypeTxt << ", "
        << ArraySizeTxt << "> " << IdentText->str();
 
     FixIts.push_back(FixItHint::CreateReplacement(
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index 6f6b1a8aa5c66..2b44e238b03ad 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -18,7 +18,7 @@ void weird_whitespace_in_declaration(unsigned idx) {
 
 void weird_comments_in_declaration(unsigned idx) {
   int   /* [ ] */   buffer_w  /* [ ] */ [ /* [ ] */ 10 /* [ ] */ ] ;
-// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int,  /* [ ] */ 10 /* [ ] */ > buffer_w"
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int   /* [ ] */,  /* [ ] */ 10 /* [ ] */ > buffer_w"
   buffer_w[idx] = 0;
 }
 
@@ -65,6 +65,21 @@ void local_array_in_template(unsigned idx) {
 // Instantiate the template function to force its analysis.
 template void local_array_in_template<int>(unsigned); // FIXME: expected note {{in instantiation of}}
 
+typedef unsigned int uint;
+void typedef_as_elem_type(unsigned idx) {
+  uint buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:18}:"std::array<uint, 10> buffer"
+  buffer[idx] = 0;
+}
+
+void macro_as_elem_type(unsigned idx) {
+#define MY_INT int
+  MY_INT buffer[10];
+// FIXME: implement support
+  buffer[idx] = 0;
+#undef MY_INT
+}
+
 void macro_as_identifier(unsigned idx) {
 #define MY_BUFFER buffer
   int MY_BUFFER[10];
@@ -76,7 +91,7 @@ void macro_as_identifier(unsigned idx) {
 void macro_as_size(unsigned idx) {
 #define MY_TEN 10
   int buffer[MY_TEN];
-// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:21}:"std::array<int, MY_TEN> buffer"
   buffer[idx] = 0;
 #undef MY_TEN
 }

>From d19a2c30194d33c0a6602b9cd59f09e61e93b9bc Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Mon, 29 Jan 2024 12:54:10 -0800
Subject: [PATCH 06/13] [-Wunsafe-buffer-usage] Fix whitespace handling in
 array types

Specifically applies to text with array element type and array size for
constant size arrays. Trimming there strings on both sides and using
llvm::StringRef::trim().
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp          | 15 +++------------
 ...unsafe-buffer-usage-fixits-local-var-array.cpp | 12 ++++++------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 1a06eff32f039..6570d339c3494 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -18,6 +18,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include <memory>
 #include <optional>
 #include <queue>
@@ -2500,16 +2501,7 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
     auto MaybeElemTypeTxt = getRangeText({D->getBeginLoc(), IdentifierLoc}, Ctx.getSourceManager(), Ctx.getLangOpts());
     if (!MaybeElemTypeTxt)
       return {};
-    std::string ElemTypeTxt = MaybeElemTypeTxt->str();
-    // Trim whitespace from the type spelling.
-    unsigned TrailingWhitespace = 0;
-    for (auto It = ElemTypeTxt.rbegin(); It < ElemTypeTxt.rend(); ++It) {
-      if (!isWhitespace(*It))
-        break;
-      ++TrailingWhitespace;
-    }
-    if (TrailingWhitespace > 0)
-      ElemTypeTxt.erase(ElemTypeTxt.length() - TrailingWhitespace);
+    const llvm::StringRef ElemTypeTxt = MaybeElemTypeTxt->trim();
 
     // Find the '[' token.
     std::optional<Token> NextTok = Lexer::findNextToken(IdentifierLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
@@ -2523,8 +2515,7 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
     auto MaybeArraySizeTxt = getRangeText({LSqBracketLoc.getLocWithOffset(1), D->getTypeSpecEndLoc()}, Ctx.getSourceManager(), Ctx.getLangOpts());
     if (!MaybeArraySizeTxt)
       return {};
-
-    const std::string ArraySizeTxt = MaybeArraySizeTxt->str();
+    const llvm::StringRef ArraySizeTxt = MaybeArraySizeTxt->trim();
 
     std::optional<StringRef> IdentText =
         getVarDeclIdentifierText(D, Ctx.getSourceManager(), Ctx.getLangOpts());
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index 2b44e238b03ad..052a085395055 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -10,15 +10,15 @@ void local_array(unsigned idx) {
   buffer[idx] = 0;
 }
 
-void weird_whitespace_in_declaration(unsigned idx) {
-  int      buffer_w   [       10 ] ;
-// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:35}:"std::array<int,        10 > buffer_w"
+void whitespace_in_declaration(unsigned idx) {
+  int      buffer_w   [       10 ];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:35}:"std::array<int, 10> buffer_w"
   buffer_w[idx] = 0;
 }
 
-void weird_comments_in_declaration(unsigned idx) {
-  int   /* [ ] */   buffer_w  /* [ ] */ [ /* [ ] */ 10 /* [ ] */ ] ;
-// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int   /* [ ] */,  /* [ ] */ 10 /* [ ] */ > buffer_w"
+void comments_in_declaration(unsigned idx) {
+  int   /* [A] */   buffer_w  /* [B] */ [  /* [C] */ 10 /* [D] */  ] ;
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:69}:"std::array<int   /* [A] */, /* [C] */ 10 /* [D] */> buffer_w"
   buffer_w[idx] = 0;
 }
 

>From 849f6431dcf14e7b245615c8b8e19256c1f41f48 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Mon, 29 Jan 2024 13:24:24 -0800
Subject: [PATCH 07/13] [-Wnsafe-buffer-usage][NFC] Fix and refactor tests

---
 ...fe-buffer-usage-fixits-local-var-array.cpp | 32 ++++++++++++-------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index 052a085395055..c39c9fd754071 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -4,7 +4,7 @@
 typedef int * Int_ptr_t;
 typedef int Int_t;
 
-void local_array(unsigned idx) {
+void simple(unsigned idx) {
   int buffer[10];
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
   buffer[idx] = 0;
@@ -22,15 +22,19 @@ void comments_in_declaration(unsigned idx) {
   buffer_w[idx] = 0;
 }
 
-void unsupported_multi_decl1(unsigned idx) {
+void multi_decl1(unsigned idx) {
   int a, buffer[10];
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
   buffer[idx] = 0;
 }
 
-void unsupported_multi_decl2(unsigned idx) {
+void multi_decl2(unsigned idx) {
   int buffer[10], b;
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
   buffer[idx] = 0;
 }
 
@@ -42,40 +46,44 @@ void local_array_ptr_to_const(unsigned idx, const int*& a) {
 
 void local_array_const_ptr(unsigned idx, int*& a) {
   int * const buffer[10] = {a};
-// FIXME: implement support
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+// FIXME: implement support
+
   a = buffer[idx];
 
 }
 
 void local_array_const_ptr_to_const(unsigned idx, const int*& a) {
   const int * const buffer[10] = {a};
-// FIXME: implement support
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+// FIXME: implement support
+
   a = buffer[idx];
 
 }
 
 template<typename T>
-void local_array_in_template(unsigned idx) {
+void unsupported_local_array_in_template(unsigned idx) {
   T buffer[10];
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
   buffer[idx] = 0;
 }
 // Instantiate the template function to force its analysis.
-template void local_array_in_template<int>(unsigned); // FIXME: expected note {{in instantiation of}}
+template void unsupported_local_array_in_template<int>(unsigned);
 
-typedef unsigned int uint;
+typedef unsigned int my_uint;
 void typedef_as_elem_type(unsigned idx) {
-  uint buffer[10];
-// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:18}:"std::array<uint, 10> buffer"
+  my_uint buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:21}:"std::array<my_uint, 10> buffer"
   buffer[idx] = 0;
 }
 
 void macro_as_elem_type(unsigned idx) {
 #define MY_INT int
   MY_INT buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
 // FIXME: implement support
+
   buffer[idx] = 0;
 #undef MY_INT
 }
@@ -119,6 +127,6 @@ void subscript_signed(int signed_idx) {
 
   // For constant-size arrays any negative index will lead to buffer underflow.
   // std::array::operator[] has unsigned parameter so the value will be casted to unsigned.
-  // This will very likely be buffer overflow but hardened std::array catch these at runtime.
+  // This will very likely be buffer overflow but hardened std::array catches these at runtime.
   buffer[signed_idx] = 0;
 }

>From b6688fb091b86a365ace76a264ec6766d934a116 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Mon, 29 Jan 2024 14:39:14 -0800
Subject: [PATCH 08/13] [-Wunsafe-buffer-usage] Bail on fixits for arrays whose
 size is auto-deduced

C++ allows the array size to be omitted in declaration if it can be deduced from the
initializer.
E. g.:
int array [] = { 1, 2, 3 };

std::array<T, N> on the other hand requires the size N to be provided.

Currently the fixit produced would use empty space for N which would lead to a
compilation failure. This patch makes the fixit-producing machine just
bail in such cases and adds a FIXME to be addressed later.
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp         |  9 +++++++++
 ...nsafe-buffer-usage-fixits-local-var-array.cpp | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6570d339c3494..18d0ca7a17d3c 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -2516,6 +2516,15 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
     if (!MaybeArraySizeTxt)
       return {};
     const llvm::StringRef ArraySizeTxt = MaybeArraySizeTxt->trim();
+    if (ArraySizeTxt.empty()) {
+      // FIXME: Support array size getting determined from the initializer.
+      // Examples:
+      //    int arr1[] = {0, 1, 2};
+      //    int arr2{3, 4, 5};
+      // We might be able to preserve the non-specified size with `auto` and `std::to_array`:
+      //    auto arr1 = std::to_array<int>({0, 1, 2});
+      return {};
+    }
 
     std::optional<StringRef> IdentText =
         getVarDeclIdentifierText(D, Ctx.getSourceManager(), Ctx.getLangOpts());
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index c39c9fd754071..c0688380c08a4 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -22,6 +22,22 @@ void comments_in_declaration(unsigned idx) {
   buffer_w[idx] = 0;
 }
 
+void auto_size(unsigned idx) {
+  int buffer[] = {0, 1, 2};
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
+  buffer[idx] = 0;
+}
+
+void universal_initialization(unsigned idx) {
+  int buffer[] {0, 1, 2};
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
+  buffer[idx] = 0;
+}
+
 void multi_decl1(unsigned idx) {
   int a, buffer[10];
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]

>From fd791ccdbf919c4810a92e700665a4766ef402a7 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Mon, 29 Jan 2024 14:39:58 -0800
Subject: [PATCH 09/13] [-Wunsafe-buffer-usage][NFC] Add testcases

---
 ...fe-buffer-usage-fixits-local-var-array.cpp | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index c0688380c08a4..7e83d8e68a9ee 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -22,6 +22,17 @@ void comments_in_declaration(unsigned idx) {
   buffer_w[idx] = 0;
 }
 
+void initializer(unsigned idx) {
+  int buffer[3] = {0};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:16}:"std::array<int, 3> buffer"
+
+  int buffer2[3] = {0, 1, 2};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 3> buffer2"
+
+  buffer[idx] = 0;
+  buffer2[idx] = 0;
+}
+
 void auto_size(unsigned idx) {
   int buffer[] = {0, 1, 2};
 // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
@@ -66,7 +77,15 @@ void local_array_const_ptr(unsigned idx, int*& a) {
 // FIXME: implement support
 
   a = buffer[idx];
+}
 
+void local_array_const_ptr_via_typedef(unsigned idx, int*& a) {
+  typedef int * const my_const_ptr;
+  my_const_ptr buffer[10] = {a};
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+// FIXME: implement support
+
+  a = buffer[idx];
 }
 
 void local_array_const_ptr_to_const(unsigned idx, const int*& a) {

>From 9183a63f3aed8e27563381d037bec0e170cbf795 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Wed, 31 Jan 2024 14:13:58 -0800
Subject: [PATCH 10/13] [-Wunsafe-buffer-usage] Clang-format std::array fixits
 work

---
 clang/lib/Analysis/UnsafeBufferUsage.cpp | 42 ++++++++++++++----------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 18d0ca7a17d3c..30723b97f6268 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1886,13 +1886,13 @@ UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const {
       return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
     // FIXME: Points inside a macro expansion.
     break;
-    }
-    case FixitStrategy::Kind::Wontfix:
-    case FixitStrategy::Kind::Iterator:
-    case FixitStrategy::Kind::Array:
-      return std::nullopt;
-    case FixitStrategy::Kind::Vector:
-      llvm_unreachable("unsupported strategies for FixableGadgets");
+  }
+  case FixitStrategy::Kind::Wontfix:
+  case FixitStrategy::Kind::Iterator:
+  case FixitStrategy::Kind::Array:
+    return std::nullopt;
+  case FixitStrategy::Kind::Vector:
+    llvm_unreachable("unsupported strategies for FixableGadgets");
   }
 
   return std::nullopt;
@@ -2008,7 +2008,6 @@ UPCPreIncrementGadget::getFixits(const FixitStrategy &S) const {
   return std::nullopt; // Not in the cases that we can handle for now, give up.
 }
 
-
 // For a non-null initializer `Init` of `T *` type, this function returns
 // `FixItHint`s producing a list initializer `{Init,  S}` as a part of a fix-it
 // to output stream.
@@ -2497,22 +2496,30 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
 
     const SourceLocation IdentifierLoc = getVarDeclIdentifierLoc(D);
 
-    // Get the spelling of the element type as written in the source file (including macros, etc.).
-    auto MaybeElemTypeTxt = getRangeText({D->getBeginLoc(), IdentifierLoc}, Ctx.getSourceManager(), Ctx.getLangOpts());
+    // Get the spelling of the element type as written in the source file
+    // (including macros, etc.).
+    auto MaybeElemTypeTxt =
+        getRangeText({D->getBeginLoc(), IdentifierLoc}, Ctx.getSourceManager(),
+                     Ctx.getLangOpts());
     if (!MaybeElemTypeTxt)
       return {};
     const llvm::StringRef ElemTypeTxt = MaybeElemTypeTxt->trim();
 
     // Find the '[' token.
-    std::optional<Token> NextTok = Lexer::findNextToken(IdentifierLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
+    std::optional<Token> NextTok = Lexer::findNextToken(
+        IdentifierLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
     while (NextTok && !NextTok->is(tok::l_square))
-      NextTok = Lexer::findNextToken(NextTok->getLocation(), Ctx.getSourceManager(), Ctx.getLangOpts());
+      NextTok = Lexer::findNextToken(NextTok->getLocation(),
+                                     Ctx.getSourceManager(), Ctx.getLangOpts());
     if (!NextTok)
       return {};
     const SourceLocation LSqBracketLoc = NextTok->getLocation();
 
-    // Get the spelling of the array size as written in the source file (including macros, etc.).
-    auto MaybeArraySizeTxt = getRangeText({LSqBracketLoc.getLocWithOffset(1), D->getTypeSpecEndLoc()}, Ctx.getSourceManager(), Ctx.getLangOpts());
+    // Get the spelling of the array size as written in the source file
+    // (including macros, etc.).
+    auto MaybeArraySizeTxt = getRangeText(
+        {LSqBracketLoc.getLocWithOffset(1), D->getTypeSpecEndLoc()},
+        Ctx.getSourceManager(), Ctx.getLangOpts());
     if (!MaybeArraySizeTxt)
       return {};
     const llvm::StringRef ArraySizeTxt = MaybeArraySizeTxt->trim();
@@ -2521,7 +2528,8 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
       // Examples:
       //    int arr1[] = {0, 1, 2};
       //    int arr2{3, 4, 5};
-      // We might be able to preserve the non-specified size with `auto` and `std::to_array`:
+      // We might be able to preserve the non-specified size with `auto` and
+      // `std::to_array`:
       //    auto arr1 = std::to_array<int>({0, 1, 2});
       return {};
     }
@@ -2536,8 +2544,8 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
 
     SmallString<32> Replacement;
     raw_svector_ostream OS(Replacement);
-    OS << "std::array<" << ElemTypeTxt << ", "
-       << ArraySizeTxt << "> " << IdentText->str();
+    OS << "std::array<" << ElemTypeTxt << ", " << ArraySizeTxt << "> "
+       << IdentText->str();
 
     FixIts.push_back(FixItHint::CreateReplacement(
         SourceRange{D->getBeginLoc(), D->getTypeSpecEndLoc()}, OS.str()));

>From 04abb733381bdde6aa823a527b7dbd3b4ced7b85 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Wed, 31 Jan 2024 17:04:30 -0800
Subject: [PATCH 11/13] [-Wunsafe-buffer-usage] Fix note message suggesting to
 use std::array

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td      |  4 ++--
 clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp |  6 +++---
 clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp       | 10 +++++-----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c0ebbe4e6834..c2239f9785944 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12113,9 +12113,9 @@ def warn_unsafe_buffer_operation : Warning<
 def note_unsafe_buffer_operation : Note<
   "used%select{| in pointer arithmetic| in buffer access}0 here">;
 def note_unsafe_buffer_variable_fixit_group : Note<
-  "change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">;
+  "change type of %0 to '%select{std::span' to preserve bounds information|std::array' to harden it|std::span::iterator' to preserve bounds information}1%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">;
 def note_unsafe_buffer_variable_fixit_together : Note<
-  "change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information"
+  "change type of %0 to '%select{std::span' to preserve bounds information|std::array' to harden it|std::span::iterator' to preserve bounds information}1"
   "%select{|, and change %2 to safe types to make function %4 bounds-safe}3">;
 def note_safe_buffer_usage_suggestions_disabled : Note<
   "pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 12fd5c69c78f0..ed12f360dc445 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -6,12 +6,12 @@
 
 
 void foo(unsigned idx) {
-  int buffer[10];         // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds}}
-                          // expected-note at -1{{change type of 'buffer' to 'std::array' to preserve bounds information}}
+  int buffer[10];         // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}}
+                          // expected-note at -1{{change type of 'buffer' to 'std::array' to harden it}}
   buffer[idx] = 0;        // expected-note{{used in buffer access here}}
 }
 
-int global_buffer[10];    // expected-warning{{'global_buffer' is an unsafe buffer that does not perform bounds}}
+int global_buffer[10];    // expected-warning{{'global_buffer' is an unsafe buffer that does not perform bounds checks}}
 void foo2(unsigned idx) {
   global_buffer[idx] = 0;        // expected-note{{used in buffer access here}}
 }
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index a70d4bda45c72..6226114bad7a3 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -61,7 +61,7 @@ void testArraySubscripts(int *p, int **pp) {
       );
 
     int a[10];          // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
-                        // expected-note at -1{{change type of 'a' to 'std::array' to preserve bounds information}}
+                        // expected-note at -1{{change type of 'a' to 'std::array' to harden it}}
     int b[10][10];      // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
 
   foo(a[1], 1[a],   // expected-note2{{used in buffer access here}}
@@ -175,7 +175,7 @@ auto file_scope_lambda = [](int *ptr) {
 void testLambdaCapture() {
   int a[10];              // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
   int b[10];              // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
-                          // expected-note at -1{{change type of 'b' to 'std::array' to preserve bounds information}}
+                          // expected-note at -1{{change type of 'b' to 'std::array' to harden it}}
   int c[10];
 
   auto Lam1 = [a]() {
@@ -193,9 +193,9 @@ void testLambdaCapture() {
 
 void testLambdaImplicitCapture() {
   int a[10];              // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
-                          // expected-note at -1{{change type of 'a' to 'std::array' to preserve bounds information}}
+                          // expected-note at -1{{change type of 'a' to 'std::array' to harden it}}
   int b[10];              // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
-                          // expected-note at -1{{change type of 'b' to 'std::array' to preserve bounds information}}
+                          // expected-note at -1{{change type of 'b' to 'std::array' to harden it}}
   
   auto Lam1 = [=]() {
     return a[1];           // expected-note{{used in buffer access here}}
@@ -348,7 +348,7 @@ template<typename T> void fArr(T t[]) {
   // expected-warning at -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}}
-                // expected-note at -1{{change type of 'ar' to 'std::array' to preserve bounds information}}
+                // expected-note at -1{{change type of 'ar' to 'std::array' to harden it}}
   foo(ar[5]);   // expected-note{{used in buffer access here}}
 }
 

>From 5540d7b3159182eb0fbd8d9b2f5cfc4ddcbcb51e Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Thu, 1 Feb 2024 13:58:34 -0800
Subject: [PATCH 12/13] [-Wunsafe-buffer-usage] Enable fixits for const ptr
 array declarations

---
 clang/lib/Analysis/UnsafeBufferUsage.cpp         | 16 ----------------
 .../SemaCXX/warn-unsafe-buffer-usage-debug.cpp   |  7 -------
 ...nsafe-buffer-usage-fixits-local-var-array.cpp |  9 +++------
 3 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 30723b97f6268..fce0716781de4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -2478,22 +2478,6 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
     const QualType &ArrayEltT = CAT->getElementType();
     assert(!ArrayEltT.isNull() && "Trying to fix a non-array type variable!");
 
-    // For most types the transformation is simple:
-    //   T foo[10]; => std::array<T, 10> foo;
-    // Cv-specifiers are straigtforward:
-    //   const T foo[10]; => std::array<const T, 10> foo;
-    // Pointers are straightforward:
-    //   T * foo[10]; => std::array<T *, 10> foo;
-    //
-    // However, for const pointers the transformation is different:
-    //   T * const foo[10]; => const std::array<T *, 10> foo;
-    if (ArrayEltT->isPointerType() && ArrayEltT.isConstQualified()) {
-      DEBUG_NOTE_DECL_FAIL(D, " : const size array of const pointers");
-      // FIXME: implement the support
-      // FIXME: bail if the const pointer is a typedef
-      return {};
-    }
-
     const SourceLocation IdentifierLoc = getVarDeclIdentifierLoc(D);
 
     // Get the spelling of the element type as written in the source file
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
index 518fafcda9d37..3a360566d1f3a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
@@ -32,13 +32,6 @@ void foo() {
                         // debug-note{{safe buffers debug: gadget 'ULCArraySubscript' refused to produce a fix}}
 }
 
-void failed_decl(const int* out, unsigned idx) {
-  const int* const a[10] = {nullptr};  // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}} \
-              // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : const size array of const pointers}}
-
-  out = a[idx];  // expected-note{{used in buffer access here}}
-}
-
 void failed_multiple_decl() {
   int *a = new int[4], b;  // expected-warning{{'a' is an unsafe pointer used for buffer access}} \
                           // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : multiple VarDecls}}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
index 7e83d8e68a9ee..496f618c496aa 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -73,8 +73,7 @@ void local_array_ptr_to_const(unsigned idx, const int*& a) {
 
 void local_array_const_ptr(unsigned idx, int*& a) {
   int * const buffer[10] = {a};
-// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
-// FIXME: implement support
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:25}:"std::array<int * const, 10> buffer"
 
   a = buffer[idx];
 }
@@ -82,16 +81,14 @@ void local_array_const_ptr(unsigned idx, int*& a) {
 void local_array_const_ptr_via_typedef(unsigned idx, int*& a) {
   typedef int * const my_const_ptr;
   my_const_ptr buffer[10] = {a};
-// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
-// FIXME: implement support
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:26}:"std::array<my_const_ptr, 10> buffer"
 
   a = buffer[idx];
 }
 
 void local_array_const_ptr_to_const(unsigned idx, const int*& a) {
   const int * const buffer[10] = {a};
-// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
-// FIXME: implement support
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:31}:"std::array<const int * const, 10> buffer"
 
   a = buffer[idx];
 

>From 7694a6d8f2b14b87447765c535f7877d63593ffa Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Thu, 1 Feb 2024 14:02:35 -0800
Subject: [PATCH 13/13] [-Wunsafe-buffer-usage] Remove unused TargetType enum

---
 clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 2c9ebf3fb42d0..5d16dcc824c50 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -95,8 +95,6 @@ class UnsafeBufferUsageHandler {
 #endif
 
 public:
-  enum class TargetType { Span, Array };
-
   UnsafeBufferUsageHandler() = default;
   virtual ~UnsafeBufferUsageHandler() = default;