This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG5a16665ed535: [clang][dataflow] Use `Strict` accessors in
more places in Transfer.cpp. (authored by mboehme).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150655/new/
https://reviews.llvm.org/D150655
Files:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -48,10 +48,8 @@
static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
Environment &Env) {
- if (auto *LHSValue =
- dyn_cast_or_null<BoolValue>(Env.getValue(LHS, SkipPast::Reference)))
- if (auto *RHSValue =
- dyn_cast_or_null<BoolValue>(Env.getValue(RHS, SkipPast::Reference)))
+ if (auto *LHSValue = dyn_cast_or_null<BoolValue>(Env.getValueStrict(LHS)))
+ if (auto *RHSValue = dyn_cast_or_null<BoolValue>(Env.getValueStrict(RHS)))
return Env.makeIff(*LHSValue, *RHSValue);
return Env.makeAtomicBoolValue();
@@ -121,9 +119,7 @@
// value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion,
// by skipping past the reference.
static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
- // FIXME: this is too flexible: it _allows_ a reference, while it should
- // _require_ one, since lvalues should always be wrapped in `ReferenceValue`.
- auto *Loc = Env.getStorageLocation(E, SkipPast::Reference);
+ auto *Loc = Env.getStorageLocationStrict(E);
if (Loc == nullptr)
return nullptr;
auto *Val = Env.getValue(*Loc);
@@ -139,6 +135,29 @@
return &UnpackedVal;
}
+static void propagateValue(const Expr &From, const Expr &To, Environment &Env) {
+ if (auto *Val = Env.getValueStrict(From))
+ Env.setValueStrict(To, *Val);
+}
+
+static void propagateStorageLocation(const Expr &From, const Expr &To,
+ Environment &Env) {
+ if (auto *Loc = Env.getStorageLocationStrict(From))
+ Env.setStorageLocationStrict(To, *Loc);
+}
+
+// Forwards the value or storage location of `From` to `To` in cases where
+// `From` may be either a glvalue or a prvalue. `To` must be a glvalue iff
+// `From` is a glvalue.
+static void propagateValueOrStorageLocation(const Expr &From, const Expr &To,
+ Environment &Env) {
+ assert(From.isGLValue() == To.isGLValue());
+ if (From.isGLValue())
+ propagateStorageLocation(From, To, Env);
+ else
+ propagateValue(From, To, Env);
+}
+
namespace {
class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
@@ -155,13 +174,11 @@
switch (S->getOpcode()) {
case BO_Assign: {
- auto *LHSLoc = Env.getStorageLocation(*LHS, SkipPast::Reference);
+ auto *LHSLoc = Env.getStorageLocationStrict(*LHS);
if (LHSLoc == nullptr)
break;
- // No skipping should be necessary, because any lvalues should have
- // already been stripped off in evaluating the LValueToRValue cast.
- auto *RHSVal = Env.getValue(*RHS, SkipPast::None);
+ auto *RHSVal = Env.getValueStrict(*RHS);
if (RHSVal == nullptr)
break;
@@ -276,7 +293,7 @@
return;
}
- if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None))
+ if (auto *InitExprVal = Env.getValueStrict(*InitExpr))
Env.setValue(Loc, *InitExprVal);
if (Env.getValue(Loc) == nullptr) {
@@ -443,7 +460,7 @@
}
case UO_LNot: {
auto *SubExprVal =
- dyn_cast_or_null<BoolValue>(Env.getValue(*SubExpr, SkipPast::None));
+ dyn_cast_or_null<BoolValue>(Env.getValueStrict(*SubExpr));
if (SubExprVal == nullptr)
break;
@@ -653,19 +670,13 @@
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
- auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
- if (SubExprLoc == nullptr)
- return;
-
- Env.setStorageLocation(*S, *SubExprLoc);
+ propagateValue(*SubExpr, *S, Env);
}
}
void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) {
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
if (Value *Val = Env.createValue(S->getType()))
- Env.setValue(Loc, *Val);
+ Env.setValueStrict(*S, *Val);
}
void VisitCallExpr(const CallExpr *S) {
@@ -703,22 +714,20 @@
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
- auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
- if (SubExprLoc == nullptr)
+ Value *SubExprVal = Env.getValueStrict(*SubExpr);
+ if (SubExprVal == nullptr)
return;
- Env.setStorageLocation(*S, *SubExprLoc);
+ auto &Loc = Env.createStorageLocation(*S);
+ Env.setStorageLocationStrict(*S, Loc);
+ Env.setValue(Loc, *SubExprVal);
}
void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
- auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
- if (SubExprLoc == nullptr)
- return;
-
- Env.setStorageLocation(*S, *SubExprLoc);
+ propagateValue(*SubExpr, *S, Env);
}
void VisitCXXStaticCastExpr(const CXXStaticCastExpr *S) {
@@ -726,11 +735,7 @@
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
- auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
- if (SubExprLoc == nullptr)
- return;
-
- Env.setStorageLocation(*S, *SubExprLoc);
+ propagateValueOrStorageLocation(*SubExpr, *S, Env);
}
}
@@ -738,10 +743,14 @@
// FIXME: Revisit this once flow conditions are added to the framework. For
// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
// condition.
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- if (Value *Val = Env.createValue(S->getType()))
- Env.setValue(Loc, *Val);
+ if (S->isGLValue()) {
+ auto &Loc = Env.createStorageLocation(*S);
+ Env.setStorageLocationStrict(*S, Loc);
+ if (Value *Val = Env.createValue(S->getType()))
+ Env.setValue(Loc, *Val);
+ } else if (Value *Val = Env.createValue(S->getType())) {
+ Env.setValueStrict(*S, *Val);
+ }
}
void VisitInitListExpr(const InitListExpr *S) {
@@ -780,9 +789,7 @@
}
void VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *S) {
- auto &Loc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, Env.getBoolLiteralValue(S->getValue()));
+ Env.setValueStrict(*S, Env.getBoolLiteralValue(S->getValue()));
}
void VisitParenExpr(const ParenExpr *S) {
@@ -814,11 +821,11 @@
if (!SubExprEnv)
return nullptr;
- if (auto *Val = dyn_cast_or_null<BoolValue>(
- SubExprEnv->getValue(SubExpr, SkipPast::Reference)))
+ if (auto *Val =
+ dyn_cast_or_null<BoolValue>(SubExprEnv->getValueStrict(SubExpr)))
return Val;
- if (Env.getStorageLocation(SubExpr, SkipPast::None) == nullptr) {
+ if (Env.getValueStrict(SubExpr) == nullptr) {
// Sub-expressions that are logic operators are not added in basic blocks
// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
// operator, it may not have been evaluated and assigned a value yet. In
@@ -827,8 +834,7 @@
Visit(&SubExpr);
}
- if (auto *Val = dyn_cast_or_null<BoolValue>(
- Env.getValue(SubExpr, SkipPast::Reference)))
+ if (auto *Val = dyn_cast_or_null<BoolValue>(Env.getValueStrict(SubExpr)))
return Val;
// If the value of `SubExpr` is still unknown, we create a fresh symbolic
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
