mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
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.
This patch handles the straightforward cases. Upcoming separate patches will
handle the cases that are more subtle.
This patch is part of the ongoing migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086 for details).
Depends On D150653 <https://reviews.llvm.org/D150653>
Repository:
rG LLVM Github Monorepo
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 forwardValue(const Expr &From, const Expr &To, Environment &Env) {
+ if (auto *Val = Env.getValueStrict(From))
+ Env.setValueStrict(To, *Val);
+}
+
+static void forwardStorageLocation(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 forwardValueOrStorageLocation(const Expr &From, const Expr &To,
+ Environment &Env) {
+ assert(From.isGLValue() == To.isGLValue());
+ if (From.isGLValue())
+ forwardStorageLocation(From, To, Env);
+ else
+ forwardValue(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);
+ forwardValue(*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);
+ forwardValue(*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);
+ forwardValueOrStorageLocation(*SubExpr, *S, Env);
}
}
@@ -738,10 +743,15 @@
// 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 +790,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 +822,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 +835,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
