diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 5cf52ad3d7223..6a6c6856cce95 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 7d9a7b7d28254..d1432fe8217a1 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 3a91025df6e12..d3d5f3338ddf4 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(getValue(Loc))); DeclToLoc[&D] = &Loc; } @@ -622,8 +615,6 @@ StorageLocation *Environment::getStorageLocation(const ValueDecl &D) const { StorageLocation *Loc = It->second; - assert(!isa_and_nonnull(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(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(Val)); if (auto *StructVal = dyn_cast(&Val)) { if (auto *ExistingVal = cast_or_null(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(Val)); - return Val; } @@ -760,6 +745,7 @@ Value *Environment::createValueUnlessSelfReferential( QualType Type, llvm::DenseSet &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(); } - 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(PointeeLoc); - else - return &arena().create(PointeeLoc); + return &arena().create(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(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(&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 f8a049adea5e5..faca68e2f5951 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 ea9052b0e7171..ef51402e171ed 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(V).getReferentLoc()); }); - break; case Value::Kind::Pointer: JOS.attributeObject( "pointee", [&] { dump(cast(V).getPointeeLoc()); }); diff --git a/clang/lib/Analysis/FlowSensitive/Value.cpp b/clang/lib/Analysis/FlowSensitive/Value.cpp index 59affa80bdce9..b069c1cd3da11 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(&Val1)) { - auto *IndVal2 = cast(&Val2); - return &IndVal1->getReferentLoc() == &IndVal2->getReferentLoc(); - } if (auto *IndVal1 = dyn_cast(&Val1)) { auto *IndVal2 = cast(&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(&Val); - return OS << "Reference(" << &RV->getReferentLoc() << ")"; - } case Value::Kind::Pointer: { const auto *PV = dyn_cast(&Val); return OS << "Pointer(" << &PV->getPointeeLoc() << ")"; diff --git a/clang/unittests/Analysis/FlowSensitive/ValueTest.cpp b/clang/unittests/Analysis/FlowSensitive/ValueTest.cpp index 19ea7a04928f2..c5d18ba74c3ed 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);