[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

diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 45c601e..838ad93 100644 (file)
--- 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 @@ public:
 
     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 @@ public:
         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 @@ public:
     }
     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 @@ public:
       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 @@ public:
     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 @@ public:
       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 @@ public:
     // 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 @@ public:
   }
 
   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 @@ private:
     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 @@ private:
       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