[clang] [clang][dataflow] Eliminate `RecordValue::getChild()`. (PR #65586)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 7 02:24:00 PDT 2023
https://github.com/martinboehme created https://github.com/llvm/llvm-project/pull/65586:
We want to eliminate the `RecordStorageLocation` from `RecordValue` and,
ultimately, eliminate `RecordValue` entirely (see the discussion linked in the
`RecordValue` class comment). This is one step in that direction.
To eliminate `RecordValue::getChild()`, we also eliminate the last remaining
caller, namely the `getFieldValue(const RecordValue *, ...)` overload. Calls
to this overload have been rewritten to use the
`getFieldValue(const RecordStorageLocation *, ...)` overload. Note that this
also makes the code slightly simpler in many cases.
>From cd8f6b6ffc5db2d2ec63ad19302cd933fda98912 Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboehme at google.com>
Date: Thu, 7 Sep 2023 09:22:37 +0000
Subject: [PATCH] [clang][dataflow] Eliminate `RecordValue::getChild()`.
We want to eliminate the `RecordStorageLocation` from `RecordValue` and,
ultimately, eliminate `RecordValue` entirely (see the discussion linked in the
`RecordValue` class comment). This is one step in that direction.
To eliminate `RecordValue::getChild()`, we also eliminate the last remaining
caller, namely the `getFieldValue(const RecordValue *, ...)` overload. Calls
to this overload have been rewritten to use the
`getFieldValue(const RecordStorageLocation *, ...)` overload. Note that this
also makes the code slightly simpler in many cases.
---
.../clang/Analysis/FlowSensitive/Value.h | 6 ---
.../FlowSensitive/DataflowEnvironmentTest.cpp | 11 +++--
.../Analysis/FlowSensitive/TestingSupport.h | 12 -----
.../Analysis/FlowSensitive/TransferTest.cpp | 44 +++++++++----------
4 files changed, 27 insertions(+), 46 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 6e911af7264ced..e6c68e5b4e93e1 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -225,12 +225,6 @@ class RecordValue final : public Value {
/// Returns the storage location that this `RecordValue` is associated with.
RecordStorageLocation &getLoc() const { return Loc; }
- /// Convenience function that returns the child storage location for `Field`.
- /// See also the documentation for `RecordStorageLocation::getChild()`.
- StorageLocation *getChild(const ValueDecl &Field) const {
- return Loc.getChild(Field);
- }
-
private:
RecordStorageLocation &Loc;
};
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index a318d9ab7391aa..846499a7b51f8c 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -91,8 +91,8 @@ TEST_F(EnvironmentTest, CreateValueRecursiveType) {
// Verify that the struct and the field (`R`) with first appearance of the
// type is created successfully.
Environment Env(DAContext, *Fun);
- RecordValue *SVal = cast<RecordValue>(Env.createValue(Ty));
- PointerValue *PV = cast_or_null<PointerValue>(getFieldValue(SVal, *R, Env));
+ auto &SLoc = cast<RecordStorageLocation>(Env.createObject(Ty));
+ PointerValue *PV = cast_or_null<PointerValue>(getFieldValue(&SLoc, *R, Env));
EXPECT_THAT(PV, NotNull());
}
@@ -171,8 +171,8 @@ TEST_F(EnvironmentTest, IncludeFieldsFromDefaultInitializers) {
// Verify that the `X` field of `S` is populated when analyzing the
// constructor, even though it is not referenced directly in the constructor.
Environment Env(DAContext, *Constructor);
- auto *Val = cast<RecordValue>(Env.createValue(QTy));
- EXPECT_THAT(getFieldValue(Val, *XDecl, Env), NotNull());
+ auto &Loc = cast<RecordStorageLocation>(Env.createObject(QTy));
+ EXPECT_THAT(getFieldValue(&Loc, *XDecl, Env), NotNull());
}
TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
@@ -216,8 +216,7 @@ TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
Environment Env(DAContext, *Fun);
const auto *GlobalLoc =
cast<RecordStorageLocation>(Env.getStorageLocation(*GlobalDecl));
- const auto *GlobalVal = cast<RecordValue>(Env.getValue(*GlobalLoc));
- auto *BarVal = getFieldValue(GlobalVal, *BarDecl, Env);
+ auto *BarVal = getFieldValue(GlobalLoc, *BarDecl, Env);
EXPECT_TRUE(isa<IntegerValue>(BarVal));
}
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index c61e9f26beff40..0cf66c218f43fd 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -463,18 +463,6 @@ inline Value *getFieldValue(const RecordStorageLocation *Loc,
return Env.getValue(*FieldLoc);
}
-/// Returns the value of a `Field` on a `Struct.
-/// Returns null if `Struct` is null.
-inline Value *getFieldValue(const RecordValue *Struct, const ValueDecl &Field,
- const Environment &Env) {
- if (Struct == nullptr)
- return nullptr;
- StorageLocation *FieldLoc = Struct->getChild(Field);
- if (FieldLoc == nullptr)
- return nullptr;
- return Env.getValue(*FieldLoc);
-}
-
/// Creates and owns constraints which are boolean values.
class ConstraintContext {
unsigned NextAtom = 0;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 28d062196cb07b..4361b769d94aaa 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -647,26 +647,26 @@ TEST(TransferTest, SelfReferentialPointerVarDecl) {
const auto &FooLoc =
*cast<ScalarStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto &FooVal = *cast<PointerValue>(Env.getValue(FooLoc));
- const auto &FooPointeeVal =
- *cast<RecordValue>(Env.getValue(FooVal.getPointeeLoc()));
+ const auto &FooPointeeLoc =
+ cast<RecordStorageLocation>(FooVal.getPointeeLoc());
const auto &BarVal =
- *cast<PointerValue>(getFieldValue(&FooPointeeVal, *BarDecl, Env));
- const auto &BarPointeeVal =
- *cast<RecordValue>(Env.getValue(BarVal.getPointeeLoc()));
+ *cast<PointerValue>(getFieldValue(&FooPointeeLoc, *BarDecl, Env));
+ const auto &BarPointeeLoc =
+ cast<RecordStorageLocation>(BarVal.getPointeeLoc());
- EXPECT_THAT(getFieldValue(&BarPointeeVal, *FooRefDecl, Env), NotNull());
+ EXPECT_THAT(getFieldValue(&BarPointeeLoc, *FooRefDecl, Env), NotNull());
const auto &FooPtrVal = *cast<PointerValue>(
- getFieldValue(&BarPointeeVal, *FooPtrDecl, Env));
+ getFieldValue(&BarPointeeLoc, *FooPtrDecl, Env));
const auto &FooPtrPointeeLoc =
cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
- EXPECT_THAT(getFieldValue(&BarPointeeVal, *BazRefDecl, Env), NotNull());
+ EXPECT_THAT(getFieldValue(&BarPointeeLoc, *BazRefDecl, Env), NotNull());
const auto &BazPtrVal = *cast<PointerValue>(
- getFieldValue(&BarPointeeVal, *BazPtrDecl, Env));
+ getFieldValue(&BarPointeeLoc, *BazPtrDecl, Env));
const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
});
@@ -2365,9 +2365,10 @@ TEST(TransferTest, BindTemporary) {
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
- const auto &FooVal = *cast<RecordValue>(Env.getValue(*FooDecl));
+ const auto &FooLoc =
+ *cast<RecordStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarVal = cast<IntegerValue>(Env.getValue(*BarDecl));
- EXPECT_EQ(BarVal, getFieldValue(&FooVal, *BazDecl, Env));
+ EXPECT_EQ(BarVal, getFieldValue(&FooLoc, *BazDecl, Env));
});
}
@@ -2857,16 +2858,14 @@ TEST(TransferTest, AggregateInitialization) {
const auto *BarArgVal = cast<IntegerValue>(Env.getValue(*BarArgDecl));
const auto *QuxArgVal = cast<IntegerValue>(Env.getValue(*QuxArgDecl));
- const auto *QuuxVal = cast<RecordValue>(Env.getValue(*QuuxDecl));
- ASSERT_THAT(QuuxVal, NotNull());
-
- const auto *BazVal =
- cast<RecordValue>(getFieldValue(QuuxVal, *BazDecl, Env));
- ASSERT_THAT(BazVal, NotNull());
+ const auto &QuuxLoc =
+ *cast<RecordStorageLocation>(Env.getStorageLocation(*QuuxDecl));
+ const auto &BazLoc =
+ *cast<RecordStorageLocation>(QuuxLoc.getChild(*BazDecl));
- EXPECT_EQ(getFieldValue(QuuxVal, *BarDecl, Env), BarArgVal);
- EXPECT_EQ(getFieldValue(BazVal, *FooDecl, Env), FooArgVal);
- EXPECT_EQ(getFieldValue(QuuxVal, *QuxDecl, Env), QuxArgVal);
+ EXPECT_EQ(getFieldValue(&QuuxLoc, *BarDecl, Env), BarArgVal);
+ EXPECT_EQ(getFieldValue(&BazLoc, *FooDecl, Env), FooArgVal);
+ EXPECT_EQ(getFieldValue(&QuuxLoc, *QuxDecl, Env), QuxArgVal);
// Check that fields initialized in an initializer list are always
// modeled in other instances of the same type.
@@ -3540,8 +3539,9 @@ TEST(TransferTest, AssignMemberBeforeCopy) {
const auto *BarVal = cast<IntegerValue>(Env.getValue(*BarDecl));
- const auto *A2Val = cast<RecordValue>(Env.getValue(*A2Decl));
- EXPECT_EQ(getFieldValue(A2Val, *FooDecl, Env), BarVal);
+ const auto &A2Loc =
+ *cast<RecordStorageLocation>(Env.getStorageLocation(*A2Decl));
+ EXPECT_EQ(getFieldValue(&A2Loc, *FooDecl, Env), BarVal);
});
}
More information about the cfe-commits
mailing list