mboehme created this revision.
Herald added subscribers: martong, xazax.hun, inglorion.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149144
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -403,14 +403,9 @@
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));
});
}
@@ -494,11 +489,9 @@
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));
@@ -507,13 +500,17 @@
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));
@@ -1058,16 +1055,9 @@
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));
});
}
@@ -1905,15 +1895,13 @@
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);
});
}
@@ -2593,9 +2581,8 @@
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());
});
}
@@ -2643,17 +2630,20 @@
void target(int *Foo) {
do {
int Bar = *Foo;
+ // [[in_loop]]
} while (true);
(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());
@@ -2662,15 +2652,14 @@
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);
+
+ ASSERT_THAT(EnvAfterLoop.getValue(*BarDecl), IsNull());
});
}
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -40,9 +40,10 @@
namespace clang {
namespace dataflow {
+namespace {
+
/// Returns the index of `Block` in the successors of `Pred`.
-static int blockIndexInPredecessor(const CFGBlock &Pred,
- const CFGBlock &Block) {
+int blockIndexInPredecessor(const CFGBlock &Pred, const CFGBlock &Block) {
auto BlockPos = llvm::find_if(
Pred.succs(), [&Block](const CFGBlock::AdjacentBlock &Succ) {
return Succ && Succ->getBlockID() == Block.getBlockID();
@@ -50,7 +51,7 @@
return BlockPos - Pred.succ_begin();
}
-static bool isLoopHead(const CFGBlock &B) {
+bool isLoopHead(const CFGBlock &B) {
if (const auto *T = B.getTerminatorStmt())
switch (T->getStmtClass()) {
case Stmt::WhileStmtClass:
@@ -193,8 +194,8 @@
/// All predecessors of `Block` except those with loop back edges must have
/// already been transferred. States in `AC.BlockStates` that are set to
/// `std::nullopt` represent basic blocks that are not evaluated yet.
-static TypeErasedDataflowAnalysisState
-computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) {
+TypeErasedDataflowAnalysisState computeBlockInputState(const CFGBlock &Block,
+ AnalysisContext &AC) {
llvm::DenseSet<const CFGBlock *> Preds;
Preds.insert(Block.pred_begin(), Block.pred_end());
if (Block.getTerminator().isTemporaryDtorsBranch()) {
@@ -319,6 +320,13 @@
}
}
+void builtinTransferScopeEnd(const CFGScopeEnd &Elt,
+ TypeErasedDataflowAnalysisState &InputState) {
+ if (const VarDecl *VD = Elt.getVarDecl()) {
+ InputState.Env.unsetStorageLocation(*VD);
+ }
+}
+
void builtinTransfer(const CFGElement &Elt,
TypeErasedDataflowAnalysisState &State,
AnalysisContext &AC) {
@@ -329,6 +337,9 @@
case CFGElement::Initializer:
builtinTransferInitializer(Elt.castAs<CFGInitializer>(), State);
break;
+ case CFGElement::ScopeEnd:
+ builtinTransferScopeEnd(Elt.castAs<CFGScopeEnd>(), State);
+ break;
default:
// FIXME: Evaluate other kinds of `CFGElement`.
break;
@@ -370,6 +381,8 @@
return State;
}
+} // namespace
+
TypeErasedDataflowAnalysisState transferBlock(
const ControlFlowContext &CFCtx,
llvm::ArrayRef<std::optional<TypeErasedDataflowAnalysisState>> BlockStates,
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -223,24 +223,7 @@
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,70 @@
// 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 +320,16 @@
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 +538,7 @@
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;
}
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -49,6 +49,19 @@
return Result;
}
+/// Returns whether there is a key that is present in both maps and maps to
+/// different values.
+template <typename K, typename V>
+bool conflictingValues(const llvm::DenseMap<K, V> &Map1,
+ const llvm::DenseMap<K, V> &Map2) {
+ for (auto &Entry : Map1) {
+ auto It = Map2.find(Entry.first);
+ if (It != Map2.end() && Entry.second != It->second)
+ return true;
+ }
+ return false;
+}
+
static bool compareDistinctValues(QualType Type, Value &Val1,
const Environment &Env1, Value &Val2,
const Environment &Env2,
@@ -247,9 +260,9 @@
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 +304,12 @@
for (const auto *ParamDecl : FuncDecl->parameters()) {
assert(ParamDecl != nullptr);
- auto &ParamLoc = createStorageLocation(*ParamDecl);
+ 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 +391,19 @@
continue;
const VarDecl *Param = *ParamIt;
- auto &Loc = createStorageLocation(*Param);
- setStorageLocation(*Param, Loc);
QualType ParamType = Param->getType();
if (ParamType->isReferenceType()) {
- auto &Val = 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 +535,7 @@
JoinedEnv.ReturnLoc = ReturnLoc;
JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
+ assert(!conflictingValues(DeclToLoc, Other.DeclToLoc));
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
Effect = LatticeJoinEffect::Changed;
@@ -589,13 +607,28 @@
void Environment::setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
assert(!DeclToLoc.contains(&D));
+ assert(dyn_cast_or_null<ReferenceValue>(getValue(Loc)) == nullptr);
DeclToLoc[&D] = &Loc;
}
+void Environment::unsetStorageLocation(const ValueDecl &D) {
+ bool erased = DeclToLoc.erase(&D);
+ if (!erased)
+ D.dump();
+ assert(erased);
+}
+
StorageLocation *Environment::getStorageLocation(const ValueDecl &D,
SkipPast SP) const {
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(dyn_cast_or_null<ReferenceValue>(getValue(*Loc)) == nullptr);
+
+ return Loc;
}
void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) {
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -75,6 +75,7 @@
Options.AddTemporaryDtors = true;
Options.AddInitializers = true;
Options.AddCXXDefaultInitExprInCtors = true;
+ Options.AddScopes = true;
// Ensure that all sub-expressions in basic blocks are evaluated.
Options.setAllAlwaysAdd();
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -264,13 +264,31 @@
///
/// 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;
+ /// Removes the storage location assigned to `D`.
+ ///
+ /// Requirements:
+ ///
+ /// `D` must have a storage location in the environment.
+ void unsetStorageLocation(const ValueDecl &D);
+
+ /// 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.
///
@@ -315,7 +333,11 @@
/// 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.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
