[clang][dataflow] Remove a deprecated CachedConstAccessorsLattice API

[jvoung]

We've already migrated known users from the old to the new version of getOrCreateConstMethodReturnStorageLocation. The conversion is pretty straightforward as well, if there are out-of-tree users:

Previously: use CallExpr as argument
New: get the direct Callee from CallExpr, null check, and use that as the argument instead.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: Jan Voung (jvoung)

Changes

We've already migrated known users from the old to the new version of
getOrCreateConstMethodReturnStorageLocation. The conversion is pretty
straightforward as well, if there are out-of-tree users:
Previously: use CallExpr as argument
New: get the direct Callee from CallExpr, null check,
and use that as the argument instead.

---

Full diff: https://github.com/llvm/llvm-project/pull/127001.diff

2 Files Affected:

• (modified) clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h (-40)
• (modified) clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp (+18-41)

diff --git a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
index aaf89f4e94d4a..78b03d325efd9 100644
--- a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
+++ b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
@@ -63,23 +63,6 @@ template <typename Base> class CachedConstAccessorsLattice : public Base {
   getOrCreateConstMethodReturnValue(const RecordStorageLocation &RecordLoc,
                                     const CallExpr *CE, Environment &Env);
 
-  /// Creates or returns a previously created `StorageLocation` associated with
-  /// a const method call `obj.getFoo()` where `RecordLoc` is the
-  /// `RecordStorageLocation` of `obj`.
-  ///
-  /// The callback `Initialize` runs on the storage location if newly created.
-  /// Returns nullptr if unable to find or create a value.
-  ///
-  /// Requirements:
-  ///
-  ///  - `CE` should return a location (GLValue or a record type).
-  ///
-  /// DEPRECATED: switch users to the below overload which takes Callee and Type
-  /// directly.
-  StorageLocation *getOrCreateConstMethodReturnStorageLocation(
-      const RecordStorageLocation &RecordLoc, const CallExpr *CE,
-      Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize);
-
   /// Creates or returns a previously created `StorageLocation` associated with
   /// a const method call `obj.getFoo()` where `RecordLoc` is the
   /// `RecordStorageLocation` of `obj`, `Callee` is the decl for `getFoo`.
@@ -208,29 +191,6 @@ Value *CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnValue(
   return Val;
 }
 
-template <typename Base>
-StorageLocation *
-CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
-    const RecordStorageLocation &RecordLoc, const CallExpr *CE,
-    Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize) {
-  assert(!CE->getType().isNull());
-  assert(CE->isGLValue() || CE->getType()->isRecordType());
-  auto &ObjMap = ConstMethodReturnStorageLocations[&RecordLoc];
-  const FunctionDecl *DirectCallee = CE->getDirectCallee();
-  if (DirectCallee == nullptr)
-    return nullptr;
-  auto it = ObjMap.find(DirectCallee);
-  if (it != ObjMap.end())
-    return it->second;
-
-  StorageLocation &Loc =
-      Env.createStorageLocation(CE->getType().getNonReferenceType());
-  Initialize(Loc);
-
-  ObjMap.insert({DirectCallee, &Loc});
-  return &Loc;
-}
-
 template <typename Base>
 StorageLocation &
 CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
diff --git a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
index d27f6a6d27e71..ffc50fbb65523 100644
--- a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
@@ -128,33 +128,6 @@ TEST_F(CachedConstAccessorsLatticeTest, SameLocBeforeClearOrDiffAfterClear) {
   RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
                             {});
 
-  LatticeT Lattice;
-  auto NopInit = [](StorageLocation &) {};
-  StorageLocation *Loc1 = Lattice.getOrCreateConstMethodReturnStorageLocation(
-      Loc, CE, Env, NopInit);
-  auto NotCalled = [](StorageLocation &) {
-    ASSERT_TRUE(false) << "Not reached";
-  };
-  StorageLocation *Loc2 = Lattice.getOrCreateConstMethodReturnStorageLocation(
-      Loc, CE, Env, NotCalled);
-
-  EXPECT_EQ(Loc1, Loc2);
-
-  Lattice.clearConstMethodReturnStorageLocations(Loc);
-  StorageLocation *Loc3 = Lattice.getOrCreateConstMethodReturnStorageLocation(
-      Loc, CE, Env, NopInit);
-
-  EXPECT_NE(Loc3, Loc1);
-  EXPECT_NE(Loc3, Loc2);
-}
-
-TEST_F(CachedConstAccessorsLatticeTest,
-       SameLocBeforeClearOrDiffAfterClearWithCallee) {
-  CommonTestInputs Inputs;
-  auto *CE = Inputs.CallRef;
-  RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
-                            {});
-
   LatticeT Lattice;
   auto NopInit = [](StorageLocation &) {};
   const FunctionDecl *Callee = CE->getDirectCallee();
@@ -204,22 +177,24 @@ TEST_F(CachedConstAccessorsLatticeTest,
   // Accessors that return a record by value are modeled by a record storage
   // location (instead of a Value).
   auto NopInit = [](StorageLocation &) {};
-  StorageLocation *Loc1 = Lattice.getOrCreateConstMethodReturnStorageLocation(
-      Loc, CE, Env, NopInit);
+  const FunctionDecl *Callee = CE->getDirectCallee();
+  ASSERT_NE(Callee, nullptr);
+  StorageLocation &Loc1 = Lattice.getOrCreateConstMethodReturnStorageLocation(
+      Loc, Callee, Env, NopInit);
   auto NotCalled = [](StorageLocation &) {
     ASSERT_TRUE(false) << "Not reached";
   };
-  StorageLocation *Loc2 = Lattice.getOrCreateConstMethodReturnStorageLocation(
-      Loc, CE, Env, NotCalled);
+  StorageLocation &Loc2 = Lattice.getOrCreateConstMethodReturnStorageLocation(
+      Loc, Callee, Env, NotCalled);
 
-  EXPECT_EQ(Loc1, Loc2);
+  EXPECT_EQ(&Loc1, &Loc2);
 
   Lattice.clearConstMethodReturnStorageLocations(Loc);
-  StorageLocation *Loc3 = Lattice.getOrCreateConstMethodReturnStorageLocation(
-      Loc, CE, Env, NopInit);
+  StorageLocation &Loc3 = Lattice.getOrCreateConstMethodReturnStorageLocation(
+      Loc, Callee, Env, NopInit);
 
-  EXPECT_NE(Loc3, Loc1);
-  EXPECT_NE(Loc3, Loc1);
+  EXPECT_NE(&Loc3, &Loc1);
+  EXPECT_NE(&Loc3, &Loc1);
 }
 
 TEST_F(CachedConstAccessorsLatticeTest, ClearDifferentLocs) {
@@ -232,18 +207,20 @@ TEST_F(CachedConstAccessorsLatticeTest, ClearDifferentLocs) {
 
   LatticeT Lattice;
   auto NopInit = [](StorageLocation &) {};
-  StorageLocation *RetLoc1 =
-      Lattice.getOrCreateConstMethodReturnStorageLocation(LocS1, CE, Env,
+  const FunctionDecl *Callee = CE->getDirectCallee();
+  ASSERT_NE(Callee, nullptr);
+  StorageLocation &RetLoc1 =
+      Lattice.getOrCreateConstMethodReturnStorageLocation(LocS1, Callee, Env,
                                                           NopInit);
   Lattice.clearConstMethodReturnStorageLocations(LocS2);
   auto NotCalled = [](StorageLocation &) {
     ASSERT_TRUE(false) << "Not reached";
   };
-  StorageLocation *RetLoc2 =
-      Lattice.getOrCreateConstMethodReturnStorageLocation(LocS1, CE, Env,
+  StorageLocation &RetLoc2 =
+      Lattice.getOrCreateConstMethodReturnStorageLocation(LocS1, Callee, Env,
                                                           NotCalled);
 
-  EXPECT_EQ(RetLoc1, RetLoc2);
+  EXPECT_EQ(&RetLoc1, &RetLoc2);
 }
 
 TEST_F(CachedConstAccessorsLatticeTest, DifferentValsFromDifferentLocs) {