Re-land "[-Wunsafe-buffer-usage][NFC] Slightly refactor and optimize the code"

This reverts commit ac9a76d7487b9af1ace626eb90064194cb12c53d.

Previously an abstract class has no pure virtual function.  It causes build error on some bots.

1 files changed, 42 insertions, 44 deletions

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 0776299..addaca4 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2175,6 +2175,41 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
   Sema &S;
   bool SuggestSuggestions;  // Recommend -fsafe-buffer-usage-suggestions?
 
+  // Lists as a string the names of variables in `VarGroupForVD` except for `VD`
+  // itself:
+  std::string listVariableGroupAsString(
+      const VarDecl *VD, const ArrayRef<const VarDecl *> &VarGroupForVD) const {
+    if (VarGroupForVD.size() <= 1)
+      return "";
+
+    std::vector<StringRef> VarNames;
+    auto PutInQuotes = [](StringRef S) -> std::string {
+      return "'" + S.str() + "'";
+    };
+
+    for (auto *V : VarGroupForVD) {
+      if (V == VD)
+        continue;
+      VarNames.push_back(V->getName());
+    }
+    if (VarNames.size() == 1) {
+      return PutInQuotes(VarNames[0]);
+    }
+    if (VarNames.size() == 2) {
+      return PutInQuotes(VarNames[0]) + " and " + PutInQuotes(VarNames[1]);
+    }
+    assert(VarGroupForVD.size() > 3);
+    const unsigned N = VarNames.size() -
+                       2; // need to print the last two names as "..., X, and Y"
+    std::string AllVars = "";
+
+    for (unsigned I = 0; I < N; ++I)
+      AllVars.append(PutInQuotes(VarNames[I]) + ", ");
+    AllVars.append(PutInQuotes(VarNames[N]) + ", and " +
+                   PutInQuotes(VarNames[N + 1]));
+    return AllVars;
+  }
+
 public:
   UnsafeBufferUsageReporter(Sema &S, bool SuggestSuggestions)
     : S(S), SuggestSuggestions(SuggestSuggestions) {}
@@ -2231,62 +2266,25 @@ public:
   }
 
   void handleUnsafeVariableGroup(const VarDecl *Variable,
-                                 const DefMapTy &VarGrpMap,
-                             FixItList &&Fixes) override {
+                                 const VariableGroupsManager &VarGrpMgr,
+                                 FixItList &&Fixes) override {
     assert(!SuggestSuggestions &&
            "Unsafe buffer usage fixits displayed without suggestions!");
     S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable)
         << Variable << (Variable->getType()->isPointerType() ? 0 : 1)
         << Variable->getSourceRange();
     if (!Fixes.empty()) {
-      const auto VarGroupForVD = VarGrpMap.find(Variable)->second;
+      const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Variable);
       unsigned FixItStrategy = 0; // For now we only have 'std::span' strategy
       const auto &FD = S.Diag(Variable->getLocation(),
                               diag::note_unsafe_buffer_variable_fixit_group);
 
       FD << Variable << FixItStrategy;
-      std::string AllVars = "";
-      if (VarGroupForVD.size() > 1) {
-        if (VarGroupForVD.size() == 2) {
-          if (VarGroupForVD[0] == Variable) {
-            AllVars.append("'" + VarGroupForVD[1]->getName().str() + "'");
-          } else {
-            AllVars.append("'" + VarGroupForVD[0]->getName().str() + "'");
-          }
-        } else {
-          bool first = false;
-          if (VarGroupForVD.size() == 3) {
-            for (const VarDecl * V : VarGroupForVD) {
-              if (V == Variable) {
-                continue;
-              }
-              if (!first) {
-                first = true;
-                AllVars.append("'" + V->getName().str() + "'" + " and ");
-              } else {
-                AllVars.append("'" + V->getName().str() + "'");
-              }
-            }
-          } else {
-            for (const VarDecl * V : VarGroupForVD) {
-              if (V == Variable) {
-                continue;
-              }
-              if (VarGroupForVD.back() != V) {
-                AllVars.append("'" + V->getName().str() + "'" + ", ");
-              } else {
-                AllVars.append("and '" + V->getName().str() + "'");
-              }
-            }
-          }
-        }
-        FD << AllVars << 1;
-      } else {
-        FD << "" << 0;
-      }
-
-      for (const auto &F : Fixes)
+      FD << listVariableGroupAsString(Variable, VarGroupForVD)
+         << (VarGroupForVD.size() > 1);
+      for (const auto &F : Fixes) {
         FD << F;
+      }
     }
 
 #ifndef NDEBUG