[clang] 44f98d0 - [clang][dataflow] Eliminate duplication between `AggregateStorageLocation` and `StructValue`.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 24 06:20:12 PDT 2023
Author: Martin Braenne
Date: 2023-07-24T13:20:01Z
New Revision: 44f98d0101fe82352e7c5fa98f1b2e9dc1159200
URL: https://github.com/llvm/llvm-project/commit/44f98d0101fe82352e7c5fa98f1b2e9dc1159200
DIFF: https://github.com/llvm/llvm-project/commit/44f98d0101fe82352e7c5fa98f1b2e9dc1159200.diff
LOG: [clang][dataflow] Eliminate duplication between `AggregateStorageLocation` and `StructValue`.
After this change, `StructValue` is just a wrapper for an `AggregateStorageLocation`. For the wider context, see https://discourse.llvm.org/t/70086.
## How to review
- Start by looking at the comments added / changed in Value.h, StorageLocation.h,
and DataflowEnvironment.h. This will give you a good overview of the semantic
changes.
- Look at the corresponding .cpp files that implement the semantic changes.
- Transfer.cpp, TypeErasedDataflowAnalysis.cpp, and RecordOps.cpp show how the
core of the framework is affected by the semantic changes.
- UncheckedOptionalAccessModel.cpp shows how this complex model is affected by
the changes.
- Many of the changes in the rest of the patch are mechanical in nature.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155446
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/RecordOps.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 9d99b6771f71e1..5cf52ad3d72235 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -334,6 +334,31 @@ class Environment {
/// in the environment.
AggregateStorageLocation *getThisPointeeStorageLocation() const;
+ /// Returns the location of the result object for a record-type prvalue.
+ ///
+ /// In C++, prvalues of record type serve only a limited purpose: They can
+ /// only be used to initialize a result object (e.g. a variable or a
+ /// temporary). This function returns the location of that result object.
+ ///
+ /// When creating a prvalue of record type, we already need the storage
+ /// location of the result object to pass in `this`, even though prvalues are
+ /// otherwise not associated with storage locations.
+ ///
+ /// FIXME: Currently, this simply returns a stable storage location for `E`,
+ /// but this doesn't do the right thing in scenarios like the following:
+ /// ```
+ /// MyClass c = some_condition()? MyClass(foo) : MyClass(bar);
+ /// ```
+ /// Here, `MyClass(foo)` and `MyClass(bar)` will have two
diff erent storage
+ /// locations, when in fact their storage locations should be the same.
+ /// Eventually, we want to propagate storage locations from result objects
+ /// down to the prvalues that initialize them, similar to the way that this is
+ /// done in Clang's CodeGen.
+ ///
+ /// Requirements:
+ /// `E` must be a prvalue of record type.
+ AggregateStorageLocation &getResultObjectLocation(const Expr &RecordPRValue);
+
/// Returns the return value of the current function. This can be null if:
/// - The function has a void return type
/// - No return value could be determined for the function, for example
@@ -385,10 +410,16 @@ class Environment {
PointerValue &getOrCreateNullPointerValue(QualType PointeeType);
/// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
- /// return null. If `Type` is a pointer or reference type, creates all the
- /// necessary storage locations and values for indirections until it finds a
+ /// returns null.
+ ///
+ /// If `Type` is a pointer or reference type, creates all the necessary
+ /// storage locations and values for indirections until it finds a
/// non-pointer/non-reference type.
///
+ /// If `Type` is a class, struct, or union type, calls `setValue()` to
+ /// associate the `StructValue` with its storage location
+ /// (`StructValue::getAggregateLoc()`).
+ ///
/// If `Type` is one of the following types, this function will always return
/// a non-null pointer:
/// - `bool`
@@ -430,7 +461,7 @@ class Environment {
void setValue(const StorageLocation &Loc, Value &Val);
/// Clears any association between `Loc` and a value in the environment.
- void clearValue(const StorageLocation &Loc);
+ void clearValue(const StorageLocation &Loc) { LocToVal.erase(&Loc); }
/// Assigns `Val` as the value of the prvalue `E` in the environment.
///
@@ -449,6 +480,10 @@ class Environment {
///
/// `E` must be a prvalue
/// `Val` must not be a `ReferenceValue`
+ /// If `Val` is a `StructValue`, its `AggregateStorageLocation` must be the
+ /// same as that of any `StructValue` that has already been associated with
+ /// `E`. This is to guarantee that the result object initialized by a prvalue
+ /// `StructValue` has a durable storage location.
void setValueStrict(const Expr &E, Value &Val);
/// Returns the value assigned to `Loc` in the environment or null if `Loc`
@@ -624,6 +659,14 @@ class Environment {
llvm::DenseSet<QualType> &Visited,
int Depth, int &CreatedValuesCount);
+ /// Creates a storage location for `Ty`. Also creates and associates a value
+ /// with the storage location, unless values of this type are not supported or
+ /// we hit one of the limits at which we stop producing values (controlled by
+ /// `Visited`, `Depth`, and `CreatedValuesCount`).
+ StorageLocation &createLocAndMaybeValue(QualType Ty,
+ llvm::DenseSet<QualType> &Visited,
+ int Depth, int &CreatedValuesCount);
+
/// Shared implementation of `createObject()` overloads.
/// `D` and `InitExpr` may be null.
StorageLocation &createObjectInternal(const VarDecl *D, QualType Ty,
@@ -672,12 +715,6 @@ class Environment {
// deterministic sequence. This in turn produces deterministic SAT formulas.
llvm::MapVector<const StorageLocation *, Value *> LocToVal;
- // Maps locations of struct members to symbolic values of the structs that own
- // them and the decls of the struct members.
- llvm::DenseMap<const StorageLocation *,
- std::pair<StructValue *, const ValueDecl *>>
- MemberLocToStruct;
-
Atom FlowConditionToken;
};
diff --git a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
index 453f1e13362b7d..62e3d5e59c6590 100644
--- a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
+++ b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
@@ -18,6 +18,7 @@
#include "clang/AST/Type.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/Support/Debug.h"
+#include <cassert>
#define DEBUG_TYPE "dataflow"
@@ -32,7 +33,9 @@ class StorageLocation {
public:
enum class Kind { Scalar, Aggregate };
- StorageLocation(Kind LocKind, QualType Type) : LocKind(LocKind), Type(Type) {}
+ StorageLocation(Kind LocKind, QualType Type) : LocKind(LocKind), Type(Type) {
+ assert(Type.isNull() || !Type->isReferenceType());
+ }
// Non-copyable because addresses of storage locations are used as their
// identities throughout framework and user code. The framework is responsible
@@ -68,6 +71,14 @@ class ScalarStorageLocation final : public StorageLocation {
/// struct with public members. The child map is flat, so when used for a struct
/// or class type, all accessible members of base struct and class types are
/// directly accesible as children of this location.
+///
+/// The storage location for a field of reference type may be null. This
+/// typically occurs in one of two situations:
+/// - The record has not been fully initialized.
+/// - The maximum depth for modelling a self-referential data structure has been
+/// reached.
+/// Storage locations for fields of all other types must be non-null.
+///
/// FIXME: Currently, the storage location of unions is modelled the same way as
/// that of structs or classes. Eventually, we need to change this modelling so
/// that all of the members of a given union have the same storage location.
@@ -78,15 +89,32 @@ class AggregateStorageLocation final : public StorageLocation {
explicit AggregateStorageLocation(QualType Type)
: AggregateStorageLocation(Type, FieldToLoc()) {}
- AggregateStorageLocation(QualType Type, FieldToLoc Children)
- : StorageLocation(Kind::Aggregate, Type), Children(std::move(Children)) {}
+ AggregateStorageLocation(QualType Type, FieldToLoc TheChildren)
+ : StorageLocation(Kind::Aggregate, Type),
+ Children(std::move(TheChildren)) {
+ assert(!Type.isNull());
+ assert(Type->isRecordType());
+ assert([this] {
+ for (auto [Field, Loc] : Children) {
+ if (!Field->getType()->isReferenceType() && Loc == nullptr)
+ return false;
+ }
+ return true;
+ }());
+ }
static bool classof(const StorageLocation *Loc) {
return Loc->getKind() == Kind::Aggregate;
}
/// Returns the child storage location for `D`.
- StorageLocation &getChild(const ValueDecl &D) const {
+ ///
+ /// May return null if `D` has reference type; guaranteed to return non-null
+ /// in all other cases.
+ ///
+ /// Note that it is an error to call this with a field that does not exist.
+ /// The function does not return null in this case.
+ StorageLocation *getChild(const ValueDecl &D) const {
auto It = Children.find(&D);
LLVM_DEBUG({
if (It == Children.end()) {
@@ -100,7 +128,19 @@ class AggregateStorageLocation final : public StorageLocation {
}
});
assert(It != Children.end());
- return *It->second;
+ return It->second;
+ }
+
+ /// Changes the child storage location for a field `D` of reference type.
+ /// All other fields cannot change their storage location and always retain
+ /// the storage location passed to the `AggregateStorageLocation` constructor.
+ ///
+ /// Requirements:
+ ///
+ /// `D` must have reference type.
+ void setChild(const ValueDecl &D, StorageLocation *Loc) {
+ assert(D.getType()->isReferenceType());
+ Children[&D] = Loc;
}
llvm::iterator_range<FieldToLoc::const_iterator> children() const {
diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h
index 1d4f8008d19307..7d9a7b7d28254b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -198,38 +198,59 @@ class PointerValue final : public Value {
StorageLocation &PointeeLoc;
};
-/// Models a value of `struct` or `class` type, with a flat map of fields to
-/// child storage locations, containing all accessible members of base struct
-/// and class types.
+/// Models a value of `struct` or `class` type.
+/// In C++, prvalues of class type serve only a limited purpose: They can only
+/// be used to initialize a result object. It is not possible to access member
+/// variables or call member functions on a prvalue of class type.
+/// Correspondingly, `StructValue` also serves only two limited purposes:
+/// - It conveys a prvalue of class type from the place where the object is
+/// constructed to the result object that it initializes.
+///
+/// When creating a prvalue of class type, we already need a storage location
+/// for `this`, even though prvalues are otherwise not associated with storage
+/// locations. `StructValue` is therefore essentially a wrapper for a storage
+/// location, which is then used to set the storage location for the result
+/// object when we process the AST node for that result object.
+///
+/// For example:
+/// MyStruct S = MyStruct(3);
+///
+/// In this example, `MyStruct(3) is a prvalue, which is modeled as a
+/// `StructValue` that wraps an `AbstractStorageLocation`. This
+// `AbstractStorageLocation` is then used as the storage location for `S`.
+///
+/// - It allows properties to be associated with an object of class type.
+/// Note that when doing so, you should avoid mutating the properties of an
+/// existing `StructValue` in place, as these changes would be visible to
+/// other `Environment`s that share the same `StructValue`. Instead, associate
+/// a new `StructValue` with the `AggregateStorageLocation` and set the
+/// properties on this new `StructValue`. (See also `refreshStructValue()` in
+/// DataflowEnvironment.h, which makes this easy.)
+/// Note also that this implies that it is common for the same
+/// `AggregateStorageLocation` to be associated with
diff erent `StructValue`s
+/// in
diff erent environments.
+/// Over time, we may eliminate `StructValue` entirely. See also the discussion
+/// here: https://reviews.llvm.org/D155204#inline-1503204
class StructValue final : public Value {
public:
- StructValue() : StructValue(llvm::DenseMap<const ValueDecl *, Value *>()) {}
-
- explicit StructValue(llvm::DenseMap<const ValueDecl *, Value *> Children)
- : Value(Kind::Struct), Children(std::move(Children)) {}
+ explicit StructValue(AggregateStorageLocation &Loc)
+ : Value(Kind::Struct), Loc(Loc) {}
static bool classof(const Value *Val) {
return Val->getKind() == Kind::Struct;
}
- /// Returns the child value that is assigned for `D` or null if the child is
- /// not initialized.
- Value *getChild(const ValueDecl &D) const { return Children.lookup(&D); }
-
- /// Assigns `Val` as the child value for `D`.
- void setChild(const ValueDecl &D, Value &Val) { Children[&D] = &Val; }
-
- /// Clears any value assigned as the child value for `D`.
- void clearChild(const ValueDecl &D) { Children.erase(&D); }
+ /// Returns the storage location that this `StructValue` is associated with.
+ AggregateStorageLocation &getAggregateLoc() const { return Loc; }
- llvm::iterator_range<
- llvm::DenseMap<const ValueDecl *, Value *>::const_iterator>
- children() const {
- return {Children.begin(), Children.end()};
+ /// Convenience function that returns the child storage location for `Field`.
+ /// See also the documentation for `AggregateStorageLocation::getChild()`.
+ StorageLocation *getChild(const ValueDecl &Field) const {
+ return Loc.getChild(Field);
}
private:
- llvm::DenseMap<const ValueDecl *, Value *> Children;
+ AggregateStorageLocation &Loc;
};
raw_ostream &operator<<(raw_ostream &OS, const Value &Val);
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index bb1c2c7a1de10d..9f72dc8f6ab38b 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -62,7 +62,11 @@ StorageLocation &DataflowAnalysisContext::createStorageLocation(QualType Type) {
if (!Type.isNull() && Type->isRecordType()) {
llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
for (const FieldDecl *Field : getModeledFields(Type))
- FieldLocs.insert({Field, &createStorageLocation(Field->getType())});
+ if (Field->getType()->isReferenceType())
+ FieldLocs.insert({Field, nullptr});
+ else
+ FieldLocs.insert({Field, &createStorageLocation(
+ Field->getType().getNonReferenceType())});
return arena().create<AggregateStorageLocation>(Type, std::move(FieldLocs));
}
return arena().create<ScalarStorageLocation>(Type);
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 54632f5662b63d..5514d25df6cc63 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -120,6 +120,24 @@ static Value *mergeDistinctValues(QualType Type, Value &Val1,
return &A.makeBoolValue(MergedVal);
}
+ Value *MergedVal = nullptr;
+ if (auto *StructVal1 = dyn_cast<StructValue>(&Val1)) {
+ auto *StructVal2 = cast<StructValue>(&Val2);
+
+ // Values to be merged are always associated with the same location in
+ // `LocToVal`. The location stored in `StructVal` should therefore also
+ // be the same.
+ assert(&StructVal1->getAggregateLoc() == &StructVal2->getAggregateLoc());
+
+ // `StructVal1` and `StructVal2` may have
diff erent properties associated
+ // with them. Create a new `StructValue` without any properties so that we
+ // soundly approximate both values. If a particular analysis needs to merge
+ // properties, it should do so in `DataflowAnalysis::merge()`.
+ MergedVal = &MergedEnv.create<StructValue>(StructVal1->getAggregateLoc());
+ } else {
+ MergedVal = MergedEnv.createValue(Type);
+ }
+
// 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
@@ -127,7 +145,7 @@ static Value *mergeDistinctValues(QualType Type, Value &Val1,
// 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 (Value *MergedVal = MergedEnv.createValue(Type))
+ if (MergedVal)
if (Model.merge(Type, Val1, Env1, Val2, Env2, *MergedVal, MergedEnv))
return MergedVal;
@@ -307,10 +325,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
// FIXME: Initialize the ThisPointeeLoc of lambdas too.
if (MethodDecl && !MethodDecl->isStatic()) {
QualType ThisPointeeType = MethodDecl->getThisObjectType();
- ThisPointeeLoc = &cast<AggregateStorageLocation>(
- createStorageLocation(ThisPointeeType));
- if (Value *ThisPointeeVal = createValue(ThisPointeeType))
- setValue(*ThisPointeeLoc, *ThisPointeeVal);
+ ThisPointeeLoc =
+ &cast<StructValue>(createValue(ThisPointeeType))->getAggregateLoc();
}
}
}
@@ -342,10 +358,7 @@ Environment Environment::pushCall(const CallExpr *Call) const {
Environment Environment::pushCall(const CXXConstructExpr *Call) const {
Environment Env(*this);
- Env.ThisPointeeLoc = &cast<AggregateStorageLocation>(
- Env.createStorageLocation(Call->getType()));
- if (Value *Val = Env.createValue(Call->getType()))
- Env.setValue(*Env.ThisPointeeLoc, *Val);
+ Env.ThisPointeeLoc = &Env.getResultObjectLocation(*Call);
Env.pushCallInternal(Call->getConstructor(),
llvm::ArrayRef(Call->getArgs(), Call->getNumArgs()));
@@ -385,7 +398,6 @@ void Environment::popCall(const CallExpr *Call, const Environment &CalleeEnv) {
// these maps capture information from the local scope, so when popping that
// scope, we do not propagate the maps.
this->LocToVal = std::move(CalleeEnv.LocToVal);
- this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
if (Call->isGLValue()) {
@@ -401,7 +413,6 @@ void Environment::popCall(const CXXConstructExpr *Call,
const Environment &CalleeEnv) {
// See also comment in `popCall(const CallExpr *, const Environment &)` above.
this->LocToVal = std::move(CalleeEnv.LocToVal);
- this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
if (Value *Val = CalleeEnv.getValue(*CalleeEnv.ThisPointeeLoc)) {
@@ -466,15 +477,8 @@ LatticeJoinEffect Environment::widen(const Environment &PrevEnv,
// block. For `DeclToLoc` and `ExprToLoc`, join guarantees that these maps are
// subsets of the maps in `PrevEnv`. So, as long as we maintain this property
// here, we don't need change their current values to widen.
- //
- // FIXME: `MemberLocToStruct` does not share the above property, because
- // `join` can cause the map size to increase (when we add fresh data in places
- // of conflict). Once this issue with join is resolved, re-enable the
- // assertion below or replace with something that captures the desired
- // invariant.
assert(DeclToLoc.size() <= PrevEnv.DeclToLoc.size());
assert(ExprToLoc.size() <= PrevEnv.ExprToLoc.size());
- // assert(MemberLocToStruct.size() <= PrevEnv.MemberLocToStruct.size());
llvm::MapVector<const StorageLocation *, Value *> WidenedLocToVal;
for (auto &Entry : LocToVal) {
@@ -501,12 +505,9 @@ LatticeJoinEffect Environment::widen(const Environment &PrevEnv,
Effect = LatticeJoinEffect::Changed;
}
LocToVal = std::move(WidenedLocToVal);
- // FIXME: update the equivalence calculation for `MemberLocToStruct`, once we
- // have a systematic way of soundly comparing this map.
if (DeclToLoc.size() != PrevEnv.DeclToLoc.size() ||
ExprToLoc.size() != PrevEnv.ExprToLoc.size() ||
- LocToVal.size() != PrevEnv.LocToVal.size() ||
- MemberLocToStruct.size() != PrevEnv.MemberLocToStruct.size())
+ LocToVal.size() != PrevEnv.LocToVal.size())
Effect = LatticeJoinEffect::Changed;
return Effect;
@@ -559,9 +560,6 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB,
JoinedEnv.ExprToLoc = intersectDenseMaps(EnvA.ExprToLoc, EnvB.ExprToLoc);
- JoinedEnv.MemberLocToStruct =
- intersectDenseMaps(EnvA.MemberLocToStruct, EnvB.MemberLocToStruct);
-
// FIXME: update join to detect backedges and simplify the flow condition
// accordingly.
JoinedEnv.FlowConditionToken = EnvA.DACtx->joinFlowConditions(
@@ -671,64 +669,46 @@ AggregateStorageLocation *Environment::getThisPointeeStorageLocation() const {
return ThisPointeeLoc;
}
+AggregateStorageLocation &
+Environment::getResultObjectLocation(const Expr &RecordPRValue) {
+ assert(RecordPRValue.getType()->isRecordType());
+ assert(RecordPRValue.isPRValue());
+
+ if (StorageLocation *ExistingLoc =
+ getStorageLocation(RecordPRValue, SkipPast::None))
+ return *cast<AggregateStorageLocation>(ExistingLoc);
+ auto &Loc = cast<AggregateStorageLocation>(
+ DACtx->getStableStorageLocation(RecordPRValue));
+ setStorageLocation(RecordPRValue, Loc);
+ return Loc;
+}
+
PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
return DACtx->getOrCreateNullPointerValue(PointeeType);
}
void Environment::setValue(const StorageLocation &Loc, Value &Val) {
- LocToVal[&Loc] = &Val;
-
- if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
- auto &AggregateLoc = *cast<AggregateStorageLocation>(&Loc);
-
- const QualType Type = AggregateLoc.getType();
- assert(Type->isRecordType());
-
- for (const FieldDecl *Field : DACtx->getModeledFields(Type)) {
- assert(Field != nullptr);
- StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
- MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
- if (auto *FieldVal = StructVal->getChild(*Field))
- setValue(FieldLoc, *FieldVal);
- }
- }
-
- auto It = MemberLocToStruct.find(&Loc);
- if (It != MemberLocToStruct.end()) {
- // `Loc` is the location of a struct member so we need to also update the
- // value of the member in the corresponding `StructValue`.
-
- assert(It->second.first != nullptr);
- StructValue &StructVal = *It->second.first;
-
- assert(It->second.second != nullptr);
- const ValueDecl &Member = *It->second.second;
-
- StructVal.setChild(Member, Val);
- }
-}
-
-void Environment::clearValue(const StorageLocation &Loc) {
- LocToVal.erase(&Loc);
-
- if (auto It = MemberLocToStruct.find(&Loc); It != MemberLocToStruct.end()) {
- // `Loc` is the location of a struct member so we need to also clear the
- // member in the corresponding `StructValue`.
-
- assert(It->second.first != nullptr);
- StructValue &StructVal = *It->second.first;
+ assert(!isa<StructValue>(&Val) ||
+ &cast<StructValue>(&Val)->getAggregateLoc() == &Loc);
- assert(It->second.second != nullptr);
- const ValueDecl &Member = *It->second.second;
-
- StructVal.clearChild(Member);
- }
+ LocToVal[&Loc] = &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)))
+ assert(&ExistingVal->getAggregateLoc() == &StructVal->getAggregateLoc());
+ if (StorageLocation *ExistingLoc = getStorageLocation(E, SkipPast::None))
+ assert(ExistingLoc == &StructVal->getAggregateLoc());
+ else
+ setStorageLocation(E, StructVal->getAggregateLoc());
+ setValue(StructVal->getAggregateLoc(), Val);
+ return;
+ }
+
StorageLocation *Loc = getStorageLocation(E, SkipPast::None);
if (Loc == nullptr) {
Loc = &createStorageLocation(E);
@@ -802,16 +782,8 @@ Value *Environment::createValueUnlessSelfReferential(
if (Type->isReferenceType() || Type->isPointerType()) {
CreatedValuesCount++;
QualType PointeeType = Type->getPointeeType();
- auto &PointeeLoc = createStorageLocation(PointeeType);
-
- if (Visited.insert(PointeeType.getCanonicalType()).second) {
- Value *PointeeVal = createValueUnlessSelfReferential(
- PointeeType, Visited, Depth, CreatedValuesCount);
- Visited.erase(PointeeType.getCanonicalType());
-
- if (PointeeVal != nullptr)
- setValue(PointeeLoc, *PointeeVal);
- }
+ StorageLocation &PointeeLoc =
+ createLocAndMaybeValue(PointeeType, Visited, Depth, CreatedValuesCount);
if (Type->isReferenceType())
return &arena().create<ReferenceValue>(PointeeLoc);
@@ -821,27 +793,54 @@ Value *Environment::createValueUnlessSelfReferential(
if (Type->isRecordType()) {
CreatedValuesCount++;
- llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
+ llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
for (const FieldDecl *Field : DACtx->getModeledFields(Type)) {
assert(Field != nullptr);
QualType FieldType = Field->getType();
- if (Visited.contains(FieldType.getCanonicalType()))
- continue;
-
- Visited.insert(FieldType.getCanonicalType());
- if (auto *FieldValue = createValueUnlessSelfReferential(
- FieldType, Visited, Depth + 1, CreatedValuesCount))
- FieldValues.insert({Field, FieldValue});
- Visited.erase(FieldType.getCanonicalType());
+
+ FieldLocs.insert(
+ {Field, &createLocAndMaybeValue(FieldType, Visited, Depth + 1,
+ CreatedValuesCount)});
}
- return &arena().create<StructValue>(std::move(FieldValues));
+ AggregateStorageLocation &Loc =
+ arena().create<AggregateStorageLocation>(Type, std::move(FieldLocs));
+ StructValue &StructVal = create<StructValue>(Loc);
+
+ // As we already have a storage location for the `StructValue`, we can and
+ // should associate them in the environment.
+ setValue(Loc, StructVal);
+
+ return &StructVal;
}
return nullptr;
}
+StorageLocation &
+Environment::createLocAndMaybeValue(QualType Ty,
+ llvm::DenseSet<QualType> &Visited,
+ int Depth, int &CreatedValuesCount) {
+ if (!Visited.insert(Ty.getCanonicalType()).second)
+ return createStorageLocation(Ty.getNonReferenceType());
+ Value *Val = createValueUnlessSelfReferential(
+ Ty.getNonReferenceType(), Visited, Depth, CreatedValuesCount);
+ Visited.erase(Ty.getCanonicalType());
+
+ Ty = Ty.getNonReferenceType();
+
+ if (Val == nullptr)
+ return createStorageLocation(Ty);
+
+ if (Ty->isRecordType())
+ return cast<StructValue>(Val)->getAggregateLoc();
+
+ StorageLocation &Loc = createStorageLocation(Ty);
+ setValue(Loc, *Val);
+ return Loc;
+}
+
StorageLocation &Environment::createObjectInternal(const VarDecl *D,
QualType Ty,
const Expr *InitExpr) {
@@ -881,6 +880,9 @@ StorageLocation &Environment::createObjectInternal(const VarDecl *D,
if (!Val)
Val = createValue(Ty);
+ if (Ty->isRecordType())
+ return cast<StructValue>(Val)->getAggregateLoc();
+
StorageLocation &Loc =
D ? createStorageLocation(*D) : createStorageLocation(Ty);
@@ -989,7 +991,7 @@ std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
StructValue &refreshStructValue(AggregateStorageLocation &Loc,
Environment &Env) {
- auto &NewVal = *cast<StructValue>(Env.createValue(Loc.getType()));
+ auto &NewVal = Env.create<StructValue>(Loc);
Env.setValue(Loc, NewVal);
return NewVal;
}
@@ -997,19 +999,28 @@ StructValue &refreshStructValue(AggregateStorageLocation &Loc,
StructValue &refreshStructValue(const Expr &Expr, Environment &Env) {
assert(Expr.getType()->isRecordType());
- auto &NewVal = *cast<StructValue>(Env.createValue(Expr.getType()));
-
if (Expr.isPRValue()) {
- Env.setValueStrict(Expr, NewVal);
- } else {
- StorageLocation *Loc = Env.getStorageLocationStrict(Expr);
- if (Loc == nullptr) {
- Loc = &Env.createStorageLocation(Expr);
- Env.setStorageLocation(Expr, *Loc);
+ if (auto *ExistingVal =
+ cast_or_null<StructValue>(Env.getValueStrict(Expr))) {
+ auto &NewVal = Env.create<StructValue>(ExistingVal->getAggregateLoc());
+ Env.setValueStrict(Expr, NewVal);
+ return NewVal;
}
+
+ auto &NewVal = *cast<StructValue>(Env.createValue(Expr.getType()));
+ Env.setValueStrict(Expr, NewVal);
+ return NewVal;
+ }
+
+ if (auto *Loc = cast_or_null<AggregateStorageLocation>(
+ Env.getStorageLocationStrict(Expr))) {
+ auto &NewVal = Env.create<StructValue>(*Loc);
Env.setValue(*Loc, NewVal);
+ return NewVal;
}
+ auto &NewVal = *cast<StructValue>(Env.createValue(Expr.getType()));
+ Env.setStorageLocationStrict(Expr, NewVal.getAggregateLoc());
return NewVal;
}
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index 0c4e838e667be1..ee89e074f84676 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -108,9 +108,13 @@ class ModelDumper {
"pointee", [&] { dump(cast<PointerValue>(V).getPointeeLoc()); });
break;
case Value::Kind::Struct:
- for (const auto &Child : cast<StructValue>(V).children())
- JOS.attributeObject("f:" + Child.first->getNameAsString(),
- [&] { dump(*Child.second); });
+ for (const auto &Child :
+ cast<StructValue>(V).getAggregateLoc().children())
+ JOS.attributeObject("f:" + Child.first->getNameAsString(), [&] {
+ if (Child.second)
+ if (Value *Val = Env.getValue(*Child.second))
+ dump(*Val);
+ });
break;
}
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index a39aa2240c4fc6..1db255dd81f3f8 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -254,21 +254,14 @@ void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {
OptionalVal.setProperty("has_value", HasValueVal);
}
-/// Creates a symbolic value for an `optional` value using `HasValueVal` as the
-/// symbolic value of its "has_value" property.
-StructValue &createOptionalValue(BoolValue &HasValueVal, Environment &Env) {
- auto &OptionalVal = Env.create<StructValue>();
- setHasValue(OptionalVal, HasValueVal);
- return OptionalVal;
-}
-
/// Creates a symbolic value for an `optional` value at an existing storage
/// location. Uses `HasValueVal` as the symbolic value of the "has_value"
/// property.
StructValue &createOptionalValue(AggregateStorageLocation &Loc,
BoolValue &HasValueVal, Environment &Env) {
- StructValue &OptionalVal = createOptionalValue(HasValueVal, Env);
+ auto &OptionalVal = Env.create<StructValue>(Loc);
Env.setValue(Loc, OptionalVal);
+ setHasValue(OptionalVal, HasValueVal);
return OptionalVal;
}
@@ -324,37 +317,38 @@ StorageLocation *maybeInitializeOptionalValueMember(QualType Q,
if (auto *ValueProp = OptionalVal.getProperty("value")) {
auto *ValuePtr = clang::cast<PointerValue>(ValueProp);
auto &ValueLoc = ValuePtr->getPointeeLoc();
- if (Env.getValue(ValueLoc) == nullptr) {
- // The property was previously set, but the value has been lost. This can
- // happen in various situations, for example:
- // - Because of an environment merge (where the two environments mapped
- // the property to
diff erent values, which resulted in them both being
- // discarded).
- // - When two blocks in the CFG, with neither a dominator of the other,
- // visit the same optional value. (FIXME: This is something we can and
- // should fix -- see also the lengthy FIXME below.)
- // - Or even when a block is revisited during testing to collect
- // per-statement state.
- //
- // FIXME: This situation means that the optional contents are not shared
- // between branches and the like. Practically, this lack of sharing
- // reduces the precision of the model when the contents are relevant to
- // the check, like another optional or a boolean that influences control
- // flow.
+ if (Env.getValue(ValueLoc) != nullptr)
+ return &ValueLoc;
+
+ // The property was previously set, but the value has been lost. This can
+ // happen in various situations, for example:
+ // - Because of an environment merge (where the two environments mapped the
+ // property to
diff erent values, which resulted in them both being
+ // discarded).
+ // - When two blocks in the CFG, with neither a dominator of the other,
+ // visit the same optional value. (FIXME: This is something we can and
+ // should fix -- see also the lengthy FIXME below.)
+ // - Or even when a block is revisited during testing to collect
+ // per-statement state.
+ // FIXME: This situation means that the optional contents are not shared
+ // between branches and the like. Practically, this lack of sharing
+ // reduces the precision of the model when the contents are relevant to
+ // the check, like another optional or a boolean that influences control
+ // flow.
+ if (ValueLoc.getType()->isRecordType()) {
+ refreshStructValue(cast<AggregateStorageLocation>(ValueLoc), Env);
+ return &ValueLoc;
+ } else {
auto *ValueVal = Env.createValue(ValueLoc.getType());
if (ValueVal == nullptr)
return nullptr;
Env.setValue(ValueLoc, *ValueVal);
+ return &ValueLoc;
}
- return &ValueLoc;
}
auto Ty = Q.getNonReferenceType();
- auto *ValueVal = Env.createValue(Ty);
- if (ValueVal == nullptr)
- return nullptr;
- auto &ValueLoc = Env.createStorageLocation(Ty);
- Env.setValue(ValueLoc, *ValueVal);
+ auto &ValueLoc = Env.createObject(Ty);
auto &ValuePtr = Env.create<PointerValue>(ValueLoc);
// FIXME:
// The change we make to the `value` property below may become visible to
@@ -467,10 +461,8 @@ void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
void transferMakeOptionalCall(const CallExpr *E,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
- auto &Loc = State.Env.createStorageLocation(*E);
- State.Env.setStorageLocation(*E, Loc);
- State.Env.setValue(
- Loc, createOptionalValue(State.Env.getBoolLiteralValue(true), State.Env));
+ createOptionalValue(State.Env.getResultObjectLocation(*E),
+ State.Env.getBoolLiteralValue(true), State.Env);
}
void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
@@ -545,15 +537,21 @@ void transferCallReturningOptional(const CallExpr *E,
if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
return;
- auto &Loc = State.Env.createStorageLocation(*E);
- State.Env.setStorageLocation(*E, Loc);
- State.Env.setValue(
- Loc, createOptionalValue(State.Env.makeAtomicBoolValue(), State.Env));
+ AggregateStorageLocation *Loc = nullptr;
+ if (E->isPRValue()) {
+ Loc = &State.Env.getResultObjectLocation(*E);
+ } else {
+ Loc = &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
+ State.Env.setStorageLocationStrict(*E, *Loc);
+ }
+
+ createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
}
void constructOptionalValue(const Expr &E, Environment &Env,
BoolValue &HasValueVal) {
- Env.setValueStrict(E, createOptionalValue(HasValueVal, Env));
+ AggregateStorageLocation &Loc = Env.getResultObjectLocation(E);
+ Env.setValueStrict(E, createOptionalValue(Loc, HasValueVal, Env));
}
/// Returns a symbolic value for the "has_value" property of an `optional<T>`
@@ -1032,7 +1030,8 @@ Value *UncheckedOptionalAccessModel::widen(QualType Type, Value &Prev,
if (isa<TopBoolValue>(CurrentHasVal))
return &Current;
}
- return &createOptionalValue(CurrentEnv.makeTopBoolValue(), CurrentEnv);
+ return &createOptionalValue(cast<StructValue>(Current).getAggregateLoc(),
+ CurrentEnv.makeTopBoolValue(), CurrentEnv);
case ComparisonResult::Unknown:
return nullptr;
}
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index eac9e3d4f93f29..60144531c25186 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -28,32 +28,33 @@ void clang::dataflow::copyRecord(AggregateStorageLocation &Src,
Src.getType().getCanonicalType().getUnqualifiedType());
for (auto [Field, SrcFieldLoc] : Src.children()) {
- assert(SrcFieldLoc != nullptr);
+ StorageLocation *DstFieldLoc = Dst.getChild(*Field);
- StorageLocation &DstFieldLoc = Dst.getChild(*Field);
+ assert(Field->getType()->isReferenceType() ||
+ (SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
if (Field->getType()->isRecordType()) {
copyRecord(cast<AggregateStorageLocation>(*SrcFieldLoc),
- cast<AggregateStorageLocation>(DstFieldLoc), Env);
+ cast<AggregateStorageLocation>(*DstFieldLoc), Env);
+ } else if (Field->getType()->isReferenceType()) {
+ Dst.setChild(*Field, SrcFieldLoc);
} else {
if (Value *Val = Env.getValue(*SrcFieldLoc))
- Env.setValue(DstFieldLoc, *Val);
+ Env.setValue(*DstFieldLoc, *Val);
else
- Env.clearValue(DstFieldLoc);
+ Env.clearValue(*DstFieldLoc);
}
}
StructValue *SrcVal = cast_or_null<StructValue>(Env.getValue(Src));
StructValue *DstVal = cast_or_null<StructValue>(Env.getValue(Dst));
- if (SrcVal == nullptr || DstVal == nullptr)
- return;
-
- auto DstChildren = DstVal->children();
- DstVal = &Env.create<StructValue>(llvm::DenseMap<const ValueDecl *, Value *>(
- DstChildren.begin(), DstChildren.end()));
+ DstVal = &Env.create<StructValue>(Dst);
Env.setValue(Dst, *DstVal);
+ if (SrcVal == nullptr)
+ return;
+
for (const auto &[Name, Value] : SrcVal->properties()) {
if (Value != nullptr)
DstVal->setProperty(Name, *Value);
@@ -75,29 +76,20 @@ bool clang::dataflow::recordsEqual(const AggregateStorageLocation &Loc1,
Loc1.getType().getCanonicalType().getUnqualifiedType());
for (auto [Field, FieldLoc1] : Loc1.children()) {
- assert(FieldLoc1 != nullptr);
+ StorageLocation *FieldLoc2 = Loc2.getChild(*Field);
- StorageLocation &FieldLoc2 = Loc2.getChild(*Field);
+ assert(Field->getType()->isReferenceType() ||
+ (FieldLoc1 != nullptr && FieldLoc2 != nullptr));
if (Field->getType()->isRecordType()) {
if (!recordsEqual(cast<AggregateStorageLocation>(*FieldLoc1), Env1,
- cast<AggregateStorageLocation>(FieldLoc2), Env2))
+ cast<AggregateStorageLocation>(*FieldLoc2), Env2))
return false;
} else if (Field->getType()->isReferenceType()) {
- auto *RefVal1 = cast_or_null<ReferenceValue>(Env1.getValue(*FieldLoc1));
- auto *RefVal2 = cast_or_null<ReferenceValue>(Env1.getValue(FieldLoc2));
- if (RefVal1 && RefVal2) {
- if (&RefVal1->getReferentLoc() != &RefVal2->getReferentLoc())
- return false;
- } else {
- // If either of `RefVal1` and `RefVal2` is null, we only consider them
- // equal if they're both null.
- if (RefVal1 || RefVal2)
- return false;
- }
- } else {
- if (Env1.getValue(*FieldLoc1) != Env2.getValue(FieldLoc2))
+ if (FieldLoc1 != FieldLoc2)
return false;
+ } else if (Env1.getValue(*FieldLoc1) != Env2.getValue(*FieldLoc2)) {
+ return false;
}
}
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index f51b9f34d98639..39faeca4b45ce3 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -460,29 +460,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (BaseLoc == nullptr)
return;
- auto &MemberLoc = BaseLoc->getChild(*Member);
- if (MemberLoc.getType()->isReferenceType()) {
- // Based on its type, `MemberLoc` must be mapped either to nothing or to a
- // `ReferenceValue`. For the former, we won't set a storage location for
- // this expression, so as to maintain an invariant lvalue expressions;
- // namely, that their location maps to a `ReferenceValue`. In this,
- // lvalues are unlike other expressions, where it is valid for their
- // location to map to nothing (because they are not modeled).
- //
- // Note: we need this invariant for lvalues so that, when accessing a
- // value, we can distinguish an rvalue from an lvalue. An alternative
- // design, which takes the expression's value category into account, would
- // avoid the need for this invariant.
- if (auto *V = Env.getValue(MemberLoc)) {
- assert(isa<ReferenceValue>(V) &&
- "reference-typed declarations map to `ReferenceValue`s");
- Env.setStorageLocation(*S, MemberLoc);
- }
- } else {
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Env.create<ReferenceValue>(MemberLoc));
- }
+ auto *MemberLoc = BaseLoc->getChild(*Member);
+ if (MemberLoc == nullptr)
+ return;
+ Env.setStorageLocationStrict(*S, *MemberLoc);
}
void VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *S) {
@@ -510,22 +491,18 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (S->isElidable()) {
Env.setStorageLocation(*S, *ArgLoc);
- } else {
- auto &Loc =
- cast<AggregateStorageLocation>(Env.createStorageLocation(*S));
- Env.setStorageLocation(*S, Loc);
- if (Value *Val = Env.createValue(S->getType())) {
- Env.setValue(Loc, *Val);
- copyRecord(*ArgLoc, Loc, Env);
- }
+ } else if (auto *ArgVal = cast_or_null<StructValue>(
+ Env.getValue(*Arg, SkipPast::Reference))) {
+ auto &Val = *cast<StructValue>(Env.createValue(S->getType()));
+ Env.setValueStrict(*S, Val);
+ copyRecord(ArgVal->getAggregateLoc(), Val.getAggregateLoc(), Env);
}
return;
}
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- if (Value *Val = Env.createValue(S->getType()))
- Env.setValue(Loc, *Val);
+ auto &InitialVal = *cast<StructValue>(Env.createValue(S->getType()));
+ copyRecord(InitialVal.getAggregateLoc(), Env.getResultObjectLocation(*S),
+ Env);
transferInlineCall(S, ConstructorDecl);
}
@@ -549,21 +526,15 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
!Method->isMoveAssignmentOperator())
return;
- auto *ObjectLoc = cast_or_null<AggregateStorageLocation>(
- Env.getStorageLocation(*Arg0, SkipPast::Reference));
- if (ObjectLoc == nullptr)
- return;
+ auto *LocSrc = cast_or_null<AggregateStorageLocation>(
+ Env.getStorageLocationStrict(*Arg1));
+ auto *LocDst = cast_or_null<AggregateStorageLocation>(
+ Env.getStorageLocationStrict(*Arg0));
- auto *ArgLoc = cast_or_null<AggregateStorageLocation>(
- Env.getStorageLocation(*Arg1, SkipPast::Reference));
- if (ArgLoc == nullptr)
- return;
-
- copyRecord(*ArgLoc, *ObjectLoc, Env);
-
- // FIXME: Add a test for the value of the whole expression.
- // Assign a storage location for the whole expression.
- Env.setStorageLocation(*S, *ObjectLoc);
+ if (LocSrc != nullptr && LocDst != nullptr) {
+ copyRecord(*LocSrc, *LocDst, Env);
+ Env.setStorageLocationStrict(*S, *LocDst);
+ }
}
}
@@ -620,9 +591,14 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (SubExprVal == nullptr)
return;
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocationStrict(*S, Loc);
+ if (StructValue *StructVal = dyn_cast<StructValue>(SubExprVal)) {
+ Env.setStorageLocation(*S, StructVal->getAggregateLoc());
+ return;
+ }
+
+ StorageLocation &Loc = Env.createStorageLocation(*S);
Env.setValue(Loc, *SubExprVal);
+ Env.setStorageLocation(*S, Loc);
}
void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
@@ -645,43 +621,43 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// FIXME: Revisit this once flow conditions are added to the framework. For
// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
// condition.
- if (S->isGLValue()) {
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocationStrict(*S, Loc);
- if (Value *Val = Env.createValue(S->getType()))
- Env.setValue(Loc, *Val);
- } else if (Value *Val = Env.createValue(S->getType())) {
+ if (S->isGLValue())
+ Env.setStorageLocationStrict(*S, Env.createObject(S->getType()));
+ else if (Value *Val = Env.createValue(S->getType()))
Env.setValueStrict(*S, *Val);
- }
}
void VisitInitListExpr(const InitListExpr *S) {
QualType Type = S->getType();
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
+ if (!Type->isStructureOrClassType()) {
+ if (auto *Val = Env.createValue(Type))
+ Env.setValueStrict(*S, *Val);
- auto *Val = Env.createValue(Type);
- if (Val == nullptr)
return;
+ }
- Env.setValue(Loc, *Val);
-
- if (Type->isStructureOrClassType()) {
- std::vector<FieldDecl *> Fields =
- getFieldsForInitListExpr(Type->getAsRecordDecl());
- for (auto [Field, Init] : llvm::zip(Fields, S->inits())) {
- assert(Field != nullptr);
- assert(Init != nullptr);
-
- if (Field->getType()->isReferenceType()) {
- if (StorageLocation *Loc = Env.getStorageLocationStrict(*Init))
- cast<StructValue>(Val)->setChild(*Field,
- Env.create<ReferenceValue>(*Loc));
- } else if (Value *InitVal = Env.getValue(*Init, SkipPast::None))
- cast<StructValue>(Val)->setChild(*Field, *InitVal);
- }
+ std::vector<FieldDecl *> Fields =
+ getFieldsForInitListExpr(Type->getAsRecordDecl());
+ llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
+
+ for (auto [Field, Init] : llvm::zip(Fields, S->inits())) {
+ assert(Field != nullptr);
+ assert(Init != nullptr);
+
+ FieldLocs.insert({Field, &Env.createObject(Field->getType(), Init)});
}
+
+ auto &Loc =
+ Env.getDataflowAnalysisContext()
+ .arena()
+ .create<AggregateStorageLocation>(Type, std::move(FieldLocs));
+ StructValue &StructVal = Env.create<StructValue>(Loc);
+
+ Env.setValue(Loc, StructVal);
+
+ Env.setValueStrict(*S, StructVal);
+
// FIXME: Implement array initialization.
}
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 1146a21b05c68c..1b68d76ffc8cd8 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -27,6 +27,7 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
#include "clang/Analysis/FlowSensitive/Transfer.h"
#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/Value.h"
@@ -371,34 +372,53 @@ builtinTransferInitializer(const CFGInitializer &Elt,
// FIXME: Handle base initialization
return;
- auto *InitStmt = Init->getInit();
- assert(InitStmt != nullptr);
+ auto *InitExpr = Init->getInit();
+ assert(InitExpr != nullptr);
const FieldDecl *Member = nullptr;
+ AggregateStorageLocation *ParentLoc = &ThisLoc;
StorageLocation *MemberLoc = nullptr;
if (Init->isMemberInitializer()) {
Member = Init->getMember();
- MemberLoc = &ThisLoc.getChild(*Member);
+ MemberLoc = ThisLoc.getChild(*Member);
} else {
IndirectFieldDecl *IndirectField = Init->getIndirectMember();
assert(IndirectField != nullptr);
MemberLoc = &ThisLoc;
for (const auto *I : IndirectField->chain()) {
Member = cast<FieldDecl>(I);
- MemberLoc = &cast<AggregateStorageLocation>(MemberLoc)->getChild(*Member);
+ ParentLoc = cast<AggregateStorageLocation>(MemberLoc);
+ MemberLoc = ParentLoc->getChild(*Member);
}
}
assert(Member != nullptr);
assert(MemberLoc != nullptr);
+ // FIXME: Instead of these case distinctions, we would ideally want to be able
+ // to simply use `Environment::createObject()` here, the same way that we do
+ // this in `TransferVisitor::VisitInitListExpr()`. However, this would require
+ // us to be able to build a list of fields that we then use to initialize an
+ // `AggregateStorageLocation` -- and the problem is that, when we get here,
+ // the `AggregateStorageLocation` already exists. We should explore if there's
+ // anything that we can do to change this.
if (Member->getType()->isReferenceType()) {
- auto *InitStmtLoc = Env.getStorageLocationStrict(*InitStmt);
- if (InitStmtLoc == nullptr)
+ auto *InitExprLoc = Env.getStorageLocationStrict(*InitExpr);
+ if (InitExprLoc == nullptr)
return;
- Env.setValue(*MemberLoc, Env.create<ReferenceValue>(*InitStmtLoc));
- } else if (auto *InitStmtVal = Env.getValueStrict(*InitStmt)) {
- Env.setValue(*MemberLoc, *InitStmtVal);
+ ParentLoc->setChild(*Member, InitExprLoc);
+ } else if (auto *InitExprVal = Env.getValueStrict(*InitExpr)) {
+ if (Member->getType()->isRecordType()) {
+ auto *InitValStruct = cast<StructValue>(InitExprVal);
+ // FIXME: Rather than performing a copy here, we should really be
+ // initializing the field in place. This would require us to propagate the
+ // storage location of the field to the AST node that creates the
+ // `StructValue`.
+ copyRecord(InitValStruct->getAggregateLoc(),
+ *cast<AggregateStorageLocation>(MemberLoc), Env);
+ } else {
+ Env.setValue(*MemberLoc, *InitExprVal);
+ }
}
}
diff --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index dc81e9594b6914..2b4b64c74481fb 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -60,13 +60,12 @@ TEST(RecordOpsTest, CopyRecord) {
auto &S1 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s1");
auto &S2 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s2");
- auto &Inner1 = cast<AggregateStorageLocation>(S1.getChild(*InnerDecl));
- auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
+ auto &Inner1 = *cast<AggregateStorageLocation>(S1.getChild(*InnerDecl));
+ auto &Inner2 = *cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
EXPECT_NE(getFieldValue(&S1, *OuterIntDecl, Env),
getFieldValue(&S2, *OuterIntDecl, Env));
- EXPECT_NE(Env.getValue(S1.getChild(*RefDecl)),
- Env.getValue(S2.getChild(*RefDecl)));
+ EXPECT_NE(S1.getChild(*RefDecl), S2.getChild(*RefDecl));
EXPECT_NE(getFieldValue(&Inner1, *InnerIntDecl, Env),
getFieldValue(&Inner2, *InnerIntDecl, Env));
@@ -80,8 +79,7 @@ TEST(RecordOpsTest, CopyRecord) {
EXPECT_EQ(getFieldValue(&S1, *OuterIntDecl, Env),
getFieldValue(&S2, *OuterIntDecl, Env));
- EXPECT_EQ(Env.getValue(S1.getChild(*RefDecl)),
- Env.getValue(S2.getChild(*RefDecl)));
+ EXPECT_EQ(S1.getChild(*RefDecl), S2.getChild(*RefDecl));
EXPECT_EQ(getFieldValue(&Inner1, *InnerIntDecl, Env),
getFieldValue(&Inner2, *InnerIntDecl, Env));
@@ -122,7 +120,7 @@ TEST(RecordOpsTest, RecordsEqual) {
auto &S1 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s1");
auto &S2 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s2");
- auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
+ auto &Inner2 = *cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
cast<StructValue>(Env.getValue(S1))
->setProperty("prop", Env.getBoolLiteralValue(true));
@@ -139,33 +137,26 @@ TEST(RecordOpsTest, RecordsEqual) {
EXPECT_TRUE(recordsEqual(S1, S2, Env));
// S2 has a
diff erent outer_int.
- Env.setValue(S2.getChild(*OuterIntDecl), Env.create<IntegerValue>());
+ Env.setValue(*S2.getChild(*OuterIntDecl), Env.create<IntegerValue>());
EXPECT_FALSE(recordsEqual(S1, S2, Env));
copyRecord(S1, S2, Env);
EXPECT_TRUE(recordsEqual(S1, S2, Env));
// S2 doesn't have outer_int at all.
- Env.clearValue(S2.getChild(*OuterIntDecl));
+ Env.clearValue(*S2.getChild(*OuterIntDecl));
EXPECT_FALSE(recordsEqual(S1, S2, Env));
copyRecord(S1, S2, Env);
EXPECT_TRUE(recordsEqual(S1, S2, Env));
// S2 has a
diff erent ref.
- Env.setValue(S2.getChild(*RefDecl),
- Env.create<ReferenceValue>(Env.createStorageLocation(
- RefDecl->getType().getNonReferenceType())));
- EXPECT_FALSE(recordsEqual(S1, S2, Env));
- copyRecord(S1, S2, Env);
- EXPECT_TRUE(recordsEqual(S1, S2, Env));
-
- // S2 doesn't have ref at all.
- Env.clearValue(S2.getChild(*RefDecl));
+ S2.setChild(*RefDecl, &Env.createStorageLocation(
+ RefDecl->getType().getNonReferenceType()));
EXPECT_FALSE(recordsEqual(S1, S2, Env));
copyRecord(S1, S2, Env);
EXPECT_TRUE(recordsEqual(S1, S2, Env));
// S2 as a
diff erent inner_int.
- Env.setValue(Inner2.getChild(*InnerIntDecl),
+ Env.setValue(*Inner2.getChild(*InnerIntDecl),
Env.create<IntegerValue>());
EXPECT_FALSE(recordsEqual(S1, S2, Env));
copyRecord(S1, S2, Env);
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index aa1a0f0e773a9a..e6f0b04c43c71f 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -457,7 +457,10 @@ inline Value *getFieldValue(const AggregateStorageLocation *Loc,
const ValueDecl &Field, const Environment &Env) {
if (Loc == nullptr)
return nullptr;
- return Env.getValue(Loc->getChild(Field));
+ StorageLocation *FieldLoc = Loc->getChild(Field);
+ if (FieldLoc == nullptr)
+ return nullptr;
+ return Env.getValue(*FieldLoc);
}
/// Returns the value of a `Field` on a `Struct.
@@ -470,7 +473,10 @@ inline Value *getFieldValue(const StructValue *Struct, const ValueDecl &Field,
const Environment &Env) {
if (Struct == nullptr)
return nullptr;
- return Struct->getChild(Field);
+ StorageLocation *FieldLoc = Struct->getChild(Field);
+ if (FieldLoc == nullptr)
+ return nullptr;
+ return Env.getValue(*FieldLoc);
}
/// Creates and owns constraints which are boolean values.
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 291f8329a60fde..78f13a044ce64b 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -27,6 +27,11 @@
#include <string>
#include <utility>
+// FIXME: There are still remaining checks here that check for consistency
+// between `StructValue` and `AggregateStorageLocation`. Now that the redundancy
+// between these two classes has been eliminated, these checks aren't needed any
+// more, so remove them.
+
namespace {
using namespace clang;
@@ -186,7 +191,7 @@ TEST(TransferTest, StructFieldUnmodeled) {
void target() {
D Bar;
- A Foo = Bar.F1.F2.F3;
+ A &Foo = Bar.F1.F2.F3;
int Zab = Foo.Unmodeled.X;
// [[p]]
}
@@ -200,8 +205,9 @@ TEST(TransferTest, StructFieldUnmodeled) {
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
ASSERT_THAT(FooDecl, NotNull());
- ASSERT_TRUE(FooDecl->getType()->isStructureType());
- auto FooFields = FooDecl->getType()->getAsRecordDecl()->fields();
+ QualType FooReferentType = FooDecl->getType()->getPointeeType();
+ ASSERT_TRUE(FooReferentType->isStructureType());
+ auto FooFields = FooReferentType->getAsRecordDecl()->fields();
FieldDecl *UnmodeledDecl = nullptr;
for (FieldDecl *Field : FooFields) {
@@ -215,9 +221,9 @@ TEST(TransferTest, StructFieldUnmodeled) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto *UnmodeledLoc = &FooLoc->getChild(*UnmodeledDecl);
- ASSERT_TRUE(isa<ScalarStorageLocation>(UnmodeledLoc));
- ASSERT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
+ const auto *UnmodeledLoc = FooLoc->getChild(*UnmodeledDecl);
+ ASSERT_TRUE(isa<AggregateStorageLocation>(UnmodeledLoc));
+ EXPECT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
const ValueDecl *ZabDecl = findValueDecl(ASTCtx, "Zab");
ASSERT_THAT(ZabDecl, NotNull());
@@ -263,7 +269,7 @@ TEST(TransferTest, StructVarDecl) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
- cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
@@ -312,7 +318,7 @@ TEST(TransferTest, StructVarDeclWithInit) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
- cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
@@ -360,7 +366,7 @@ TEST(TransferTest, ClassVarDecl) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
- cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
@@ -479,34 +485,26 @@ TEST(TransferTest, SelfReferentialReferenceVarDecl) {
const auto &FooLoc =
*cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto &FooReferentVal = *cast<StructValue>(Env.getValue(FooLoc));
- const auto &BarVal =
- *cast<ReferenceValue>(FooReferentVal.getChild(*BarDecl));
- const auto &BarReferentVal =
- *cast<StructValue>(Env.getValue(BarVal.getReferentLoc()));
+ const auto &BarLoc =
+ *cast<AggregateStorageLocation>(FooLoc.getChild(*BarDecl));
- const auto &FooRefVal =
- *cast<ReferenceValue>(getFieldValue(&BarReferentVal, *FooRefDecl, Env));
const auto &FooReferentLoc =
- cast<AggregateStorageLocation>(FooRefVal.getReferentLoc());
+ *cast<AggregateStorageLocation>(BarLoc.getChild(*FooRefDecl));
EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
EXPECT_THAT(getFieldValue(&FooReferentLoc, *BarDecl, Env), IsNull());
const auto &FooPtrVal =
- *cast<PointerValue>(getFieldValue(&BarReferentVal, *FooPtrDecl, Env));
+ *cast<PointerValue>(getFieldValue(&BarLoc, *FooPtrDecl, Env));
const auto &FooPtrPointeeLoc =
cast<AggregateStorageLocation>(FooPtrVal.getPointeeLoc());
EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
EXPECT_THAT(getFieldValue(&FooPtrPointeeLoc, *BarDecl, Env), IsNull());
- const auto &BazRefVal =
- *cast<ReferenceValue>(getFieldValue(&BarReferentVal, *BazRefDecl, Env));
- const StorageLocation &BazReferentLoc = BazRefVal.getReferentLoc();
- EXPECT_THAT(Env.getValue(BazReferentLoc), NotNull());
+ EXPECT_THAT(getFieldValue(&BarLoc, *BazRefDecl, Env), NotNull());
const auto &BazPtrVal =
- *cast<PointerValue>(getFieldValue(&BarReferentVal, *BazPtrDecl, Env));
+ *cast<PointerValue>(getFieldValue(&BarLoc, *BazPtrDecl, Env));
const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
});
@@ -648,10 +646,7 @@ TEST(TransferTest, SelfReferentialPointerVarDecl) {
const auto &BarPointeeVal =
*cast<StructValue>(Env.getValue(BarVal.getPointeeLoc()));
- const auto &FooRefVal = *cast<ReferenceValue>(
- getFieldValue(&BarPointeeVal, *FooRefDecl, Env));
- const StorageLocation &FooReferentLoc = FooRefVal.getReferentLoc();
- EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
+ EXPECT_THAT(getFieldValue(&BarPointeeVal, *FooRefDecl, Env), NotNull());
const auto &FooPtrVal = *cast<PointerValue>(
getFieldValue(&BarPointeeVal, *FooPtrDecl, Env));
@@ -659,10 +654,7 @@ TEST(TransferTest, SelfReferentialPointerVarDecl) {
cast<AggregateStorageLocation>(FooPtrVal.getPointeeLoc());
EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
- const auto &BazRefVal = *cast<ReferenceValue>(
- getFieldValue(&BarPointeeVal, *BazRefDecl, Env));
- const StorageLocation &BazReferentLoc = BazRefVal.getReferentLoc();
- EXPECT_THAT(Env.getValue(BazReferentLoc), NotNull());
+ EXPECT_THAT(getFieldValue(&BarPointeeVal, *BazRefDecl, Env), NotNull());
const auto &BazPtrVal = *cast<PointerValue>(
getFieldValue(&BarPointeeVal, *BazPtrDecl, Env));
@@ -689,9 +681,7 @@ TEST(TransferTest, DirectlySelfReferentialReference) {
const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
auto *ThisLoc = Env.getThisPointeeStorageLocation();
- auto *RefVal =
- cast<ReferenceValue>(Env.getValue(ThisLoc->getChild(*SelfDecl)));
- ASSERT_EQ(&RefVal->getReferentLoc(), ThisLoc);
+ ASSERT_EQ(ThisLoc->getChild(*SelfDecl), ThisLoc);
});
}
@@ -1057,7 +1047,7 @@ TEST(TransferTest, StructParamDecl) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
- cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
@@ -1290,36 +1280,8 @@ TEST(TransferTest, DerivedBaseMemberClass) {
ASSERT_THAT(APrivateDecl, NotNull());
ASSERT_THAT(APublicDecl, NotNull());
- const auto &FooLoc =
- *cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto &FooVal = *cast<StructValue>(Env.getValue(FooLoc));
-
- // Note: we can't test presence of children in `FooLoc`, because
- // `getChild` requires its argument be present (or fails an assert). So,
- // we limit to testing presence in `FooVal` and coherence between the
- // two.
-
- // Base-class fields.
- EXPECT_THAT(FooVal.getChild(*ADefaultDecl), NotNull());
- EXPECT_THAT(FooVal.getChild(*APrivateDecl), NotNull());
-
- EXPECT_THAT(FooVal.getChild(*AProtectedDecl), NotNull());
- EXPECT_EQ(Env.getValue(FooLoc.getChild(*APublicDecl)),
- FooVal.getChild(*APublicDecl));
- EXPECT_THAT(FooVal.getChild(*APublicDecl), NotNull());
- EXPECT_EQ(Env.getValue(FooLoc.getChild(*AProtectedDecl)),
- FooVal.getChild(*AProtectedDecl));
-
- // Derived-class fields.
- EXPECT_THAT(FooVal.getChild(*BDefaultDecl), NotNull());
- EXPECT_EQ(Env.getValue(FooLoc.getChild(*BDefaultDecl)),
- FooVal.getChild(*BDefaultDecl));
- EXPECT_THAT(FooVal.getChild(*BProtectedDecl), NotNull());
- EXPECT_EQ(Env.getValue(FooLoc.getChild(*BProtectedDecl)),
- FooVal.getChild(*BProtectedDecl));
- EXPECT_THAT(FooVal.getChild(*BPrivateDecl), NotNull());
- EXPECT_EQ(Env.getValue(FooLoc.getChild(*BPrivateDecl)),
- FooVal.getChild(*BPrivateDecl));
+ ASSERT_TRUE(
+ isa<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl)));
});
}
@@ -1352,8 +1314,7 @@ static void derivedBaseMemberExpectations(
const auto &FooLoc =
*cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto &FooVal = *cast<StructValue>(Env.getValue(FooLoc));
- EXPECT_THAT(FooVal.getChild(*BarDecl), NotNull());
- EXPECT_EQ(Env.getValue(FooLoc.getChild(*BarDecl)), FooVal.getChild(*BarDecl));
+ EXPECT_EQ(&FooVal.getAggregateLoc(), &FooLoc);
}
TEST(TransferTest, DerivedBaseMemberStructDefault) {
@@ -1522,10 +1483,8 @@ TEST(TransferTest, ReferenceMember) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
- const auto *BarVal = cast<ReferenceValue>(FooVal->getChild(*BarDecl));
const auto *BarReferentVal =
- cast<IntegerValue>(Env.getValue(BarVal->getReferentLoc()));
+ cast<IntegerValue>(getFieldValue(FooLoc, *BarDecl, Env));
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
@@ -1566,7 +1525,7 @@ TEST(TransferTest, StructThisMember) {
ASSERT_THAT(BarDecl, NotNull());
const auto *BarLoc =
- cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(ThisLoc->getChild(*BarDecl));
ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
const Value *BarVal = Env.getValue(*BarLoc);
@@ -1593,12 +1552,12 @@ TEST(TransferTest, StructThisMember) {
ASSERT_THAT(BazDecl, NotNull());
const auto *QuxLoc =
- cast<AggregateStorageLocation>(&ThisLoc->getChild(*QuxDecl));
+ cast<AggregateStorageLocation>(ThisLoc->getChild(*QuxDecl));
const auto *QuxVal = dyn_cast<StructValue>(Env.getValue(*QuxLoc));
ASSERT_THAT(QuxVal, NotNull());
const auto *BazLoc =
- cast<ScalarStorageLocation>(&QuxLoc->getChild(*BazDecl));
+ cast<ScalarStorageLocation>(QuxLoc->getChild(*BazDecl));
const auto *BazVal =
cast<IntegerValue>(getFieldValue(QuxVal, *BazDecl, Env));
EXPECT_EQ(Env.getValue(*BazLoc), BazVal);
@@ -1641,7 +1600,7 @@ TEST(TransferTest, ClassThisMember) {
ASSERT_THAT(BarDecl, NotNull());
const auto *BarLoc =
- cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(ThisLoc->getChild(*BarDecl));
ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
const Value *BarVal = Env.getValue(*BarLoc);
@@ -1668,12 +1627,12 @@ TEST(TransferTest, ClassThisMember) {
ASSERT_THAT(BazDecl, NotNull());
const auto *QuxLoc =
- cast<AggregateStorageLocation>(&ThisLoc->getChild(*QuxDecl));
+ cast<AggregateStorageLocation>(ThisLoc->getChild(*QuxDecl));
const auto *QuxVal = dyn_cast<StructValue>(Env.getValue(*QuxLoc));
ASSERT_THAT(QuxVal, NotNull());
const auto *BazLoc =
- cast<ScalarStorageLocation>(&QuxLoc->getChild(*BazDecl));
+ cast<ScalarStorageLocation>(QuxLoc->getChild(*BazDecl));
const auto *BazVal =
cast<IntegerValue>(getFieldValue(QuxVal, *BazDecl, Env));
EXPECT_EQ(Env.getValue(*BazLoc), BazVal);
@@ -1713,7 +1672,7 @@ TEST(TransferTest, UnionThisMember) {
ASSERT_THAT(FooDecl, NotNull());
const auto *FooLoc =
- cast<ScalarStorageLocation>(&ThisLoc->getChild(*FooDecl));
+ cast<ScalarStorageLocation>(ThisLoc->getChild(*FooDecl));
ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
const Value *FooVal = Env.getValue(*FooLoc);
@@ -1723,7 +1682,7 @@ TEST(TransferTest, UnionThisMember) {
ASSERT_THAT(BarDecl, NotNull());
const auto *BarLoc =
- cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(ThisLoc->getChild(*BarDecl));
ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
const Value *BarVal = Env.getValue(*BarLoc);
@@ -1758,7 +1717,7 @@ TEST(TransferTest, StructThisInLambda) {
ASSERT_THAT(BarDecl, NotNull());
const auto *BarLoc =
- cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(ThisLoc->getChild(*BarDecl));
ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
const Value *BarVal = Env.getValue(*BarLoc);
@@ -1796,7 +1755,7 @@ TEST(TransferTest, StructThisInLambda) {
ASSERT_THAT(BarDecl, NotNull());
const auto *BarLoc =
- cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(ThisLoc->getChild(*BarDecl));
ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
const Value *BarVal = Env.getValue(*BarLoc);
@@ -1957,7 +1916,7 @@ TEST(TransferTest, TemporaryObject) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
- cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
@@ -1996,7 +1955,7 @@ TEST(TransferTest, ElidableConstructor) {
const auto *FooLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
- cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
+ cast<ScalarStorageLocation>(FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
@@ -2341,12 +2300,12 @@ TEST(TransferTest, MoveConstructor) {
const auto *BarLoc1 =
cast<AggregateStorageLocation>(Env1.getStorageLocation(*BarDecl));
+ EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
+
const auto *FooVal1 = cast<StructValue>(Env1.getValue(*FooLoc1));
const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1));
EXPECT_NE(FooVal1, BarVal1);
- EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
-
const auto *FooBazVal1 =
cast<IntegerValue>(getFieldValue(FooLoc1, *BazDecl, Env1));
const auto *BarBazVal1 =
@@ -2938,9 +2897,7 @@ TEST(TransferTest, AggregateInitializationReferenceField) {
auto &ILoc = getLocForDecl<StorageLocation>(ASTCtx, Env, "i");
auto &SLoc = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s");
- auto &RefValue =
- *cast<ReferenceValue>(getFieldValue(&SLoc, *RefFieldDecl, Env));
- EXPECT_EQ(&RefValue.getReferentLoc(), &ILoc);
+ EXPECT_EQ(SLoc.getChild(*RefFieldDecl), &ILoc);
});
}
@@ -5512,8 +5469,8 @@ TEST(TransferTest, NewExpressions_Structs) {
auto &OuterLoc = cast<AggregateStorageLocation>(P.getPointeeLoc());
auto &OuterFieldLoc =
- cast<AggregateStorageLocation>(OuterLoc.getChild(*OuterField));
- auto &InnerFieldLoc = OuterFieldLoc.getChild(*InnerField);
+ *cast<AggregateStorageLocation>(OuterLoc.getChild(*OuterField));
+ auto &InnerFieldLoc = *OuterFieldLoc.getChild(*InnerField);
// Values for the struct and all fields exist after the new.
EXPECT_THAT(Env.getValue(OuterLoc), NotNull());
@@ -5620,7 +5577,7 @@ TEST(TransferTest, AnonymousStruct) {
auto *S =
cast<AggregateStorageLocation>(Env.getStorageLocation(*SDecl));
- auto &AnonStruct = cast<AggregateStorageLocation>(
+ auto &AnonStruct = *cast<AggregateStorageLocation>(
S->getChild(*cast<ValueDecl>(IndirectField->chain().front())));
auto *B = cast<BoolValue>(getFieldValue(&AnonStruct, *BDecl, Env));
@@ -5651,7 +5608,7 @@ TEST(TransferTest, AnonymousStructWithInitializer) {
auto *ThisLoc =
cast<AggregateStorageLocation>(Env.getThisPointeeStorageLocation());
- auto &AnonStruct = cast<AggregateStorageLocation>(ThisLoc->getChild(
+ auto &AnonStruct = *cast<AggregateStorageLocation>(ThisLoc->getChild(
*cast<ValueDecl>(IndirectField->chain().front())));
auto *B = cast<BoolValue>(getFieldValue(&AnonStruct, *BDecl, Env));
@@ -5684,13 +5641,10 @@ TEST(TransferTest, AnonymousStructWithReferenceField) {
auto *ThisLoc =
cast<AggregateStorageLocation>(Env.getThisPointeeStorageLocation());
- auto &AnonStruct = cast<AggregateStorageLocation>(ThisLoc->getChild(
+ auto &AnonStruct = *cast<AggregateStorageLocation>(ThisLoc->getChild(
*cast<ValueDecl>(IndirectField->chain().front())));
- auto *RefVal =
- cast<ReferenceValue>(Env.getValue(AnonStruct.getChild(*IDecl)));
-
- ASSERT_EQ(&RefVal->getReferentLoc(),
+ ASSERT_EQ(AnonStruct.getChild(*IDecl),
Env.getStorageLocation(*GlobalIDecl));
});
}
More information about the cfe-commits
mailing list