llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-analysis Author: Jan Voung (jvoung) <details> <summary>Changes</summary> 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 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) { `````````` </details> https://github.com/llvm/llvm-project/pull/127001 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits