[clang] [clang][dataflow] Add a lattice to help cache const accessor methods (PR #111006)
Jan Voung via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 15 18:58:58 PDT 2024
https://github.com/jvoung updated https://github.com/llvm/llvm-project/pull/111006
>From f82e63e470f704f29f4c161579fd592c27301868 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Thu, 3 Oct 2024 15:21:32 +0000
Subject: [PATCH 1/4] [clang][dataflow] Add a lattice to help represent cache
const accessor methods
By caching const accessor methods we can sometimes treat method call
results as stable (e.g., for issue
https://github.com/llvm/llvm-project/issues/58510).
Users can clear the cache when a non-const method is called that may
modify the state of an object.
This is represented as mixin.
It will be used in a follow on patch to change
bugprone-unchecked-optional-access's lattice from
NoopLattice to CachedConstAccessorsLattice<NoopLattice>,
along with some additional transfer functions.
---
.../CachedConstAccessorsLattice.h | 221 ++++++++++++++++++
.../Analysis/FlowSensitive/CMakeLists.txt | 1 +
.../CachedConstAccessorsLatticeTest.cpp | 217 +++++++++++++++++
3 files changed, 439 insertions(+)
create mode 100644 clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
create mode 100644 clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
diff --git a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
new file mode 100644
index 00000000000000..b8517fabd1f550
--- /dev/null
+++ b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
@@ -0,0 +1,221 @@
+//===-- CachedConstAccessorsLattice.h ---------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the lattice mixin that additionally maintains a cache of
+// stable method call return values to model const accessor member functions.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
+
+#include "clang/AST/Expr.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
+
+namespace clang {
+namespace dataflow {
+
+/// A mixin for a lattice that additionally maintains a cache of stable method
+/// call return values to model const accessors methods. When a non-const method
+/// is called, the cache should be cleared causing the next call to a const
+/// method to be considered a different value. NOTE: The user is responsible for
+/// clearing the cache.
+///
+/// For example:
+///
+/// class Bar {
+/// public:
+/// const std::optional<Foo>& getFoo() const;
+/// void clear();
+/// };
+//
+/// void func(Bar& s) {
+/// if (s.getFoo().has_value()) {
+/// use(s.getFoo().value()); // safe (checked earlier getFoo())
+/// s.clear();
+/// use(s.getFoo().value()); // unsafe (invalidate cache for s)
+/// }
+/// }
+template <typename Base>
+class CachedConstAccessorsLattice : public Base {
+public:
+ using Base::Base; // inherit all constructors
+
+ /// Creates or returns a previously created `Value` associated with a const
+ /// method call `obj.getFoo()` where `RecordLoc` is the
+ /// `RecordStorageLocation` of `obj`.
+ /// Returns nullptr if unable to find or create a value.
+ ///
+ /// Requirements:
+ ///
+ /// - `CE` should return a value (not a reference or record type)
+ Value *
+ getOrCreateConstMethodReturnValue(const RecordStorageLocation &RecordLoc,
+ const CallExpr *CE, Environment &Env);
+
+ /// Creates or returns a previously created `StorageLocation` associated 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).
+ StorageLocation *getOrCreateConstMethodReturnStorageLocation(
+ const RecordStorageLocation &RecordLoc, const CallExpr *CE,
+ Environment &Env,
+ llvm::function_ref<void(StorageLocation &)> Initialize);
+
+ void clearConstMethodReturnValues(const RecordStorageLocation &RecordLoc) {
+ ConstMethodReturnValues.erase(&RecordLoc);
+ }
+
+ void clearConstMethodReturnStorageLocations(
+ const RecordStorageLocation &RecordLoc) {
+ ConstMethodReturnStorageLocations.erase(&RecordLoc);
+ }
+
+ bool operator==(const CachedConstAccessorsLattice &Other) const {
+ return Base::operator==(Other);
+ }
+
+ LatticeJoinEffect join(const CachedConstAccessorsLattice &Other);
+
+private:
+ // Maps a record storage location and const method to the value to return
+ // from that const method.
+ using ConstMethodReturnValuesType = llvm::SmallDenseMap<
+ const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, Value *>>;
+ ConstMethodReturnValuesType ConstMethodReturnValues;
+
+ // Maps a record storage location and const method to the record storage
+ // location to return from that const method.
+ using ConstMethodReturnStorageLocationsType = llvm::SmallDenseMap<
+ const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, StorageLocation *>>;
+ ConstMethodReturnStorageLocationsType ConstMethodReturnStorageLocations;
+};
+
+namespace internal {
+
+template <typename T>
+llvm::SmallDenseMap<const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, T *>>
+joinConstMethodMap(
+ const llvm::SmallDenseMap<const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, T *>>
+ &Map1,
+ const llvm::SmallDenseMap<const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, T *>>
+ &Map2,
+ LatticeEffect &Effect) {
+ llvm::SmallDenseMap<const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, T *>>
+ Result;
+ for (auto &[Loc, DeclToT] : Map1) {
+ auto It = Map2.find(Loc);
+ if (It == Map2.end()) {
+ Effect = LatticeJoinEffect::Changed;
+ continue;
+ }
+ const auto &OtherDeclToT = It->second;
+ auto &JoinedDeclToT = Result[Loc];
+ for (auto [Func, Var] : DeclToT) {
+ T *OtherVar = OtherDeclToT.lookup(Func);
+ if (OtherVar == nullptr || OtherVar != Var) {
+ Effect = LatticeJoinEffect::Changed;
+ continue;
+ }
+ JoinedDeclToT.insert({Func, Var});
+ }
+ }
+ return Result;
+}
+
+} // namespace internal
+
+template <typename Base>
+LatticeEffect CachedConstAccessorsLattice<Base>::join(
+ const CachedConstAccessorsLattice<Base> &Other) {
+
+ LatticeEffect Effect = Base::join(Other);
+
+ // For simplicity, we only retain values that are identical, but not ones that
+ // are non-identical but equivalent. This is likely to be sufficient in
+ // practice, and it reduces implementation complexity considerably.
+
+ ConstMethodReturnValues = internal::joinConstMethodMap<Value>(
+ ConstMethodReturnValues, Other.ConstMethodReturnValues, Effect);
+
+ ConstMethodReturnStorageLocations =
+ internal::joinConstMethodMap<StorageLocation>(
+ ConstMethodReturnStorageLocations,
+ Other.ConstMethodReturnStorageLocations, Effect);
+
+ return Effect;
+}
+
+template <typename Base>
+Value *CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnValue(
+ const RecordStorageLocation &RecordLoc, const CallExpr *CE,
+ Environment &Env) {
+ QualType Type = CE->getType();
+ assert(!Type.isNull());
+ assert(!Type->isReferenceType());
+ assert(!Type->isRecordType());
+
+ auto &ObjMap = ConstMethodReturnValues[&RecordLoc];
+ const FunctionDecl *DirectCallee = CE->getDirectCallee();
+ if (DirectCallee == nullptr)
+ return nullptr;
+ auto it = ObjMap.find(DirectCallee);
+ if (it != ObjMap.end())
+ return it->second;
+
+ Value *Val = Env.createValue(Type);
+ if (Val != nullptr)
+ ObjMap.insert({DirectCallee, Val});
+ return Val;
+}
+
+template <typename Base>
+StorageLocation *
+CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
+ const RecordStorageLocation &RecordLoc, const CallExpr *CE,
+ Environment &Env,
+ llvm::function_ref<void(StorageLocation &)> Initialize) {
+ QualType Type = CE->getType();
+ assert(!Type.isNull());
+ assert(CE->isGLValue() || Type->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;
+}
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
diff --git a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
index 12fee5dc2789ce..4e1819bfa166a8 100644
--- a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests
ArenaTest.cpp
ASTOpsTest.cpp
CFGMatchSwitchTest.cpp
+ CachedConstAccessorsLatticeTest.cpp
ChromiumCheckModelTest.cpp
DataflowAnalysisContextTest.cpp
DataflowEnvironmentTest.cpp
diff --git a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
new file mode 100644
index 00000000000000..eb6f081fd22c70
--- /dev/null
+++ b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
@@ -0,0 +1,217 @@
+//===- unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp ==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h"
+
+#include <cassert>
+#include <memory>
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Testing/TestAST.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::dataflow {
+namespace {
+
+using ast_matchers::callee;
+using ast_matchers::cxxMemberCallExpr;
+using ast_matchers::functionDecl;
+using ast_matchers::hasName;
+using ast_matchers::match;
+using ast_matchers::selectFirst;
+
+using dataflow::DataflowAnalysisContext;
+using dataflow::Environment;
+using dataflow::LatticeJoinEffect;
+using dataflow::RecordStorageLocation;
+using dataflow::Value;
+using dataflow::WatchedLiteralsSolver;
+
+NamedDecl *lookup(StringRef Name, const DeclContext &DC) {
+ auto Result = DC.lookup(&DC.getParentASTContext().Idents.get(Name));
+ EXPECT_TRUE(Result.isSingleResult()) << Name;
+ return Result.front();
+}
+
+class CachedConstAccessorsLatticeTest : public ::testing::Test {
+protected:
+ using LatticeT = CachedConstAccessorsLattice<NoopLattice>;
+
+ DataflowAnalysisContext DACtx{std::make_unique<WatchedLiteralsSolver>()};
+ Environment Env{DACtx};
+};
+
+// Basic test AST with two const methods (return a value, and return a ref).
+struct CommonTestInputs {
+ CommonTestInputs()
+ : AST(R"cpp(
+ struct S {
+ int *valProperty() const;
+ int &refProperty() const;
+ };
+ void target() {
+ S s;
+ s.valProperty();
+ S s2;
+ s2.refProperty();
+ }
+ )cpp") {
+ auto *SDecl = cast<CXXRecordDecl>(
+ lookup("S", *AST.context().getTranslationUnitDecl()));
+ SType = AST.context().getRecordType(SDecl);
+ CallVal = selectFirst<CallExpr>(
+ "call",
+ match(cxxMemberCallExpr(callee(functionDecl(hasName("valProperty"))))
+ .bind("call"),
+ AST.context()));
+ assert(CallVal != nullptr);
+
+ CallRef = selectFirst<CallExpr>(
+ "call",
+ match(cxxMemberCallExpr(callee(functionDecl(hasName("refProperty"))))
+ .bind("call"),
+ AST.context()));
+ assert(CallRef != nullptr);
+}
+
+ TestAST AST;
+ QualType SType;
+ const CallExpr *CallVal;
+ const CallExpr *CallRef;
+};
+
+TEST_F(CachedConstAccessorsLatticeTest, SameValBeforeClearOrDiffAfterClear) {
+ CommonTestInputs Inputs;
+ auto *CE = Inputs.CallVal;
+ RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
+ {});
+
+ LatticeT Lattice;
+ Value *Val1 = Lattice.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+ Value *Val2 = Lattice.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+
+ EXPECT_EQ(Val1, Val2);
+
+ Lattice.clearConstMethodReturnValues(Loc);
+ Value *Val3 = Lattice.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+
+ EXPECT_NE(Val3, Val1);
+ EXPECT_NE(Val3, Val2);
+}
+
+TEST_F(CachedConstAccessorsLatticeTest, SameLocBeforeClearOrDiffAfterClear) {
+ CommonTestInputs Inputs;
+ auto *CE = Inputs.CallRef;
+ 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, ClearDifferentLocs) {
+ CommonTestInputs Inputs;
+ auto *CE = Inputs.CallRef;
+ RecordStorageLocation LocS1(Inputs.SType, RecordStorageLocation::FieldToLoc(),
+ {});
+ RecordStorageLocation LocS2(Inputs.SType, RecordStorageLocation::FieldToLoc(),
+ {});
+
+ LatticeT Lattice;
+ auto NopInit = [](StorageLocation &) {};
+ StorageLocation *RetLoc1 =
+ Lattice.getOrCreateConstMethodReturnStorageLocation(LocS1, CE, Env,
+ NopInit);
+ Lattice.clearConstMethodReturnStorageLocations(LocS2);
+ auto NotCalled = [](StorageLocation &) {
+ ASSERT_TRUE(false) << "Not reached";
+ };
+ StorageLocation *RetLoc2 =
+ Lattice.getOrCreateConstMethodReturnStorageLocation(LocS1, CE, Env,
+ NotCalled);
+
+ EXPECT_EQ(RetLoc1, RetLoc2);
+}
+
+TEST_F(CachedConstAccessorsLatticeTest, JoinSameNoop) {
+ CommonTestInputs Inputs;
+ auto *CE = Inputs.CallVal;
+ RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
+ {});
+
+ LatticeT EmptyLattice;
+ LatticeT EmptyLattice2;
+ EXPECT_EQ(EmptyLattice.join(EmptyLattice2), LatticeJoinEffect::Unchanged);
+
+ LatticeT Lattice1;
+ Lattice1.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+ EXPECT_EQ(Lattice1.join(Lattice1), LatticeJoinEffect::Unchanged);
+}
+
+TEST_F(CachedConstAccessorsLatticeTest, ProducesNewValueAfterJoinDistinct) {
+ CommonTestInputs Inputs;
+ auto *CE = Inputs.CallVal;
+ RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
+ {});
+
+ // L1 w/ v vs L2 empty
+ LatticeT Lattice1;
+ Value *Val1 = Lattice1.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+
+ LatticeT EmptyLattice;
+
+ EXPECT_EQ(Lattice1.join(EmptyLattice), LatticeJoinEffect::Changed);
+ Value *ValAfterJoin =
+ Lattice1.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+
+ EXPECT_NE(ValAfterJoin, Val1);
+
+ // L1 w/ v1 vs L3 w/ v2
+ LatticeT Lattice3;
+ Value *Val3 = Lattice3.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+
+ EXPECT_EQ(Lattice1.join(Lattice3), LatticeJoinEffect::Changed);
+ Value *ValAfterJoin2 =
+ Lattice1.getOrCreateConstMethodReturnValue(Loc, CE, Env);
+
+ EXPECT_NE(ValAfterJoin2, ValAfterJoin);
+ EXPECT_NE(ValAfterJoin2, Val3);
+}
+
+} // namespace
+} // namespace clang::dataflow
>From 4885d657a6ec13f304d851dc18f8b26089738a40 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Thu, 3 Oct 2024 15:32:37 +0000
Subject: [PATCH 2/4] clang-format
---
.../CachedConstAccessorsLattice.h | 19 ++++++++-----------
.../CachedConstAccessorsLatticeTest.cpp | 2 +-
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
index b8517fabd1f550..52114173e21bba 100644
--- a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
+++ b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
@@ -45,10 +45,9 @@ namespace dataflow {
/// use(s.getFoo().value()); // unsafe (invalidate cache for s)
/// }
/// }
-template <typename Base>
-class CachedConstAccessorsLattice : public Base {
+template <typename Base> class CachedConstAccessorsLattice : public Base {
public:
- using Base::Base; // inherit all constructors
+ using Base::Base; // inherit all constructors
/// Creates or returns a previously created `Value` associated with a const
/// method call `obj.getFoo()` where `RecordLoc` is the
@@ -74,8 +73,7 @@ class CachedConstAccessorsLattice : public Base {
/// - `CE` should return a location (GLValue or a record type).
StorageLocation *getOrCreateConstMethodReturnStorageLocation(
const RecordStorageLocation &RecordLoc, const CallExpr *CE,
- Environment &Env,
- llvm::function_ref<void(StorageLocation &)> Initialize);
+ Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize);
void clearConstMethodReturnValues(const RecordStorageLocation &RecordLoc) {
ConstMethodReturnValues.erase(&RecordLoc);
@@ -95,9 +93,9 @@ class CachedConstAccessorsLattice : public Base {
private:
// Maps a record storage location and const method to the value to return
// from that const method.
- using ConstMethodReturnValuesType = llvm::SmallDenseMap<
- const RecordStorageLocation *,
- llvm::SmallDenseMap<const FunctionDecl *, Value *>>;
+ using ConstMethodReturnValuesType =
+ llvm::SmallDenseMap<const RecordStorageLocation *,
+ llvm::SmallDenseMap<const FunctionDecl *, Value *>>;
ConstMethodReturnValuesType ConstMethodReturnValues;
// Maps a record storage location and const method to the record storage
@@ -194,8 +192,7 @@ template <typename Base>
StorageLocation *
CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
const RecordStorageLocation &RecordLoc, const CallExpr *CE,
- Environment &Env,
- llvm::function_ref<void(StorageLocation &)> Initialize) {
+ Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize) {
QualType Type = CE->getType();
assert(!Type.isNull());
assert(CE->isGLValue() || Type->isRecordType());
@@ -218,4 +215,4 @@ CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
} // namespace dataflow
} // namespace clang
-#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
+#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
diff --git a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
index eb6f081fd22c70..de0c9278fae457 100644
--- a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
@@ -91,7 +91,7 @@ struct CommonTestInputs {
.bind("call"),
AST.context()));
assert(CallRef != nullptr);
-}
+ }
TestAST AST;
QualType SType;
>From 10e69fd93ffa31f5b6b0e937092f8aab6f9cecc2 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Wed, 16 Oct 2024 01:52:19 +0000
Subject: [PATCH 3/4] Fix comments and add two more tests.
- a test for an accessor returning a record type
- a test for calling accessors of different objects
---
.../CachedConstAccessorsLattice.h | 6 +-
.../CachedConstAccessorsLatticeTest.cpp | 94 ++++++++++++++++++-
2 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
index 52114173e21bba..3c3028eb9452fe 100644
--- a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
+++ b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h
@@ -50,7 +50,7 @@ template <typename Base> class CachedConstAccessorsLattice : public Base {
using Base::Base; // inherit all constructors
/// Creates or returns a previously created `Value` associated with a const
- /// method call `obj.getFoo()` where `RecordLoc` is the
+ /// method call `obj.getFoo()` where `RecordLoc` is the
/// `RecordStorageLocation` of `obj`.
/// Returns nullptr if unable to find or create a value.
///
@@ -61,8 +61,8 @@ template <typename Base> class CachedConstAccessorsLattice : public Base {
getOrCreateConstMethodReturnValue(const RecordStorageLocation &RecordLoc,
const CallExpr *CE, Environment &Env);
- /// Creates or returns a previously created `StorageLocation` associated a
- /// const method call `obj.getFoo()` where `RecordLoc` is the
+ /// 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.
diff --git a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
index de0c9278fae457..d88fc684a0504a 100644
--- a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
@@ -32,6 +32,7 @@
namespace clang::dataflow {
namespace {
+using ast_matchers::BoundNodes;
using ast_matchers::callee;
using ast_matchers::cxxMemberCallExpr;
using ast_matchers::functionDecl;
@@ -46,6 +47,8 @@ using dataflow::RecordStorageLocation;
using dataflow::Value;
using dataflow::WatchedLiteralsSolver;
+using testing::SizeIs;
+
NamedDecl *lookup(StringRef Name, const DeclContext &DC) {
auto Result = DC.lookup(&DC.getParentASTContext().Idents.get(Name));
EXPECT_TRUE(Result.isSingleResult()) << Name;
@@ -99,7 +102,8 @@ struct CommonTestInputs {
const CallExpr *CallRef;
};
-TEST_F(CachedConstAccessorsLatticeTest, SameValBeforeClearOrDiffAfterClear) {
+TEST_F(CachedConstAccessorsLatticeTest,
+ SamePrimitiveValBeforeClearOrDiffAfterClear) {
CommonTestInputs Inputs;
auto *CE = Inputs.CallVal;
RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
@@ -144,6 +148,51 @@ TEST_F(CachedConstAccessorsLatticeTest, SameLocBeforeClearOrDiffAfterClear) {
EXPECT_NE(Loc3, Loc2);
}
+TEST_F(CachedConstAccessorsLatticeTest,
+ SameStructValBeforeClearOrDiffAfterClear) {
+ TestAST AST(R"cpp(
+ struct S {
+ S structValProperty() const;
+ };
+ void target() {
+ S s;
+ s.structValProperty();
+ }
+ )cpp");
+ auto *SDecl =
+ cast<CXXRecordDecl>(lookup("S", *AST.context().getTranslationUnitDecl()));
+ QualType SType = AST.context().getRecordType(SDecl);
+ const CallExpr *CE = selectFirst<CallExpr>(
+ "call", match(cxxMemberCallExpr(
+ callee(functionDecl(hasName("structValProperty"))))
+ .bind("call"),
+ AST.context()));
+ ASSERT_NE(CE, nullptr);
+
+ RecordStorageLocation Loc(SType, RecordStorageLocation::FieldToLoc(), {});
+
+ LatticeT Lattice;
+ // 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);
+ 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, Loc1);
+}
+
TEST_F(CachedConstAccessorsLatticeTest, ClearDifferentLocs) {
CommonTestInputs Inputs;
auto *CE = Inputs.CallRef;
@@ -168,6 +217,49 @@ TEST_F(CachedConstAccessorsLatticeTest, ClearDifferentLocs) {
EXPECT_EQ(RetLoc1, RetLoc2);
}
+TEST_F(CachedConstAccessorsLatticeTest, DifferentValsFromDifferentLocs) {
+ TestAST AST(R"cpp(
+ struct S {
+ int *valProperty() const;
+ };
+ void target() {
+ S s1;
+ s1.valProperty();
+ S s2;
+ s2.valProperty();
+ }
+ )cpp");
+ auto *SDecl =
+ cast<CXXRecordDecl>(lookup("S", *AST.context().getTranslationUnitDecl()));
+ QualType SType = AST.context().getRecordType(SDecl);
+ SmallVector<BoundNodes, 1> valPropertyCalls =
+ match(cxxMemberCallExpr(callee(functionDecl(hasName("valProperty"))))
+ .bind("call"),
+ AST.context());
+ ASSERT_THAT(valPropertyCalls, SizeIs(2));
+
+ const CallExpr *CE1 = selectFirst<CallExpr>(
+ "call", valPropertyCalls);
+ ASSERT_NE(CE1, nullptr);
+
+ valPropertyCalls.erase(valPropertyCalls.begin());
+ const CallExpr *CE2 = selectFirst<CallExpr>(
+ "call", valPropertyCalls);
+ ASSERT_NE(CE2, nullptr);
+ ASSERT_NE(CE1, CE2);
+
+ RecordStorageLocation LocS1(SType, RecordStorageLocation::FieldToLoc(), {});
+ RecordStorageLocation LocS2(SType, RecordStorageLocation::FieldToLoc(), {});
+
+ LatticeT Lattice;
+ Value *Val1 =
+ Lattice.getOrCreateConstMethodReturnValue(LocS1, CE1, Env);
+ Value *Val2 =
+ Lattice.getOrCreateConstMethodReturnValue(LocS2, CE2, Env);
+
+ EXPECT_NE(Val1, Val2);
+}
+
TEST_F(CachedConstAccessorsLatticeTest, JoinSameNoop) {
CommonTestInputs Inputs;
auto *CE = Inputs.CallVal;
>From 5578e3192774fdfc30aa5781ea5ab4aab28b2d12 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Wed, 16 Oct 2024 01:58:26 +0000
Subject: [PATCH 4/4] Fix formatting
---
.../CachedConstAccessorsLatticeTest.cpp | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
index d88fc684a0504a..6488833bd14cf2 100644
--- a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp
@@ -238,13 +238,11 @@ TEST_F(CachedConstAccessorsLatticeTest, DifferentValsFromDifferentLocs) {
AST.context());
ASSERT_THAT(valPropertyCalls, SizeIs(2));
- const CallExpr *CE1 = selectFirst<CallExpr>(
- "call", valPropertyCalls);
+ const CallExpr *CE1 = selectFirst<CallExpr>("call", valPropertyCalls);
ASSERT_NE(CE1, nullptr);
valPropertyCalls.erase(valPropertyCalls.begin());
- const CallExpr *CE2 = selectFirst<CallExpr>(
- "call", valPropertyCalls);
+ const CallExpr *CE2 = selectFirst<CallExpr>("call", valPropertyCalls);
ASSERT_NE(CE2, nullptr);
ASSERT_NE(CE1, CE2);
@@ -252,10 +250,8 @@ TEST_F(CachedConstAccessorsLatticeTest, DifferentValsFromDifferentLocs) {
RecordStorageLocation LocS2(SType, RecordStorageLocation::FieldToLoc(), {});
LatticeT Lattice;
- Value *Val1 =
- Lattice.getOrCreateConstMethodReturnValue(LocS1, CE1, Env);
- Value *Val2 =
- Lattice.getOrCreateConstMethodReturnValue(LocS2, CE2, Env);
+ Value *Val1 = Lattice.getOrCreateConstMethodReturnValue(LocS1, CE1, Env);
+ Value *Val2 = Lattice.getOrCreateConstMethodReturnValue(LocS2, CE2, Env);
EXPECT_NE(Val1, Val2);
}
More information about the cfe-commits
mailing list