[clang] 5a16665 - [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Sun May 21 23:51:26 PDT 2023
Author: Martin Braenne
Date: 2023-05-22T06:51:10Z
New Revision: 5a16665ed53500b7ee6703be3d5b9e0bca6f1c14
URL: https://github.com/llvm/llvm-project/commit/5a16665ed53500b7ee6703be3d5b9e0bca6f1c14
DIFF: https://github.com/llvm/llvm-project/commit/5a16665ed53500b7ee6703be3d5b9e0bca6f1c14.diff
LOG: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.
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
Reviewed By: sammccall, ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D150655
Added:
Modified:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
Removed:
################################################################################
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 45c601ed49146..838ad934b5fbd 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -48,10 +48,8 @@ const Environment *StmtToEnvMap::getEnvironment(const Stmt &S) const {
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 @@ static BoolValue &unpackValue(BoolValue &V, Environment &Env) {
// 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 @@ static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// 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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
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
More information about the cfe-commits
mailing list