[clang] [clang][nullability] Remove `RecordValue`. (PR #89052)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 19 00:39:21 PDT 2024
https://github.com/martinboehme updated https://github.com/llvm/llvm-project/pull/89052
>From a705853deb0cfa6596245c929e9b86ea2d2a7c26 Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboehme at google.com>
Date: Wed, 17 Apr 2024 11:31:12 +0000
Subject: [PATCH] [clang][nullability] Remove `RecordValue`.
This class no longer serves any purpose; see also the discussion here:
https://reviews.llvm.org/D155204#inline-1503204
A lot of existing tests in TransferTest.cpp check for the existence of
`RecordValue`s. Some of these checks are now simply redundant and have been
removed. In other cases, tests were checking for the existence of a
`RecordValue` as a way of testing whether a record has been initialized. I have
typically changed these test to instead check whether a field of the record has
a value.
---
.../FlowSensitive/DataflowEnvironment.h | 38 +++---
.../clang/Analysis/FlowSensitive/Value.h | 41 ------
.../FlowSensitive/DataflowEnvironment.cpp | 98 ++++----------
.../Analysis/FlowSensitive/DebugSupport.cpp | 2 -
.../lib/Analysis/FlowSensitive/HTMLLogger.cpp | 13 +-
.../Models/UncheckedOptionalAccessModel.cpp | 46 +++----
.../lib/Analysis/FlowSensitive/RecordOps.cpp | 3 -
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 62 +++------
.../TypeErasedDataflowAnalysis.cpp | 6 +-
clang/lib/Analysis/FlowSensitive/Value.cpp | 2 -
.../FlowSensitive/DataflowEnvironmentTest.cpp | 81 -----------
.../Analysis/FlowSensitive/RecordOpsTest.cpp | 8 --
.../Analysis/FlowSensitive/TransferTest.cpp | 126 ++++++------------
13 files changed, 130 insertions(+), 396 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 4277792219c0af..d50dba35f8264c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -84,7 +84,7 @@ class Environment {
virtual ComparisonResult compare(QualType Type, const Value &Val1,
const Environment &Env1, const Value &Val2,
const Environment &Env2) {
- // FIXME: Consider adding QualType to RecordValue and removing the Type
+ // FIXME: Consider adding `QualType` to `Value` and removing the `Type`
// argument here.
return ComparisonResult::Unknown;
}
@@ -407,20 +407,15 @@ class Environment {
/// storage locations and values for indirections until it finds a
/// non-pointer/non-reference type.
///
- /// If `Type` is a class, struct, or union type, creates values for all
- /// modeled fields (including synthetic fields) and calls `setValue()` to
- /// associate the `RecordValue` with its storage location
- /// (`RecordValue::getLoc()`).
- ///
/// If `Type` is one of the following types, this function will always return
/// a non-null pointer:
/// - `bool`
/// - Any integer type
- /// - Any class, struct, or union type
///
/// Requirements:
///
- /// `Type` must not be null.
+ /// - `Type` must not be null.
+ /// - `Type` must not be a reference type or record type.
Value *createValue(QualType Type);
/// Creates an object (i.e. a storage location with an associated value) of
@@ -452,6 +447,7 @@ class Environment {
/// Initializes the fields (including synthetic fields) of `Loc` with values,
/// unless values of the field type are not supported or we hit one of the
/// limits at which we stop producing values.
+ /// If a field already has a value, that value is preserved.
/// If `Type` is provided, initializes only those fields that are modeled for
/// `Type`; this is intended for use in cases where `Loc` is a derived type
/// and we only want to initialize the fields of a base type.
@@ -461,6 +457,10 @@ class Environment {
}
/// Assigns `Val` as the value of `Loc` in the environment.
+ ///
+ /// Requirements:
+ ///
+ /// `Loc` must not be a `RecordStorageLocation`.
void setValue(const StorageLocation &Loc, Value &Val);
/// Clears any association between `Loc` and a value in the environment.
@@ -470,20 +470,24 @@ class Environment {
///
/// Requirements:
///
- /// - `E` must be a prvalue
- /// - If `Val` is a `RecordValue`, its `RecordStorageLocation` must be
- /// `getResultObjectLocation(E)`. An exception to this is if `E` is an
- /// expression that originally creates a `RecordValue` (such as a
- /// `CXXConstructExpr` or `CallExpr`), as these establish the location of
- /// the result object in the first place.
+ /// - `E` must be a prvalue.
+ /// - `E` must not have record type.
void setValue(const Expr &E, Value &Val);
/// Returns the value assigned to `Loc` in the environment or null if `Loc`
/// isn't assigned a value in the environment.
+ ///
+ /// Requirements:
+ ///
+ /// `Loc` must not be a `RecordStorageLocation`.
Value *getValue(const StorageLocation &Loc) const;
/// Equivalent to `getValue(getStorageLocation(D))` if `D` is assigned a
/// storage location in the environment, otherwise returns null.
+ ///
+ /// Requirements:
+ ///
+ /// `D` must not have record type.
Value *getValue(const ValueDecl &D) const;
/// Equivalent to `getValue(getStorageLocation(E, SP))` if `E` is assigned a
@@ -775,12 +779,6 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
const Environment &Env);
-/// Associates a new `RecordValue` with `Loc` and returns the new value.
-RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env);
-
-/// Associates a new `RecordValue` with `Expr` and returns the new value.
-RecordValue &refreshRecordValue(const Expr &Expr, Environment &Env);
-
} // namespace dataflow
} // namespace clang
diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h
index be1bf9324c87b4..97efa3a93ce6d9 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -35,7 +35,6 @@ class Value {
enum class Kind {
Integer,
Pointer,
- Record,
// TODO: Top values should not be need to be type-specific.
TopBool,
@@ -67,7 +66,6 @@ class Value {
/// Properties may not be set on `RecordValue`s; use synthetic fields instead
/// (for details, see documentation for `RecordStorageLocation`).
void setProperty(llvm::StringRef Name, Value &Val) {
- assert(getKind() != Kind::Record);
Properties.insert_or_assign(Name, &Val);
}
@@ -184,45 +182,6 @@ class PointerValue final : public Value {
StorageLocation &PointeeLoc;
};
-/// 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, `RecordValue` also serves only a limited purpose: 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. `RecordValue` 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
-/// `RecordValue` that wraps a `RecordStorageLocation`. This
-/// `RecordStorageLocation` is then used as the storage location for `S`.
-///
-/// Over time, we may eliminate `RecordValue` entirely. See also the discussion
-/// here: https://reviews.llvm.org/D155204#inline-1503204
-class RecordValue final : public Value {
-public:
- explicit RecordValue(RecordStorageLocation &Loc)
- : Value(Kind::Record), Loc(Loc) {}
-
- static bool classof(const Value *Val) {
- return Val->getKind() == Kind::Record;
- }
-
- /// Returns the storage location that this `RecordValue` is associated with.
- RecordStorageLocation &getLoc() const { return Loc; }
-
-private:
- RecordStorageLocation &Loc;
-};
-
raw_ostream &operator<<(raw_ostream &OS, const Value &Val);
} // namespace dataflow
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 138773460dd749..6998e6d7a5170b 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -24,6 +24,7 @@
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/ErrorHandling.h"
#include <cassert>
#include <utility>
@@ -80,7 +81,6 @@ static bool equateUnknownValues(Value::Kind K) {
switch (K) {
case Value::Kind::Integer:
case Value::Kind::Pointer:
- case Value::Kind::Record:
return true;
default:
return false;
@@ -145,25 +145,7 @@ static Value *joinDistinctValues(QualType Type, Value &Val1,
return &A.makeBoolValue(JoinedVal);
}
- Value *JoinedVal = nullptr;
- if (auto *RecordVal1 = dyn_cast<RecordValue>(&Val1)) {
- auto *RecordVal2 = cast<RecordValue>(&Val2);
-
- if (&RecordVal1->getLoc() == &RecordVal2->getLoc())
- // `RecordVal1` and `RecordVal2` may have different properties associated
- // with them. Create a new `RecordValue` with the same location but
- // without any properties so that we soundly approximate both values. If a
- // particular analysis needs to join properties, it should do so in
- // `DataflowAnalysis::join()`.
- JoinedVal = &JoinedEnv.create<RecordValue>(RecordVal1->getLoc());
- else
- // If the locations for the two records are different, need to create a
- // completely new value.
- JoinedVal = JoinedEnv.createValue(Type);
- } else {
- JoinedVal = JoinedEnv.createValue(Type);
- }
-
+ Value *JoinedVal = JoinedEnv.createValue(Type);
if (JoinedVal)
Model.join(Type, Val1, Env1, Val2, Env2, *JoinedVal, JoinedEnv);
@@ -565,7 +547,6 @@ void Environment::initialize() {
auto &ThisLoc =
cast<RecordStorageLocation>(createStorageLocation(ThisPointeeType));
setThisPointeeStorageLocation(ThisLoc);
- refreshRecordValue(ThisLoc, *this);
// Initialize fields of `*this` with values, but only if we're not
// analyzing a constructor; after all, it's the constructor's job to do
// this (and we want to be able to test that).
@@ -709,10 +690,6 @@ void Environment::popCall(const CXXConstructExpr *Call,
// See also comment in `popCall(const CallExpr *, const Environment &)` above.
this->LocToVal = std::move(CalleeEnv.LocToVal);
this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
-
- if (Value *Val = CalleeEnv.getValue(*CalleeEnv.ThisPointeeLoc)) {
- setValue(*Call, *Val);
- }
}
bool Environment::equivalentTo(const Environment &Other,
@@ -936,24 +913,23 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
}
void Environment::setValue(const StorageLocation &Loc, Value &Val) {
- assert(!isa<RecordValue>(&Val) || &cast<RecordValue>(&Val)->getLoc() == &Loc);
-
+ // Records should not be associated with values.
+ assert(!isa<RecordStorageLocation>(Loc));
LocToVal[&Loc] = &Val;
}
void Environment::setValue(const Expr &E, Value &Val) {
const Expr &CanonE = ignoreCFGOmittedNodes(E);
- if (auto *RecordVal = dyn_cast<RecordValue>(&Val)) {
- assert(&RecordVal->getLoc() == &getResultObjectLocation(CanonE));
- (void)RecordVal;
- }
-
assert(CanonE.isPRValue());
+ // Records should not be associated with values.
+ assert(!CanonE.getType()->isRecordType());
ExprToVal[&CanonE] = &Val;
}
Value *Environment::getValue(const StorageLocation &Loc) const {
+ // Records should not be associated with values.
+ assert(!isa<RecordStorageLocation>(Loc));
return LocToVal.lookup(&Loc);
}
@@ -965,6 +941,9 @@ Value *Environment::getValue(const ValueDecl &D) const {
}
Value *Environment::getValue(const Expr &E) const {
+ // Records should not be associated with values.
+ assert(!E.getType()->isRecordType());
+
if (E.isPRValue()) {
auto It = ExprToVal.find(&ignoreCFGOmittedNodes(E));
return It == ExprToVal.end() ? nullptr : It->second;
@@ -993,6 +972,7 @@ Value *Environment::createValueUnlessSelfReferential(
int &CreatedValuesCount) {
assert(!Type.isNull());
assert(!Type->isReferenceType());
+ assert(!Type->isRecordType());
// Allow unlimited fields at depth 1; only cap at deeper nesting levels.
if ((Depth > 1 && CreatedValuesCount > MaxCompositeValueSize) ||
@@ -1021,15 +1001,6 @@ Value *Environment::createValueUnlessSelfReferential(
return &arena().create<PointerValue>(PointeeLoc);
}
- if (Type->isRecordType()) {
- CreatedValuesCount++;
- auto &Loc = cast<RecordStorageLocation>(createStorageLocation(Type));
- initializeFieldsWithValues(Loc, Loc.getType(), Visited, Depth,
- CreatedValuesCount);
-
- return &refreshRecordValue(Loc, *this);
- }
-
return nullptr;
}
@@ -1039,20 +1010,23 @@ Environment::createLocAndMaybeValue(QualType Ty,
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());
+ auto EraseVisited = llvm::make_scope_exit(
+ [&Visited, Ty] { Visited.erase(Ty.getCanonicalType()); });
Ty = Ty.getNonReferenceType();
- if (Val == nullptr)
- return createStorageLocation(Ty);
-
- if (Ty->isRecordType())
- return cast<RecordValue>(Val)->getLoc();
+ if (Ty->isRecordType()) {
+ auto &Loc = cast<RecordStorageLocation>(createStorageLocation(Ty));
+ initializeFieldsWithValues(Loc, Ty, Visited, Depth, CreatedValuesCount);
+ return Loc;
+ }
StorageLocation &Loc = createStorageLocation(Ty);
- setValue(Loc, *Val);
+
+ if (Value *Val = createValueUnlessSelfReferential(Ty, Visited, Depth,
+ CreatedValuesCount))
+ setValue(Loc, *Val);
+
return Loc;
}
@@ -1064,10 +1038,11 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
auto initField = [&](QualType FieldType, StorageLocation &FieldLoc) {
if (FieldType->isRecordType()) {
auto &FieldRecordLoc = cast<RecordStorageLocation>(FieldLoc);
- setValue(FieldRecordLoc, create<RecordValue>(FieldRecordLoc));
initializeFieldsWithValues(FieldRecordLoc, FieldRecordLoc.getType(),
Visited, Depth + 1, CreatedValuesCount);
} else {
+ if (getValue(FieldLoc) != nullptr)
+ return;
if (!Visited.insert(FieldType.getCanonicalType()).second)
return;
if (Value *Val = createValueUnlessSelfReferential(
@@ -1125,7 +1100,6 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D,
auto &RecordLoc = cast<RecordStorageLocation>(Loc);
if (!InitExpr)
initializeFieldsWithValues(RecordLoc);
- refreshRecordValue(RecordLoc, *this);
} else {
Value *Val = nullptr;
if (InitExpr)
@@ -1264,25 +1238,5 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
return Env.get<RecordStorageLocation>(*Base);
}
-RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env) {
- auto &NewVal = Env.create<RecordValue>(Loc);
- Env.setValue(Loc, NewVal);
- return NewVal;
-}
-
-RecordValue &refreshRecordValue(const Expr &Expr, Environment &Env) {
- assert(Expr.getType()->isRecordType());
-
- if (Expr.isPRValue())
- refreshRecordValue(Env.getResultObjectLocation(Expr), Env);
-
- if (auto *Loc = Env.get<RecordStorageLocation>(Expr))
- refreshRecordValue(*Loc, Env);
-
- auto &NewVal = *cast<RecordValue>(Env.createValue(Expr.getType()));
- Env.setStorageLocation(Expr, NewVal.getLoc());
- return NewVal;
-}
-
} // namespace dataflow
} // namespace clang
diff --git a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
index 573c4b1d474bf4..d40aab7a7f1035 100644
--- a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -28,8 +28,6 @@ llvm::StringRef debugString(Value::Kind Kind) {
return "Integer";
case Value::Kind::Pointer:
return "Pointer";
- case Value::Kind::Record:
- return "Record";
case Value::Kind::AtomicBool:
return "AtomicBool";
case Value::Kind::TopBool:
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index 397a8d87e114d7..a36cb41a63dfb1 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -95,7 +95,6 @@ class ModelDumper {
switch (V.getKind()) {
case Value::Kind::Integer:
- case Value::Kind::Record:
case Value::Kind::TopBool:
case Value::Kind::AtomicBool:
case Value::Kind::FormulaBool:
@@ -126,8 +125,9 @@ class ModelDumper {
return;
JOS.attribute("type", L.getType().getAsString());
- if (auto *V = Env.getValue(L))
- dump(*V);
+ if (!L.getType()->isRecordType())
+ if (auto *V = Env.getValue(L))
+ dump(*V);
if (auto *RLoc = dyn_cast<RecordStorageLocation>(&L)) {
for (const auto &Child : RLoc->children())
@@ -281,9 +281,10 @@ class HTMLLogger : public Logger {
Iters.back().Block->Elements[ElementIndex - 1].getAs<CFGStmt>();
if (const Expr *E = S ? llvm::dyn_cast<Expr>(S->getStmt()) : nullptr) {
if (E->isPRValue()) {
- if (auto *V = State.Env.getValue(*E))
- JOS.attributeObject(
- "value", [&] { ModelDumper(JOS, State.Env).dump(*V); });
+ if (!E->getType()->isRecordType())
+ if (auto *V = State.Env.getValue(*E))
+ JOS.attributeObject(
+ "value", [&] { ModelDumper(JOS, State.Env).dump(*V); });
} else {
if (auto *Loc = State.Env.getStorageLocation(*E))
JOS.attributeObject(
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index cadb1ceb2d8507..0707aa662e4cc2 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -339,17 +339,6 @@ void setHasValue(RecordStorageLocation &OptionalLoc, BoolValue &HasValueVal,
Env.setValue(locForHasValue(OptionalLoc), HasValueVal);
}
-/// Creates a symbolic value for an `optional` value at an existing storage
-/// location. Uses `HasValueVal` as the symbolic value of the "has_value"
-/// property.
-RecordValue &createOptionalValue(RecordStorageLocation &Loc,
- BoolValue &HasValueVal, Environment &Env) {
- auto &OptionalVal = Env.create<RecordValue>(Loc);
- Env.setValue(Loc, OptionalVal);
- setHasValue(Loc, HasValueVal, Env);
- return OptionalVal;
-}
-
/// Returns the symbolic value that represents the "has_value" property of the
/// optional at `OptionalLoc`. Returns null if `OptionalLoc` is null.
BoolValue *getHasValue(Environment &Env, RecordStorageLocation *OptionalLoc) {
@@ -413,9 +402,8 @@ void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
void transferMakeOptionalCall(const CallExpr *E,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
- State.Env.setValue(
- *E, createOptionalValue(State.Env.getResultObjectLocation(*E),
- State.Env.getBoolLiteralValue(true), State.Env));
+ setHasValue(State.Env.getResultObjectLocation(*E),
+ State.Env.getBoolLiteralValue(true), State.Env);
}
void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
@@ -483,9 +471,6 @@ void transferValueOrNotEqX(const Expr *ComparisonExpr,
void transferCallReturningOptional(const CallExpr *E,
const MatchFinder::MatchResult &Result,
LatticeTransferState &State) {
- if (State.Env.getValue(*E) != nullptr)
- return;
-
RecordStorageLocation *Loc = nullptr;
if (E->isPRValue()) {
Loc = &State.Env.getResultObjectLocation(*E);
@@ -497,16 +482,16 @@ void transferCallReturningOptional(const CallExpr *E,
}
}
- RecordValue &Val =
- createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
- if (E->isPRValue())
- State.Env.setValue(*E, Val);
+ if (State.Env.getValue(locForHasValue(*Loc)) != nullptr)
+ return;
+
+ setHasValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
}
void constructOptionalValue(const Expr &E, Environment &Env,
BoolValue &HasValueVal) {
RecordStorageLocation &Loc = Env.getResultObjectLocation(E);
- Env.setValue(E, createOptionalValue(Loc, HasValueVal, Env));
+ setHasValue(Loc, HasValueVal, Env);
}
/// Returns a symbolic value for the "has_value" property of an `optional<T>`
@@ -555,7 +540,7 @@ void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal,
assert(E->getNumArgs() > 0);
if (auto *Loc = State.Env.get<RecordStorageLocation>(*E->getArg(0))) {
- createOptionalValue(*Loc, HasValueVal, State.Env);
+ setHasValue(*Loc, HasValueVal, State.Env);
// Assign a storage location for the whole expression.
State.Env.setStorageLocation(*E, *Loc);
@@ -587,11 +572,11 @@ void transferSwap(RecordStorageLocation *Loc1, RecordStorageLocation *Loc2,
if (Loc1 == nullptr) {
if (Loc2 != nullptr)
- createOptionalValue(*Loc2, Env.makeAtomicBoolValue(), Env);
+ setHasValue(*Loc2, Env.makeAtomicBoolValue(), Env);
return;
}
if (Loc2 == nullptr) {
- createOptionalValue(*Loc1, Env.makeAtomicBoolValue(), Env);
+ setHasValue(*Loc1, Env.makeAtomicBoolValue(), Env);
return;
}
@@ -609,8 +594,8 @@ void transferSwap(RecordStorageLocation *Loc1, RecordStorageLocation *Loc2,
if (BoolVal2 == nullptr)
BoolVal2 = &Env.makeAtomicBoolValue();
- createOptionalValue(*Loc1, *BoolVal2, Env);
- createOptionalValue(*Loc2, *BoolVal1, Env);
+ setHasValue(*Loc1, *BoolVal2, Env);
+ setHasValue(*Loc2, *BoolVal1, Env);
}
void transferSwapCall(const CXXMemberCallExpr *E,
@@ -806,8 +791,7 @@ auto buildTransferMatchSwitch() {
LatticeTransferState &State) {
if (RecordStorageLocation *Loc =
getImplicitObjectLocation(*E, State.Env)) {
- createOptionalValue(*Loc, State.Env.getBoolLiteralValue(true),
- State.Env);
+ setHasValue(*Loc, State.Env.getBoolLiteralValue(true), State.Env);
}
})
@@ -818,8 +802,8 @@ auto buildTransferMatchSwitch() {
LatticeTransferState &State) {
if (RecordStorageLocation *Loc =
getImplicitObjectLocation(*E, State.Env)) {
- createOptionalValue(*Loc, State.Env.getBoolLiteralValue(false),
- State.Env);
+ setHasValue(*Loc, State.Env.getBoolLiteralValue(false),
+ State.Env);
}
})
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index 2f0b0e5c5640c3..b8401230a83d43 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -83,9 +83,6 @@ void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
copySyntheticField(SrcFieldLoc->getType(), *SrcFieldLoc,
Dst.getSyntheticField(Name), Env);
}
-
- RecordValue *DstVal = &Env.create<RecordValue>(Dst);
- Env.setValue(Dst, *DstVal);
}
bool recordsEqual(const RecordStorageLocation &Loc1, const Environment &Env1,
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 1e034771014eaa..2771c8b2e37ebb 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -96,6 +96,8 @@ static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
}
static void propagateValue(const Expr &From, const Expr &To, Environment &Env) {
+ if (From.getType()->isRecordType())
+ return;
if (auto *Val = Env.getValue(From))
Env.setValue(To, *Val);
}
@@ -403,6 +405,9 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return;
if (Ret->isPRValue()) {
+ if (Ret->getType()->isRecordType())
+ return;
+
auto *Val = Env.getValue(*Ret);
if (Val == nullptr)
return;
@@ -457,15 +462,9 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
assert(ArgExpr != nullptr);
propagateValueOrStorageLocation(*ArgExpr, *S, Env);
- // If this is a prvalue of record type, we consider it to be an "original
- // record constructor", which we always require to have a `RecordValue`.
- // So make sure we have a value if we didn't propagate one above.
if (S->isPRValue() && S->getType()->isRecordType()) {
- if (Env.getValue(*S) == nullptr) {
- auto &Loc = Env.getResultObjectLocation(*S);
- Env.initializeFieldsWithValues(Loc);
- refreshRecordValue(Loc, Env);
- }
+ auto &Loc = Env.getResultObjectLocation(*S);
+ Env.initializeFieldsWithValues(Loc);
}
}
@@ -495,7 +494,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}
RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
- Env.setValue(*S, refreshRecordValue(Loc, Env));
if (ConstructorDecl->isCopyOrMoveConstructor()) {
// It is permissible for a copy/move constructor to have additional
@@ -542,8 +540,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
RecordStorageLocation *LocSrc = nullptr;
if (Arg1->isPRValue()) {
- if (auto *Val = Env.get<RecordValue>(*Arg1))
- LocSrc = &Val->getLoc();
+ LocSrc = &Env.getResultObjectLocation(*Arg1);
} else {
LocSrc = Env.get<RecordStorageLocation>(*Arg1);
}
@@ -575,15 +572,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
propagateValue(*RBO->getSemanticForm(), *RBO, Env);
}
- void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *S) {
- if (S->getCastKind() == CK_ConstructorConversion) {
- const Expr *SubExpr = S->getSubExpr();
- assert(SubExpr != nullptr);
-
- propagateValue(*SubExpr, *S, Env);
- }
- }
-
void VisitCallExpr(const CallExpr *S) {
// Of clang's builtins, only `__builtin_expect` is handled explicitly, since
// others (like trap, debugtrap, and unreachable) are handled by CFG
@@ -613,12 +601,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// If this call produces a prvalue of record type, initialize its fields
// with values.
- if (S->getType()->isRecordType() && S->isPRValue())
- if (Env.getValue(*S) == nullptr) {
- RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
- Env.initializeFieldsWithValues(Loc);
- Env.setValue(*S, refreshRecordValue(Loc, Env));
- }
+ if (S->getType()->isRecordType() && S->isPRValue()) {
+ RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
+ Env.initializeFieldsWithValues(Loc);
+ }
}
}
@@ -626,18 +612,16 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
- Value *SubExprVal = Env.getValue(*SubExpr);
- if (SubExprVal == nullptr)
- return;
+ StorageLocation &Loc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, Loc);
- if (RecordValue *RecordVal = dyn_cast<RecordValue>(SubExprVal)) {
- Env.setStorageLocation(*S, RecordVal->getLoc());
+ if (SubExpr->getType()->isRecordType())
+ // Nothing else left to do -- we initialized the record when transferring
+ // `SubExpr`.
return;
- }
- StorageLocation &Loc = Env.createStorageLocation(*S);
- Env.setValue(Loc, *SubExprVal);
- Env.setStorageLocation(*S, Loc);
+ if (Value *SubExprVal = Env.getValue(*SubExpr))
+ Env.setValue(Loc, *SubExprVal);
}
void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
@@ -683,15 +667,11 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return;
}
- // In case the initializer list is transparent, we just need to propagate
- // the value that it contains.
- if (S->isSemanticForm() && S->isTransparent()) {
- propagateValue(*S->getInit(0), *S, Env);
+ // If the initializer list is transparent, there's nothing to do.
+ if (S->isSemanticForm() && S->isTransparent())
return;
- }
RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
- Env.setValue(*S, refreshRecordValue(Loc, Env));
// Initialization of base classes and fields of record type happens when we
// visit the nested `CXXConstructExpr` or `InitListExpr` for that base class
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 1b73c5d6830161..71d5c1a6c4f4a3 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -367,11 +367,11 @@ builtinTransferInitializer(const CFGInitializer &Elt,
return;
ParentLoc->setChild(*Member, InitExprLoc);
- } else if (auto *InitExprVal = Env.getValue(*InitExpr)) {
- assert(MemberLoc != nullptr);
// Record-type initializers construct themselves directly into the result
// object, so there is no need to handle them here.
- if (!Member->getType()->isRecordType())
+ } else if (!Member->getType()->isRecordType()) {
+ assert(MemberLoc != nullptr);
+ if (auto *InitExprVal = Env.getValue(*InitExpr))
Env.setValue(*MemberLoc, *InitExprVal);
}
}
diff --git a/clang/lib/Analysis/FlowSensitive/Value.cpp b/clang/lib/Analysis/FlowSensitive/Value.cpp
index 7fad6deb0e918f..d70e5a82ea2327 100644
--- a/clang/lib/Analysis/FlowSensitive/Value.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Value.cpp
@@ -46,8 +46,6 @@ raw_ostream &operator<<(raw_ostream &OS, const Value &Val) {
return OS << "Integer(@" << &Val << ")";
case Value::Kind::Pointer:
return OS << "Pointer(" << &cast<PointerValue>(Val).getPointeeLoc() << ")";
- case Value::Kind::Record:
- return OS << "Record(" << &cast<RecordValue>(Val).getLoc() << ")";
case Value::Kind::TopBool:
return OS << "TopBool(" << cast<TopBoolValue>(Val).getAtom() << ")";
case Value::Kind::AtomicBool:
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index cc20623f881ff4..4195648161246c 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -150,56 +150,6 @@ TEST_F(EnvironmentTest, CreateValueRecursiveType) {
EXPECT_THAT(PV, NotNull());
}
-TEST_F(EnvironmentTest, JoinRecords) {
- using namespace ast_matchers;
-
- std::string Code = R"cc(
- struct S {};
- // Need to use the type somewhere so that the `QualType` gets created;
- S s;
- )cc";
-
- auto Unit =
- tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
- auto &Context = Unit->getASTContext();
-
- ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
- auto Results =
- match(qualType(hasDeclaration(recordDecl(hasName("S")))).bind("SType"),
- Context);
- const QualType *TyPtr = selectFirst<QualType>("SType", Results);
- ASSERT_THAT(TyPtr, NotNull());
- QualType Ty = *TyPtr;
- ASSERT_FALSE(Ty.isNull());
-
- auto *ConstructExpr = CXXConstructExpr::CreateEmpty(Context, 0);
- ConstructExpr->setType(Ty);
- ConstructExpr->setValueKind(VK_PRValue);
-
- // Two different `RecordValue`s with the same location are joined into a
- // third `RecordValue` with that same location.
- {
- Environment Env1(DAContext);
- auto &Val1 = *cast<RecordValue>(Env1.createValue(Ty));
- RecordStorageLocation &Loc = Val1.getLoc();
- Env1.setValue(Loc, Val1);
-
- Environment Env2(DAContext);
- auto &Val2 = Env2.create<RecordValue>(Loc);
- Env2.setValue(Loc, Val2);
- Env2.setValue(Loc, Val2);
-
- Environment::ValueModel Model;
- Environment EnvJoined =
- Environment::join(Env1, Env2, Model, Environment::DiscardExprState);
- auto *JoinedVal = cast<RecordValue>(EnvJoined.getValue(Loc));
- EXPECT_NE(JoinedVal, &Val1);
- EXPECT_NE(JoinedVal, &Val2);
- EXPECT_EQ(&JoinedVal->getLoc(), &Loc);
- }
-}
-
TEST_F(EnvironmentTest, DifferentReferenceLocInJoin) {
// This tests the case where the storage location for a reference-type
// variable is different for two states being joined. We used to believe this
@@ -453,35 +403,4 @@ TEST_F(EnvironmentTest,
Contains(Member));
}
-TEST_F(EnvironmentTest, RefreshRecordValue) {
- using namespace ast_matchers;
-
- std::string Code = R"cc(
- struct S {};
- void target () {
- S s;
- s;
- }
- )cc";
-
- auto Unit =
- tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
- auto &Context = Unit->getASTContext();
-
- ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
- auto Results = match(functionDecl(hasName("target")).bind("target"), Context);
- const auto *Target = selectFirst<FunctionDecl>("target", Results);
- ASSERT_THAT(Target, NotNull());
-
- Results = match(declRefExpr(to(varDecl(hasName("s")))).bind("s"), Context);
- const auto *DRE = selectFirst<DeclRefExpr>("s", Results);
- ASSERT_THAT(DRE, NotNull());
-
- Environment Env(DAContext, *Target);
- EXPECT_THAT(Env.getStorageLocation(*DRE), IsNull());
- refreshRecordValue(*DRE, Env);
- EXPECT_THAT(Env.getStorageLocation(*DRE), NotNull());
-}
-
} // namespace
diff --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index 55baa4e2a53775..88b92668c850c6 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -85,10 +85,6 @@ TEST(RecordOpsTest, CopyRecord) {
EXPECT_NE(Env.getValue(S1.getSyntheticField("synth_int")),
Env.getValue(S2.getSyntheticField("synth_int")));
- auto *S1Val = cast<RecordValue>(Env.getValue(S1));
- auto *S2Val = cast<RecordValue>(Env.getValue(S2));
- EXPECT_NE(S1Val, S2Val);
-
copyRecord(S1, S2, Env);
EXPECT_EQ(getFieldValue(&S1, *OuterIntDecl, Env),
@@ -98,10 +94,6 @@ TEST(RecordOpsTest, CopyRecord) {
getFieldValue(&Inner2, *InnerIntDecl, Env));
EXPECT_EQ(Env.getValue(S1.getSyntheticField("synth_int")),
Env.getValue(S2.getSyntheticField("synth_int")));
-
- S1Val = cast<RecordValue>(Env.getValue(S1));
- S2Val = cast<RecordValue>(Env.getValue(S2));
- EXPECT_NE(S1Val, S2Val);
});
}
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 01b8fd0df23519..bb16138126c8f9 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -260,9 +260,6 @@ TEST(TransferTest, StructIncomplete) {
ASSERT_THAT(FooValue, NotNull());
EXPECT_TRUE(isa<RecordStorageLocation>(FooValue->getPointeeLoc()));
- auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
- ASSERT_THAT(FooPointeeValue, NotNull());
- EXPECT_TRUE(isa<RecordValue>(FooPointeeValue));
});
}
@@ -311,9 +308,10 @@ TEST(TransferTest, StructFieldUnmodeled) {
const auto *FooLoc =
cast<RecordStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto *UnmodeledLoc = FooLoc->getChild(*UnmodeledDecl);
- ASSERT_TRUE(isa<RecordStorageLocation>(UnmodeledLoc));
- EXPECT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
+ const auto &UnmodeledLoc =
+ *cast<RecordStorageLocation>(FooLoc->getChild(*UnmodeledDecl));
+ StorageLocation &UnmodeledXLoc = getFieldLoc(UnmodeledLoc, "X", ASTCtx);
+ EXPECT_EQ(Env.getValue(UnmodeledXLoc), nullptr);
const ValueDecl *ZabDecl = findValueDecl(ASTCtx, "Zab");
ASSERT_THAT(ZabDecl, NotNull());
@@ -492,9 +490,6 @@ TEST(TransferTest, ReferenceVarDecl) {
const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
ASSERT_TRUE(isa_and_nonnull<RecordStorageLocation>(FooLoc));
-
- const Value *FooReferentVal = Env.getValue(*FooLoc);
- EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooReferentVal));
});
}
@@ -585,22 +580,21 @@ TEST(TransferTest, SelfReferentialReferenceVarDecl) {
const auto &FooReferentLoc =
*cast<RecordStorageLocation>(BarLoc.getChild(*FooRefDecl));
- EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
- EXPECT_THAT(getFieldValue(&FooReferentLoc, *BarDecl, Env), IsNull());
+ EXPECT_EQ(Env.getValue(*cast<RecordStorageLocation>(
+ FooReferentLoc.getChild(*BarDecl))
+ ->getChild(*FooPtrDecl)),
+ nullptr);
const auto &FooPtrVal =
*cast<PointerValue>(getFieldValue(&BarLoc, *FooPtrDecl, Env));
const auto &FooPtrPointeeLoc =
cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
- EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
- EXPECT_THAT(getFieldValue(&FooPtrPointeeLoc, *BarDecl, Env), IsNull());
-
- EXPECT_THAT(getFieldValue(&BarLoc, *BazRefDecl, Env), NotNull());
+ EXPECT_EQ(Env.getValue(*cast<RecordStorageLocation>(
+ FooPtrPointeeLoc.getChild(*BarDecl))
+ ->getChild(*FooPtrDecl)),
+ nullptr);
- const auto &BazPtrVal =
- *cast<PointerValue>(getFieldValue(&BarLoc, *BazPtrDecl, Env));
- const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
- EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+ EXPECT_TRUE(isa<PointerValue>(getFieldValue(&BarLoc, *BazPtrDecl, Env)));
});
}
@@ -631,9 +625,6 @@ TEST(TransferTest, PointerVarDecl) {
const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
EXPECT_TRUE(isa<RecordStorageLocation>(&FooPointeeLoc));
-
- const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
- EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooPointeeVal));
});
}
@@ -740,20 +731,14 @@ TEST(TransferTest, SelfReferentialPointerVarDecl) {
const auto &BarPointeeLoc =
cast<RecordStorageLocation>(BarVal.getPointeeLoc());
- EXPECT_THAT(getFieldValue(&BarPointeeLoc, *FooRefDecl, Env), NotNull());
-
const auto &FooPtrVal = *cast<PointerValue>(
getFieldValue(&BarPointeeLoc, *FooPtrDecl, Env));
const auto &FooPtrPointeeLoc =
cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
- EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+ EXPECT_EQ(Env.getValue(*FooPtrPointeeLoc.getChild(*BarDecl)), nullptr);
- EXPECT_THAT(getFieldValue(&BarPointeeLoc, *BazRefDecl, Env), NotNull());
-
- const auto &BazPtrVal = *cast<PointerValue>(
- getFieldValue(&BarPointeeLoc, *BazPtrDecl, Env));
- const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
- EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+ EXPECT_TRUE(
+ isa<PointerValue>(getFieldValue(&BarPointeeLoc, *BazPtrDecl, Env)));
});
}
@@ -1165,9 +1150,6 @@ TEST(TransferTest, ReferenceParamDecl) {
const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
ASSERT_TRUE(isa_and_nonnull<RecordStorageLocation>(FooLoc));
-
- const Value *FooReferentVal = Env.getValue(*FooLoc);
- EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooReferentVal));
});
}
@@ -1196,9 +1178,6 @@ TEST(TransferTest, PointerParamDecl) {
const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
EXPECT_TRUE(isa<RecordStorageLocation>(&FooPointeeLoc));
-
- const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
- EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooPointeeVal));
});
}
@@ -1400,8 +1379,7 @@ static void derivedBaseMemberExpectations(
const auto &FooLoc =
*cast<RecordStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto &FooVal = *cast<RecordValue>(Env.getValue(FooLoc));
- EXPECT_EQ(&FooVal.getLoc(), &FooLoc);
+ EXPECT_NE(Env.getValue(*FooLoc.getChild(*BarDecl)), nullptr);
}
TEST(TransferTest, DerivedBaseMemberStructDefault) {
@@ -1850,7 +1828,6 @@ TEST(TransferTest, StructThisMember) {
const auto *QuxLoc =
cast<RecordStorageLocation>(ThisLoc->getChild(*QuxDecl));
- EXPECT_THAT(dyn_cast<RecordValue>(Env.getValue(*QuxLoc)), NotNull());
const auto *BazVal =
cast<IntegerValue>(getFieldValue(QuxLoc, *BazDecl, Env));
@@ -1921,7 +1898,6 @@ TEST(TransferTest, ClassThisMember) {
const auto *QuxLoc =
cast<RecordStorageLocation>(ThisLoc->getChild(*QuxDecl));
- EXPECT_THAT(dyn_cast<RecordValue>(Env.getValue(*QuxLoc)), NotNull());
const auto *BazVal =
cast<IntegerValue>(getFieldValue(QuxLoc, *BazDecl, Env));
@@ -2297,10 +2273,6 @@ TEST(TransferTest, AssignmentOperator) {
const auto *BarLoc2 =
cast<RecordStorageLocation>(Env2.getStorageLocation(*BarDecl));
- const auto *FooVal2 = cast<RecordValue>(Env2.getValue(*FooLoc2));
- const auto *BarVal2 = cast<RecordValue>(Env2.getValue(*BarLoc2));
- EXPECT_NE(FooVal2, BarVal2);
-
EXPECT_TRUE(recordsEqual(*FooLoc2, *BarLoc2, Env2));
const auto *FooBazVal2 =
@@ -2652,12 +2624,7 @@ TEST(TransferTest, CopyConstructor) {
const auto *BarLoc =
cast<RecordStorageLocation>(Env.getStorageLocation(*BarDecl));
- // `Foo` and `Bar` have different `RecordValue`s associated with them.
- const auto *FooVal = cast<RecordValue>(Env.getValue(*FooLoc));
- const auto *BarVal = cast<RecordValue>(Env.getValue(*BarLoc));
- EXPECT_NE(FooVal, BarVal);
-
- // But the records compare equal.
+ // The records compare equal.
EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
// In particular, the value of `Baz` in both records is the same.
@@ -2878,10 +2845,6 @@ TEST(TransferTest, MoveConstructor) {
EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
- const auto *FooVal1 = cast<RecordValue>(Env1.getValue(*FooLoc1));
- const auto *BarVal1 = cast<RecordValue>(Env1.getValue(*BarLoc1));
- EXPECT_NE(FooVal1, BarVal1);
-
const auto *FooBazVal1 =
cast<IntegerValue>(getFieldValue(FooLoc1, *BazDecl, Env1));
const auto *BarBazVal1 =
@@ -2890,8 +2853,6 @@ TEST(TransferTest, MoveConstructor) {
const auto *FooLoc2 =
cast<RecordStorageLocation>(Env2.getStorageLocation(*FooDecl));
- const auto *FooVal2 = cast<RecordValue>(Env2.getValue(*FooLoc2));
- EXPECT_NE(FooVal2, BarVal1);
EXPECT_TRUE(recordsEqual(*FooLoc2, Env2, *BarLoc1, Env1));
const auto *FooBazVal2 =
@@ -3516,7 +3477,8 @@ TEST(TransferTest, NullToPointerCast) {
const StorageLocation &BazPointeeLoc = BazVal->getPointeeLoc();
EXPECT_TRUE(isa<RecordStorageLocation>(BazPointeeLoc));
- EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+ EXPECT_EQ(BazVal, &Env.fork().getOrCreateNullPointerValue(
+ BazPointeeLoc.getType()));
const StorageLocation &NullPointeeLoc = NullVal->getPointeeLoc();
EXPECT_TRUE(isa<ScalarStorageLocation>(NullPointeeLoc));
@@ -3670,10 +3632,14 @@ TEST(TransferTest, CannotAnalyzeMethodOfClassTemplate) {
TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
std::string Code = R"(
- struct A {};
+ struct A {
+ int i;
+ };
void target(A Foo, A Bar, bool Cond) {
A Baz = Cond ? Foo : Bar;
+ // Make sure A::i is modeled.
+ Baz.i;
/*[[p]]*/
}
)";
@@ -3681,26 +3647,20 @@ TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
-
- const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
- ASSERT_THAT(BarDecl, NotNull());
+ auto *FooIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Foo"), "i",
+ ASTCtx, Env));
+ auto *BarIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Bar"), "i",
+ ASTCtx, Env));
+ auto *BazIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Baz"), "i",
+ ASTCtx, Env));
- const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
- ASSERT_THAT(BazDecl, NotNull());
-
- const auto *FooVal = cast<RecordValue>(Env.getValue(*FooDecl));
- const auto *BarVal = cast<RecordValue>(Env.getValue(*BarDecl));
-
- const auto *BazVal = dyn_cast<RecordValue>(Env.getValue(*BazDecl));
- ASSERT_THAT(BazVal, NotNull());
-
- EXPECT_NE(BazVal, FooVal);
- EXPECT_NE(BazVal, BarVal);
+ EXPECT_NE(BazIVal, FooIVal);
+ EXPECT_NE(BazIVal, BarIVal);
});
}
@@ -3967,7 +3927,6 @@ TEST(TransferTest, AssignToUnionMember) {
const auto *BazLoc = dyn_cast_or_null<RecordStorageLocation>(
Env.getStorageLocation(*BazDecl));
ASSERT_THAT(BazLoc, NotNull());
- ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
const auto *FooVal =
cast<IntegerValue>(getFieldValue(BazLoc, *FooDecl, Env));
@@ -5558,17 +5517,15 @@ TEST(TransferTest, ContextSensitiveReturnReferenceWithConditionalOperator) {
ASSERT_THAT(SDecl, NotNull());
auto *SLoc = Env.getStorageLocation(*SDecl);
- ASSERT_THAT(SLoc, NotNull());
- EXPECT_THAT(Env.getValue(*SLoc), NotNull());
+ EXPECT_THAT(SLoc, NotNull());
auto *Loc = Env.getReturnStorageLocation();
- ASSERT_THAT(Loc, NotNull());
- EXPECT_THAT(Env.getValue(*Loc), NotNull());
+ EXPECT_THAT(Loc, NotNull());
// TODO: We would really like to make this stronger assertion, but that
// doesn't work because we don't propagate values correctly through
// the conditional operator yet.
- // ASSERT_THAT(Loc, Eq(SLoc));
+ // EXPECT_EQ(Loc, SLoc);
},
{BuiltinOptions{ContextSensitiveOptions{}}});
}
@@ -6634,7 +6591,7 @@ TEST(TransferTest, NewExpressions_Structs) {
void target() {
Outer *p = new Outer;
// Access the fields to make sure the analysis actually generates children
- // for them in the `RecordStorageLocation` and `RecordValue`.
+ // for them in the `RecordStorageLocation`.
p->OuterField.InnerField;
// [[after_new]]
}
@@ -6656,9 +6613,6 @@ TEST(TransferTest, NewExpressions_Structs) {
*cast<RecordStorageLocation>(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());
- EXPECT_THAT(Env.getValue(OuterFieldLoc), NotNull());
EXPECT_THAT(Env.getValue(InnerFieldLoc), NotNull());
});
}
More information about the cfe-commits
mailing list