[clang] 1b334a2 - [clang][dataflow] Eliminate `ReferenceValue`.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 27 06:14:58 PDT 2023
Author: Martin Braenne
Date: 2023-07-27T13:14:47Z
New Revision: 1b334a2ae7af8ad8b2e3367db6433f1bc7b2b1a4
URL: https://github.com/llvm/llvm-project/commit/1b334a2ae7af8ad8b2e3367db6433f1bc7b2b1a4
DIFF: https://github.com/llvm/llvm-project/commit/1b334a2ae7af8ad8b2e3367db6433f1bc7b2b1a4.diff
LOG: [clang][dataflow] Eliminate `ReferenceValue`.
There are no remaining uses of this class in the framework.
This patch is part of the ongoing migration to strict handling of value categories (see https://discourse.llvm.org/t/70086 for details).
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D155922
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
clang/lib/Analysis/FlowSensitive/Value.cpp
clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 5cf52ad3d72235..6a6c6856cce95b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -47,6 +47,7 @@ enum class SkipPast {
/// No indirections should be skipped past.
None,
/// An optional reference should be skipped past.
+ /// This is deprecated; it is equivalent to `None` and will be removed.
Reference,
};
@@ -304,9 +305,10 @@ class Environment {
/// `E` must be a glvalue or a `BuiltinType::BuiltinFn`
void setStorageLocationStrict(const Expr &E, StorageLocation &Loc);
- /// Returns the storage location assigned to `E` in the environment, applying
- /// the `SP` policy for skipping past indirections, or null if `E` isn't
- /// assigned a storage location in the environment.
+ /// Returns the storage location assigned to `E` in the environment, or null
+ /// if `E` isn't assigned a storage location in the environment.
+ ///
+ /// The `SP` parameter has no effect.
///
/// This function is deprecated; prefer `getStorageLocationStrict()`.
/// For details, see https://discourse.llvm.org/t/70086.
@@ -490,12 +492,14 @@ class Environment {
/// isn't assigned a value in the environment.
Value *getValue(const StorageLocation &Loc) const;
- /// Equivalent to `getValue(getStorageLocation(D, SP), SkipPast::None)` if `D`
- /// is assigned a storage location in the environment, otherwise returns null.
+ /// Equivalent to `getValue(getStorageLocation(D))` if `D` is assigned a
+ /// storage location in the environment, otherwise returns null.
Value *getValue(const ValueDecl &D) const;
- /// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if `E`
- /// is assigned a storage location in the environment, otherwise returns null.
+ /// Equivalent to `getValue(getStorageLocation(E, SP))` if `E` is assigned a
+ /// storage location in the environment, otherwise returns null.
+ ///
+ /// The `SP` parameter has no effect.
///
/// This function is deprecated; prefer `getValueStrict()`. For details, see
/// https://discourse.llvm.org/t/70086.
@@ -672,9 +676,6 @@ class Environment {
StorageLocation &createObjectInternal(const VarDecl *D, QualType Ty,
const Expr *InitExpr);
- StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
- const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
-
/// Shared implementation of `pushCall` overloads. Note that unlike
/// `pushCall`, this member is invoked on the environment of the callee, not
/// of the caller.
diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 7d9a7b7d28254b..d1432fe8217a13 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -34,7 +34,6 @@ class Value {
public:
enum class Kind {
Integer,
- Reference,
Pointer,
Struct,
@@ -165,23 +164,6 @@ class IntegerValue : public Value {
}
};
-/// Models a dereferenced pointer. For example, a reference in C++ or an lvalue
-/// in C.
-class ReferenceValue final : public Value {
-public:
- explicit ReferenceValue(StorageLocation &ReferentLoc)
- : Value(Kind::Reference), ReferentLoc(ReferentLoc) {}
-
- static bool classof(const Value *Val) {
- return Val->getKind() == Kind::Reference;
- }
-
- StorageLocation &getReferentLoc() const { return ReferentLoc; }
-
-private:
- StorageLocation &ReferentLoc;
-};
-
/// Models a symbolic pointer. Specifically, any value of type `T*`.
class PointerValue final : public Value {
public:
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 3a91025df6e12a..d3d5f3338ddf49 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -68,7 +68,6 @@ static bool compareDistinctValues(QualType Type, Value &Val1,
case ComparisonResult::Unknown:
switch (Val1.getKind()) {
case Value::Kind::Integer:
- case Value::Kind::Reference:
case Value::Kind::Pointer:
case Value::Kind::Struct:
// FIXME: this choice intentionally introduces unsoundness to allow
@@ -140,11 +139,6 @@ static Value *mergeDistinctValues(QualType Type, Value &Val1,
// FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge`
// returns false to avoid storing unneeded values in `DACtx`.
- // FIXME: Creating the value based on the type alone creates misshapen values
- // for lvalues, since the type does not reflect the need for `ReferenceValue`.
- // This issue will be resolved when `ReferenceValue` is eliminated as part
- // of the ongoing migration to strict handling of value categories (see
- // https://discourse.llvm.org/t/70086 for details).
if (MergedVal)
if (Model.merge(Type, Val1, Env1, Val2, Env2, *MergedVal, MergedEnv))
return MergedVal;
@@ -611,7 +605,6 @@ StorageLocation &Environment::createStorageLocation(const Expr &E) {
void Environment::setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
assert(!DeclToLoc.contains(&D));
- assert(!isa_and_nonnull<ReferenceValue>(getValue(Loc)));
DeclToLoc[&D] = &Loc;
}
@@ -622,8 +615,6 @@ StorageLocation *Environment::getStorageLocation(const ValueDecl &D) const {
StorageLocation *Loc = It->second;
- assert(!isa_and_nonnull<ReferenceValue>(getValue(*Loc)));
-
return Loc;
}
@@ -647,7 +638,7 @@ StorageLocation *Environment::getStorageLocation(const Expr &E,
SkipPast SP) const {
// FIXME: Add a test with parens.
auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
- return It == ExprToLoc.end() ? nullptr : &skip(*It->second, SP);
+ return It == ExprToLoc.end() ? nullptr : &*It->second;
}
StorageLocation *Environment::getStorageLocationStrict(const Expr &E) const {
@@ -659,9 +650,6 @@ StorageLocation *Environment::getStorageLocationStrict(const Expr &E) const {
if (Loc == nullptr)
return nullptr;
- if (auto *RefVal = dyn_cast_or_null<ReferenceValue>(getValue(*Loc)))
- return &RefVal->getReferentLoc();
-
return Loc;
}
@@ -696,7 +684,6 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) {
void Environment::setValueStrict(const Expr &E, Value &Val) {
assert(E.isPRValue());
- assert(!isa<ReferenceValue>(Val));
if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
if (auto *ExistingVal = cast_or_null<StructValue>(getValueStrict(E)))
@@ -739,8 +726,6 @@ Value *Environment::getValueStrict(const Expr &E) const {
assert(E.isPRValue());
Value *Val = getValue(E, SkipPast::None);
- assert(Val == nullptr || !isa<ReferenceValue>(Val));
-
return Val;
}
@@ -760,6 +745,7 @@ Value *Environment::createValueUnlessSelfReferential(
QualType Type, llvm::DenseSet<QualType> &Visited, int Depth,
int &CreatedValuesCount) {
assert(!Type.isNull());
+ assert(!Type->isReferenceType());
// Allow unlimited fields at depth 1; only cap at deeper nesting levels.
if ((Depth > 1 && CreatedValuesCount > MaxCompositeValueSize) ||
@@ -779,16 +765,13 @@ Value *Environment::createValueUnlessSelfReferential(
return &arena().create<IntegerValue>();
}
- if (Type->isReferenceType() || Type->isPointerType()) {
+ if (Type->isPointerType()) {
CreatedValuesCount++;
QualType PointeeType = Type->getPointeeType();
StorageLocation &PointeeLoc =
createLocAndMaybeValue(PointeeType, Visited, Depth, CreatedValuesCount);
- if (Type->isReferenceType())
- return &arena().create<ReferenceValue>(PointeeLoc);
- else
- return &arena().create<PointerValue>(PointeeLoc);
+ return &arena().create<PointerValue>(PointeeLoc);
}
if (Type->isRecordType()) {
@@ -892,25 +875,6 @@ StorageLocation &Environment::createObjectInternal(const VarDecl *D,
return Loc;
}
-StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const {
- switch (SP) {
- case SkipPast::None:
- return Loc;
- case SkipPast::Reference:
- // References cannot be chained so we only need to skip past one level of
- // indirection.
- if (auto *Val = dyn_cast_or_null<ReferenceValue>(getValue(Loc)))
- return Val->getReferentLoc();
- return Loc;
- }
- llvm_unreachable("bad SkipPast kind");
-}
-
-const StorageLocation &Environment::skip(const StorageLocation &Loc,
- SkipPast SP) const {
- return skip(*const_cast<StorageLocation *>(&Loc), SP);
-}
-
void Environment::addToFlowCondition(const Formula &Val) {
DACtx->addFlowConditionConstraint(FlowConditionToken, Val);
}
diff --git a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
index f8a049adea5e5d..faca68e2f5951f 100644
--- a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -26,8 +26,6 @@ llvm::StringRef debugString(Value::Kind Kind) {
switch (Kind) {
case Value::Kind::Integer:
return "Integer";
- case Value::Kind::Reference:
- return "Reference";
case Value::Kind::Pointer:
return "Pointer";
case Value::Kind::Struct:
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index ea9052b0e7171e..ef51402e171ed6 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -99,10 +99,6 @@ class ModelDumper {
case Value::Kind::AtomicBool:
case Value::Kind::FormulaBool:
break;
- case Value::Kind::Reference:
- JOS.attributeObject(
- "referent", [&] { dump(cast<ReferenceValue>(V).getReferentLoc()); });
- break;
case Value::Kind::Pointer:
JOS.attributeObject(
"pointee", [&] { dump(cast<PointerValue>(V).getPointeeLoc()); });
diff --git a/clang/lib/Analysis/FlowSensitive/Value.cpp b/clang/lib/Analysis/FlowSensitive/Value.cpp
index 59affa80bdce99..b069c1cd3da117 100644
--- a/clang/lib/Analysis/FlowSensitive/Value.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Value.cpp
@@ -19,10 +19,6 @@ namespace dataflow {
static bool areEquivalentIndirectionValues(const Value &Val1,
const Value &Val2) {
- if (auto *IndVal1 = dyn_cast<ReferenceValue>(&Val1)) {
- auto *IndVal2 = cast<ReferenceValue>(&Val2);
- return &IndVal1->getReferentLoc() == &IndVal2->getReferentLoc();
- }
if (auto *IndVal1 = dyn_cast<PointerValue>(&Val1)) {
auto *IndVal2 = cast<PointerValue>(&Val2);
return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
@@ -38,10 +34,6 @@ bool areEquivalentValues(const Value &Val1, const Value &Val2) {
raw_ostream &operator<<(raw_ostream &OS, const Value &Val) {
switch (Val.getKind()) {
- case Value::Kind::Reference: {
- const auto *RV = cast<ReferenceValue>(&Val);
- return OS << "Reference(" << &RV->getReferentLoc() << ")";
- }
case Value::Kind::Pointer: {
const auto *PV = dyn_cast<PointerValue>(&Val);
return OS << "Pointer(" << &PV->getPointeeLoc() << ")";
diff --git a/clang/unittests/Analysis/FlowSensitive/ValueTest.cpp b/clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
index 19ea7a04928f28..c5d18ba74c3ed6 100644
--- a/clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
@@ -29,14 +29,6 @@ TEST(ValueTest, DifferentIntegerValuesNotEquivalent) {
EXPECT_FALSE(areEquivalentValues(V1, V2));
}
-TEST(ValueTest, AliasedReferencesEquivalent) {
- auto L = ScalarStorageLocation(QualType());
- ReferenceValue V1(L);
- ReferenceValue V2(L);
- EXPECT_TRUE(areEquivalentValues(V1, V2));
- EXPECT_TRUE(areEquivalentValues(V2, V1));
-}
-
TEST(ValueTest, AliasedPointersEquivalent) {
auto L = ScalarStorageLocation(QualType());
PointerValue V1(L);
@@ -68,21 +60,12 @@ TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
TEST(ValueTest, DifferentKindsNotEquivalent) {
Arena A;
auto L = ScalarStorageLocation(QualType());
- ReferenceValue V1(L);
+ PointerValue V1(L);
TopBoolValue V2(A.makeAtomRef(Atom(0)));
EXPECT_FALSE(areEquivalentValues(V1, V2));
EXPECT_FALSE(areEquivalentValues(V2, V1));
}
-TEST(ValueTest, NotAliasedReferencesNotEquivalent) {
- auto L1 = ScalarStorageLocation(QualType());
- ReferenceValue V1(L1);
- auto L2 = ScalarStorageLocation(QualType());
- ReferenceValue V2(L2);
- EXPECT_FALSE(areEquivalentValues(V1, V2));
- EXPECT_FALSE(areEquivalentValues(V2, V1));
-}
-
TEST(ValueTest, NotAliasedPointersNotEquivalent) {
auto L1 = ScalarStorageLocation(QualType());
PointerValue V1(L1);
More information about the cfe-commits
mailing list