[clang] bfbe137 - [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Thu May 4 13:57:39 PDT 2023
Author: Martin Braenne
Date: 2023-05-04T20:57:30Z
New Revision: bfbe137888151dfd506df6b3319d08c4de0e00f5
URL: https://github.com/llvm/llvm-project/commit/bfbe137888151dfd506df6b3319d08c4de0e00f5
DIFF: https://github.com/llvm/llvm-project/commit/bfbe137888151dfd506df6b3319d08c4de0e00f5.diff
LOG: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.
For the wider context of this change, see the RFC at
https://discourse.llvm.org/t/70086.
After this change, global and local variables of reference type are associated
directly with the `StorageLocation` of the referenced object instead of the
`StorageLocation` of a `ReferenceValue`.
Some tests that explicitly check for an existence of `ReferenceValue` for a
variable of reference type have been modified accordingly.
As discussed in the RFC, I have added an assertion to `Environment::join()` to
check that if both environments contain an entry for the same declaration in
`DeclToLoc`, they both map to the same `StorageLocation`. As discussed in
https://discourse.llvm.org/t/70086/5, this also necessitates removing
declarations from `DeclToLoc` when they go out of scope.
In the RFC, I proposed a gradual migration for this change, but it appears
that all of the callers of `Environment::setStorageLocation(const ValueDecl &,
SkipPast` are in the dataflow framework itself, and that there are only a few of
them.
As this is the function whose semantics are changing in a way that callers
potentially need to adapt to, I've decided to change the semantics of the
function directly.
The semantics of `getStorageLocation(const ValueDecl &, SkipPast SP` now no
longer depend on the behavior of the `SP` parameter. (There don't appear to be
any callers that use `SkipPast::ReferenceThenPointer`, so I've added an
assertion that forbids this usage.)
This patch adds a default argument for the `SP` parameter and removes the
explicit `SP` argument at the callsites that are touched by this change. A
followup patch will remove the argument from the remaining callsites,
allowing the `SkipPast` parameter to be removed entirely. (I don't want to do
that in this patch so that semantics-changing changes can be reviewed separately
from semantics-neutral changes.)
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D149144
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
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 9c027ef4552ee..cd1703be0507b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -269,13 +269,24 @@ class Environment {
///
/// Requirements:
///
- /// `D` must not be assigned a storage location in the environment.
+ /// `D` must not already have a storage location in the environment.
+ ///
+ /// If `D` has reference type, `Loc` must refer directly to the referenced
+ /// object (if any), not to a `ReferenceValue`, and it is not permitted to
+ /// later change `Loc` to refer to a `ReferenceValue.`
void setStorageLocation(const ValueDecl &D, StorageLocation &Loc);
- /// Returns the storage location assigned to `D` in the environment, applying
- /// the `SP` policy for skipping past indirections, or null if `D` isn't
- /// assigned a storage location in the environment.
- StorageLocation *getStorageLocation(const ValueDecl &D, SkipPast SP) const;
+ /// Returns the storage location assigned to `D` in the environment, or null
+ /// if `D` isn't assigned a storage location in the environment.
+ ///
+ /// Note that if `D` has reference type, the storage location that is returned
+ /// refers directly to the referenced object, not a `ReferenceValue`.
+ ///
+ /// The `SP` parameter is deprecated and has no effect. In addition, it is
+ /// not permitted to pass `SkipPast::ReferenceThenPointer` for this parameter.
+ /// New uses of this function should use the default argument for `SP`.
+ StorageLocation *getStorageLocation(const ValueDecl &D,
+ SkipPast SP = SkipPast::None) const;
/// Assigns `Loc` as the storage location of `E` in the environment.
///
@@ -320,7 +331,11 @@ class Environment {
/// Equivalent to `getValue(getStorageLocation(D, SP), SkipPast::None)` if `D`
/// is assigned a storage location in the environment, otherwise returns null.
- Value *getValue(const ValueDecl &D, SkipPast SP) const;
+ ///
+ /// The `SP` parameter is deprecated and has no effect. In addition, it is
+ /// not permitted to pass `SkipPast::ReferenceThenPointer` for this parameter.
+ /// New uses of this function should use the default argument for `SP`.
+ Value *getValue(const ValueDecl &D, SkipPast SP = SkipPast::None) const;
/// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if `E`
/// is assigned a storage location in the environment, otherwise returns null.
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 91969cd3386b2..a4d3768b8b189 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -247,9 +247,9 @@ void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
for (const VarDecl *D : Vars) {
if (getStorageLocation(*D, SkipPast::None) != nullptr)
continue;
- auto &Loc = createStorageLocation(*D);
+ auto &Loc = createStorageLocation(D->getType().getNonReferenceType());
setStorageLocation(*D, Loc);
- if (auto *Val = createValue(D->getType()))
+ if (auto *Val = createValue(D->getType().getNonReferenceType()))
setValue(Loc, *Val);
}
@@ -291,10 +291,16 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
for (const auto *ParamDecl : FuncDecl->parameters()) {
assert(ParamDecl != nullptr);
- auto &ParamLoc = createStorageLocation(*ParamDecl);
+ // References aren't objects, so the reference itself doesn't have a
+ // storage location. Instead, the storage location for a reference refers
+ // directly to an object of the referenced type -- so strip off any
+ // reference from the type.
+ auto &ParamLoc =
+ createStorageLocation(ParamDecl->getType().getNonReferenceType());
setStorageLocation(*ParamDecl, ParamLoc);
- if (Value *ParamVal = createValue(ParamDecl->getType()))
- setValue(ParamLoc, *ParamVal);
+ if (Value *ParamVal =
+ createValue(ParamDecl->getType().getNonReferenceType()))
+ setValue(ParamLoc, *ParamVal);
}
QualType ReturnType = FuncDecl->getReturnType();
@@ -376,17 +382,19 @@ void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
continue;
const VarDecl *Param = *ParamIt;
- auto &Loc = createStorageLocation(*Param);
- setStorageLocation(*Param, Loc);
QualType ParamType = Param->getType();
if (ParamType->isReferenceType()) {
- auto &Val = DACtx->arena().create<ReferenceValue>(*ArgLoc);
- setValue(Loc, Val);
- } else if (auto *ArgVal = getValue(*ArgLoc)) {
- setValue(Loc, *ArgVal);
- } else if (Value *Val = createValue(ParamType)) {
- setValue(Loc, *Val);
+ setStorageLocation(*Param, *ArgLoc);
+ } else {
+ auto &Loc = createStorageLocation(*Param);
+ setStorageLocation(*Param, Loc);
+
+ if (auto *ArgVal = getValue(*ArgLoc)) {
+ setValue(Loc, *ArgVal);
+ } else if (Value *Val = createValue(ParamType)) {
+ setValue(Loc, *Val);
+ }
}
}
}
@@ -518,6 +526,10 @@ LatticeJoinEffect Environment::join(const Environment &Other,
JoinedEnv.ReturnLoc = ReturnLoc;
JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
+ // FIXME: Once we're able to remove declarations from `DeclToLoc` when their
+ // lifetime ends, add an assertion that there aren't any entries in
+ // `DeclToLoc` and `Other.DeclToLoc` that map the same declaration to
+ //
diff erent storage locations.
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
Effect = LatticeJoinEffect::Changed;
@@ -589,13 +601,23 @@ StorageLocation &Environment::createStorageLocation(const Expr &E) {
void Environment::setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
assert(!DeclToLoc.contains(&D));
+ assert(!isa_and_nonnull<ReferenceValue>(getValue(Loc)));
DeclToLoc[&D] = &Loc;
}
StorageLocation *Environment::getStorageLocation(const ValueDecl &D,
SkipPast SP) const {
+ assert(SP != SkipPast::ReferenceThenPointer);
+
auto It = DeclToLoc.find(&D);
- return It == DeclToLoc.end() ? nullptr : &skip(*It->second, SP);
+ if (It == DeclToLoc.end())
+ return nullptr;
+
+ StorageLocation *Loc = It->second;
+
+ assert(!isa_and_nonnull<ReferenceValue>(getValue(*Loc)));
+
+ return Loc;
}
void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) {
@@ -662,6 +684,8 @@ Value *Environment::getValue(const StorageLocation &Loc) const {
}
Value *Environment::getValue(const ValueDecl &D, SkipPast SP) const {
+ assert(SP != SkipPast::ReferenceThenPointer);
+
auto *Loc = getStorageLocation(D, SP);
if (Loc == nullptr)
return nullptr;
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index a11ff8668236e..ddabf352fd111 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -223,24 +223,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (DeclLoc == nullptr)
return;
- // If the value is already an lvalue, don't double-wrap it.
- if (isa_and_nonnull<ReferenceValue>(Env.getValue(*DeclLoc))) {
- // We only expect to encounter a `ReferenceValue` for a reference type
- // (always) or for `BindingDecl` (sometimes). For the latter, we can't
- // rely on type, because their type does not indicate whether they are a
- // reference type. The assert is not strictly necessary, since we don't
- // depend on its truth to proceed. But, it verifies our assumptions,
- // which, if violated, probably indicate a problem elsewhere.
- assert((VD->getType()->isReferenceType() || isa<BindingDecl>(VD)) &&
- "Only reference-typed declarations or `BindingDecl`s should map "
- "to `ReferenceValue`s");
- Env.setStorageLocation(*S, *DeclLoc);
- } else {
- auto &Loc = Env.createStorageLocation(*S);
- auto &Val = Env.create<ReferenceValue>(*DeclLoc);
- Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Val);
- }
+ Env.setStorageLocation(*S, *DeclLoc);
}
void VisitDeclStmt(const DeclStmt *S) {
@@ -248,55 +231,69 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// is safe.
const auto &D = *cast<VarDecl>(S->getSingleDecl());
+ ProcessVarDecl(D);
+ }
+
+ void ProcessVarDecl(const VarDecl &D) {
// Static local vars are already initialized in `Environment`.
if (D.hasGlobalStorage())
return;
- // The storage location for `D` could have been created earlier, before the
- // variable's declaration statement (for example, in the case of
- // BindingDecls).
- auto *MaybeLoc = Env.getStorageLocation(D, SkipPast::None);
- if (MaybeLoc == nullptr) {
- MaybeLoc = &Env.createStorageLocation(D);
- Env.setStorageLocation(D, *MaybeLoc);
- }
- auto &Loc = *MaybeLoc;
+ if (D.getType()->isReferenceType()) {
+ // If this is the holding variable for a `BindingDecl`, we may already
+ // have a storage location set up -- so check. (See also explanation below
+ // where we process the `BindingDecl`.)
+ if (Env.getStorageLocation(D) == nullptr) {
+ const Expr *InitExpr = D.getInit();
+ assert(InitExpr != nullptr);
+ if (auto *InitExprLoc =
+ Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
+ Env.setStorageLocation(D, *InitExprLoc);
+ } else {
+ // Even though we have an initializer, we might not get an
+ // InitExprLoc, for example if the InitExpr is a CallExpr for which we
+ // don't have a function body. In this case, we just invent a storage
+ // location and value -- it's the best we can do.
+ StorageLocation &Loc =
+ Env.createStorageLocation(D.getType().getNonReferenceType());
+ Env.setStorageLocation(D, Loc);
+ if (Value *Val = Env.createValue(D.getType().getNonReferenceType()))
+ Env.setValue(Loc, *Val);
+ }
+ }
+ } else {
+ // Not a reference type.
- const Expr *InitExpr = D.getInit();
- if (InitExpr == nullptr) {
- // No initializer expression - associate `Loc` with a new value.
- if (Value *Val = Env.createValue(D.getType()))
- Env.setValue(Loc, *Val);
- return;
- }
+ assert(Env.getStorageLocation(D) == nullptr);
+ StorageLocation &Loc = Env.createStorageLocation(D);
+ Env.setStorageLocation(D, Loc);
- if (D.getType()->isReferenceType()) {
- // Initializing a reference variable - do not create a reference to
- // reference.
- // FIXME: reuse the ReferenceValue instead of creating a new one.
- if (auto *InitExprLoc =
- Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
- auto &Val = Env.create<ReferenceValue>(*InitExprLoc);
- Env.setValue(Loc, Val);
+ const Expr *InitExpr = D.getInit();
+ if (InitExpr == nullptr) {
+ // No initializer expression - associate `Loc` with a new value.
+ if (Value *Val = Env.createValue(D.getType()))
+ Env.setValue(Loc, *Val);
+ return;
}
- } else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
- Env.setValue(Loc, *InitExprVal);
- }
- if (Env.getValue(Loc) == nullptr) {
- // We arrive here in (the few) cases where an expression is intentionally
- // "uninterpreted". There are two ways to handle this situation: propagate
- // the status, so that uninterpreted initializers result in uninterpreted
- // variables, or provide a default value. We choose the latter so that
- // later refinements of the variable can be used for reasoning about the
- // surrounding code.
- //
- // FIXME. If and when we interpret all language cases, change this to
- // assert that `InitExpr` is interpreted, rather than supplying a default
- // value (assuming we don't update the environment API to return
- // references).
- if (Value *Val = Env.createValue(D.getType()))
- Env.setValue(Loc, *Val);
+ if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None))
+ Env.setValue(Loc, *InitExprVal);
+
+ if (Env.getValue(Loc) == nullptr) {
+ // We arrive here in (the few) cases where an expression is
+ // intentionally "uninterpreted". There are two ways to handle this
+ // situation: propagate the status, so that uninterpreted initializers
+ // result in uninterpreted variables, or provide a default value. We
+ // choose the latter so that later refinements of the variable can be
+ // used for reasoning about the surrounding code.
+ //
+ // FIXME. If and when we interpret all language cases, change this to
+ // assert that `InitExpr` is interpreted, rather than supplying a
+ // default value (assuming we don't update the environment API to return
+ // references).
+ if (Value *Val = Env.createValue(D.getType()))
+ Env.setValue(Loc, *Val);
+ }
}
// `DecompositionDecl` must be handled after we've interpreted the loc
@@ -322,15 +319,16 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (auto *Loc = Env.getStorageLocation(*ME, SkipPast::Reference))
Env.setStorageLocation(*B, *Loc);
} else if (auto *VD = B->getHoldingVar()) {
- // Holding vars are used to back the BindingDecls of tuple-like
- // types. The holding var declarations appear *after* this statement,
- // so we have to create a location for them here to share with `B`. We
- // don't visit the binding, because we know it will be a DeclRefExpr
- // to `VD`. Note that, by construction of the AST, `VD` will always be
- // a reference -- either lvalue or rvalue.
- auto &VDLoc = Env.createStorageLocation(*VD);
- Env.setStorageLocation(*VD, VDLoc);
- Env.setStorageLocation(*B, VDLoc);
+ // Holding vars are used to back the `BindingDecl`s of tuple-like
+ // types. The holding var declarations appear after the
+ // `DecompositionDecl`, so we have to explicitly process them here
+ // to know their storage location. They will be processed a second
+ // time when we visit their `VarDecl`s, so we have code that protects
+ // against this above.
+ ProcessVarDecl(*VD);
+ auto *VDLoc = Env.getStorageLocation(*VD);
+ assert(VDLoc != nullptr);
+ Env.setStorageLocation(*B, *VDLoc);
}
}
}
@@ -539,15 +537,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (VarDeclLoc == nullptr)
return;
- if (VarDeclLoc->getType()->isReferenceType()) {
- assert(isa_and_nonnull<ReferenceValue>(Env.getValue((*VarDeclLoc))) &&
- "reference-typed declarations map to `ReferenceValue`s");
- Env.setStorageLocation(*S, *VarDeclLoc);
- } else {
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Env.create<ReferenceValue>(*VarDeclLoc));
- }
+ Env.setStorageLocation(*S, *VarDeclLoc);
return;
}
}
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 99319620f43f9..418a6cff21cc1 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -64,6 +64,8 @@ static bool isLoopHead(const CFGBlock &B) {
return false;
}
+namespace {
+
// The return type of the visit functions in TerminatorVisitor. The first
// element represents the terminator expression (that is the conditional
// expression in case of a path split in the CFG). The second element
@@ -186,6 +188,8 @@ struct AnalysisContext {
llvm::ArrayRef<std::optional<TypeErasedDataflowAnalysisState>> BlockStates;
};
+} // namespace
+
/// Computes the input state for a given basic block by joining the output
/// states of its predecessors.
///
@@ -277,17 +281,19 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) {
}
/// Built-in transfer function for `CFGStmt`.
-void builtinTransferStatement(const CFGStmt &Elt,
- TypeErasedDataflowAnalysisState &InputState,
- AnalysisContext &AC) {
+static void
+builtinTransferStatement(const CFGStmt &Elt,
+ TypeErasedDataflowAnalysisState &InputState,
+ AnalysisContext &AC) {
const Stmt *S = Elt.getStmt();
assert(S != nullptr);
transfer(StmtToEnvMap(AC.CFCtx, AC.BlockStates), *S, InputState.Env);
}
/// Built-in transfer function for `CFGInitializer`.
-void builtinTransferInitializer(const CFGInitializer &Elt,
- TypeErasedDataflowAnalysisState &InputState) {
+static void
+builtinTransferInitializer(const CFGInitializer &Elt,
+ TypeErasedDataflowAnalysisState &InputState) {
const CXXCtorInitializer *Init = Elt.getInitializer();
assert(Init != nullptr);
@@ -320,9 +326,9 @@ void builtinTransferInitializer(const CFGInitializer &Elt,
}
}
-void builtinTransfer(const CFGElement &Elt,
- TypeErasedDataflowAnalysisState &State,
- AnalysisContext &AC) {
+static void builtinTransfer(const CFGElement &Elt,
+ TypeErasedDataflowAnalysisState &State,
+ AnalysisContext &AC) {
switch (Elt.getKind()) {
case CFGElement::Statement:
builtinTransferStatement(Elt.castAs<CFGStmt>(), State, AC);
@@ -331,7 +337,18 @@ void builtinTransfer(const CFGElement &Elt,
builtinTransferInitializer(Elt.castAs<CFGInitializer>(), State);
break;
default:
- // FIXME: Evaluate other kinds of `CFGElement`.
+ // FIXME: Evaluate other kinds of `CFGElement`, including:
+ // - When encountering `CFGLifetimeEnds`, remove the declaration from
+ // `Environment::DeclToLoc`. This would serve two purposes:
+ // a) Eliminate unnecessary clutter from `Environment::DeclToLoc`
+ // b) Allow us to implement an assertion that, when joining two
+ // `Environments`, the two `DeclToLoc` maps never contain entries that
+ // map the same declaration to
diff erent storage locations.
+ // Unfortunately, however, we can't currently process `CFGLifetimeEnds`
+ // because the corresponding CFG option `AddLifetime` is incompatible with
+ // the option 'AddImplicitDtors`, which we already use. We will first
+ // need to modify the CFG implementation to make these two options
+ // compatible before we can process `CFGLifetimeEnds`.
break;
}
}
@@ -344,7 +361,7 @@ void builtinTransfer(const CFGElement &Elt,
/// user-specified analysis.
/// `PostVisitCFG` (if provided) will be applied to the element after evaluation
/// by the user-specified analysis.
-TypeErasedDataflowAnalysisState
+static TypeErasedDataflowAnalysisState
transferCFGBlock(const CFGBlock &Block, AnalysisContext &AC,
std::function<void(const CFGElement &,
const TypeErasedDataflowAnalysisState &)>
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index c8161c8f40fc9..0283fe74e7790 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -404,14 +404,9 @@ TEST(TransferTest, ReferenceVarDecl) {
const StorageLocation *FooLoc =
Env.getStorageLocation(*FooDecl, SkipPast::None);
- ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
-
- const ReferenceValue *FooVal =
- cast<ReferenceValue>(Env.getValue(*FooLoc));
- const StorageLocation &FooReferentLoc = FooVal->getReferentLoc();
- EXPECT_TRUE(isa<AggregateStorageLocation>(&FooReferentLoc));
+ ASSERT_TRUE(isa_and_nonnull<AggregateStorageLocation>(FooLoc));
- const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+ const Value *FooReferentVal = Env.getValue(*FooLoc);
EXPECT_TRUE(isa_and_nonnull<StructValue>(FooReferentVal));
});
}
@@ -495,11 +490,9 @@ TEST(TransferTest, SelfReferentialReferenceVarDecl) {
ASSERT_THAT(BazRefDecl, NotNull());
ASSERT_THAT(BazPtrDecl, NotNull());
- const auto *FooLoc = cast<ScalarStorageLocation>(
+ const auto *FooLoc = cast<AggregateStorageLocation>(
Env.getStorageLocation(*FooDecl, SkipPast::None));
- const auto *FooVal = cast<ReferenceValue>(Env.getValue(*FooLoc));
- const auto *FooReferentVal =
- cast<StructValue>(Env.getValue(FooVal->getReferentLoc()));
+ const auto *FooReferentVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal =
cast<ReferenceValue>(FooReferentVal->getChild(*BarDecl));
@@ -508,13 +501,17 @@ TEST(TransferTest, SelfReferentialReferenceVarDecl) {
const auto *FooRefVal =
cast<ReferenceValue>(BarReferentVal->getChild(*FooRefDecl));
- const StorageLocation &FooReferentLoc = FooRefVal->getReferentLoc();
- EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
+ const auto &FooReferentLoc =
+ cast<AggregateStorageLocation>(FooRefVal->getReferentLoc());
+ EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
+ EXPECT_THAT(Env.getValue(FooReferentLoc.getChild(*BarDecl)), IsNull());
const auto *FooPtrVal =
cast<PointerValue>(BarReferentVal->getChild(*FooPtrDecl));
- const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
- EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+ const auto &FooPtrPointeeLoc =
+ cast<AggregateStorageLocation>(FooPtrVal->getPointeeLoc());
+ EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
+ EXPECT_THAT(Env.getValue(FooPtrPointeeLoc.getChild(*BarDecl)), IsNull());
const auto *BazRefVal =
cast<ReferenceValue>(BarReferentVal->getChild(*BazRefDecl));
@@ -1059,16 +1056,9 @@ TEST(TransferTest, ReferenceParamDecl) {
const StorageLocation *FooLoc =
Env.getStorageLocation(*FooDecl, SkipPast::None);
- ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
-
- const ReferenceValue *FooVal =
- dyn_cast<ReferenceValue>(Env.getValue(*FooLoc));
- ASSERT_THAT(FooVal, NotNull());
+ ASSERT_TRUE(isa_and_nonnull<AggregateStorageLocation>(FooLoc));
- const StorageLocation &FooReferentLoc = FooVal->getReferentLoc();
- EXPECT_TRUE(isa<AggregateStorageLocation>(&FooReferentLoc));
-
- const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+ const Value *FooReferentVal = Env.getValue(*FooLoc);
EXPECT_TRUE(isa_and_nonnull<StructValue>(FooReferentVal));
});
}
@@ -1906,15 +1896,13 @@ TEST(TransferTest, DefaultInitializerReference) {
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
ASSERT_THAT(FooDecl, NotNull());
- const auto *FooVal =
- cast<ReferenceValue>(Env.getValue(*FooDecl, SkipPast::None));
+ const auto *FooLoc = Env.getStorageLocation(*FooDecl);
const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux");
ASSERT_THAT(QuxDecl, NotNull());
- const auto *QuxVal =
- cast<ReferenceValue>(Env.getValue(*QuxDecl, SkipPast::None));
- EXPECT_EQ(&QuxVal->getReferentLoc(), &FooVal->getReferentLoc());
+ const auto *QuxLoc = Env.getStorageLocation(*QuxDecl);
+ EXPECT_EQ(QuxLoc, FooLoc);
});
}
@@ -2594,9 +2582,8 @@ TEST(TransferTest, DerefDependentPtr) {
const auto *FooVal =
cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
- const auto *BarVal =
- cast<ReferenceValue>(Env.getValue(*BarDecl, SkipPast::None));
- EXPECT_EQ(&BarVal->getReferentLoc(), &FooVal->getPointeeLoc());
+ const auto *BarLoc = Env.getStorageLocation(*BarDecl);
+ EXPECT_EQ(BarLoc, &FooVal->getPointeeLoc());
});
}
@@ -2644,17 +2631,20 @@ TEST(TransferTest, VarDeclInDoWhile) {
void target(int *Foo) {
do {
int Bar = *Foo;
+ // [[in_loop]]
} while (false);
(void)0;
- /*[[p]]*/
+ // [[after_loop]]
}
)";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+ const Environment &EnvInLoop =
+ getEnvironmentAtAnnotation(Results, "in_loop");
+ const Environment &EnvAfterLoop =
+ getEnvironmentAtAnnotation(Results, "after_loop");
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
ASSERT_THAT(FooDecl, NotNull());
@@ -2663,15 +2653,17 @@ TEST(TransferTest, VarDeclInDoWhile) {
ASSERT_THAT(BarDecl, NotNull());
const auto *FooVal =
- cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
+ cast<PointerValue>(EnvAfterLoop.getValue(*FooDecl));
const auto *FooPointeeVal =
- cast<IntegerValue>(Env.getValue(FooVal->getPointeeLoc()));
-
- const auto *BarVal = dyn_cast_or_null<IntegerValue>(
- Env.getValue(*BarDecl, SkipPast::None));
- ASSERT_THAT(BarVal, NotNull());
+ cast<IntegerValue>(EnvAfterLoop.getValue(FooVal->getPointeeLoc()));
+ const auto *BarVal = cast<IntegerValue>(EnvInLoop.getValue(*BarDecl));
EXPECT_EQ(BarVal, FooPointeeVal);
+
+ // FIXME: This assertion documents current behavior, but we would prefer
+ // declarations to be removed from the environment when their lifetime
+ // ends. Once this is the case, change this assertion accordingly.
+ ASSERT_THAT(EnvAfterLoop.getValue(*BarDecl), BarVal);
});
}
More information about the cfe-commits
mailing list