mboehme updated this revision to Diff 544335.
mboehme added a comment.

Rebase to head


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155921/new/

https://reviews.llvm.org/D155921

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -390,7 +390,7 @@
     if (const auto *E = selectFirst<CXXConstructExpr>(
             "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
                           getASTContext()))) {
-      cast<StructValue>(Env.getValueStrict(*E))
+      cast<StructValue>(Env.getValue(*E))
           ->setProperty("is_set", Env.getBoolLiteralValue(false));
     } else if (const auto *E = selectFirst<CXXMemberCallExpr>(
                    "call", match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass(
@@ -547,7 +547,7 @@
         *S, getASTContext());
     if (const auto *E = selectFirst<CXXConstructExpr>(
             "construct", Matches)) {
-      cast<StructValue>(Env.getValueStrict(*E))
+      cast<StructValue>(Env.getValue(*E))
           ->setProperty("has_value", Env.getBoolLiteralValue(false));
     } else if (const auto *E =
                    selectFirst<CXXOperatorCallExpr>("operator", Matches)) {
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5524,7 +5524,7 @@
                   ASTCtx));
 
         ASSERT_THAT(ImplicitCast, NotNull());
-        EXPECT_THAT(Env.getValueStrict(*ImplicitCast), IsNull());
+        EXPECT_THAT(Env.getValue(*ImplicitCast), IsNull());
       });
 }
 
Index: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -114,7 +114,7 @@
     return {nullptr, {}, {}};
 
   // Value of the unary op.
-  auto *UnaryOpValue = State.Env.getValueStrict(*UO);
+  auto *UnaryOpValue = State.Env.getValue(*UO);
   if (!UnaryOpValue) {
     UnaryOpValue = &State.Env.makeAtomicBoolValue();
     State.Env.setValueStrict(*UO, *UnaryOpValue);
@@ -133,7 +133,7 @@
                     LatticeTransferState &State) {
   auto &A = State.Env.arena();
   const Formula *Comp;
-  if (BoolValue *V = cast_or_null<BoolValue>(State.Env.getValueStrict(*BO))) {
+  if (BoolValue *V = cast_or_null<BoolValue>(State.Env.getValue(*BO))) {
     Comp = &V->formula();
   } else {
     Comp = &A.makeAtomRef(A.makeAtom());
@@ -143,8 +143,8 @@
   // FIXME Use this as well:
   // auto *NegatedComp = &State.Env.makeNot(*Comp);
 
-  auto *LHS = State.Env.getValueStrict(*BO->getLHS());
-  auto *RHS = State.Env.getValueStrict(*BO->getRHS());
+  auto *LHS = State.Env.getValue(*BO->getLHS());
+  auto *RHS = State.Env.getValue(*BO->getRHS());
 
   if (!LHS || !RHS)
     return;
@@ -271,7 +271,7 @@
       Env.setValue(*Loc, *Val);
     }
   } else {
-    Val = Env.getValueStrict(*E);
+    Val = Env.getValue(*E);
     if (!Val) {
       Val = Env.createValue(E->getType());
       Env.setValueStrict(*E, *Val);
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -125,10 +125,10 @@
 private:
   TerminatorVisitorRetTy extendFlowCondition(const Expr &Cond) {
     // The terminator sub-expression might not be evaluated.
-    if (Env.getValueStrict(Cond) == nullptr)
+    if (Env.getValue(Cond) == nullptr)
       transfer(StmtToEnv, Cond, Env);
 
-    auto *Val = cast_or_null<BoolValue>(Env.getValueStrict(Cond));
+    auto *Val = cast_or_null<BoolValue>(Env.getValue(Cond));
     // Value merging depends on flow conditions from different environments
     // being mutually exclusive -- that is, they cannot both be true in their
     // entirety (even if they may share some clauses). So, we need *some* value
@@ -407,7 +407,7 @@
       return;
 
     ParentLoc->setChild(*Member, InitExprLoc);
-  } else if (auto *InitExprVal = Env.getValueStrict(*InitExpr)) {
+  } else if (auto *InitExprVal = Env.getValue(*InitExpr)) {
     if (Member->getType()->isRecordType()) {
       auto *InitValStruct = cast<StructValue>(InitExprVal);
       // FIXME: Rather than performing a copy here, we should really be
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -49,8 +49,8 @@
 
 static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
                                           Environment &Env) {
-  Value *LHSValue = Env.getValueStrict(LHS);
-  Value *RHSValue = Env.getValueStrict(RHS);
+  Value *LHSValue = Env.getValue(LHS);
+  Value *RHSValue = Env.getValue(RHS);
 
   if (LHSValue == RHSValue)
     return Env.getBoolLiteralValue(true);
@@ -91,7 +91,7 @@
 }
 
 static void propagateValue(const Expr &From, const Expr &To, Environment &Env) {
-  if (auto *Val = Env.getValueStrict(From))
+  if (auto *Val = Env.getValue(From))
     Env.setValueStrict(To, *Val);
 }
 
@@ -133,7 +133,7 @@
       if (LHSLoc == nullptr)
         break;
 
-      auto *RHSVal = Env.getValueStrict(*RHS);
+      auto *RHSVal = Env.getValue(*RHS);
       if (RHSVal == nullptr)
         break;
 
@@ -266,7 +266,7 @@
       // model that with a fresh value in the environment, unless it's already a
       // boolean.
       if (auto *SubExprVal =
-              dyn_cast_or_null<BoolValue>(Env.getValueStrict(*SubExpr)))
+              dyn_cast_or_null<BoolValue>(Env.getValue(*SubExpr)))
         Env.setValueStrict(*S, *SubExprVal);
       else
         // FIXME: If integer modeling is added, then update this code to create
@@ -350,7 +350,7 @@
     switch (S->getOpcode()) {
     case UO_Deref: {
       const auto *SubExprVal =
-          cast_or_null<PointerValue>(Env.getValueStrict(*SubExpr));
+          cast_or_null<PointerValue>(Env.getValue(*SubExpr));
       if (SubExprVal == nullptr)
         break;
 
@@ -367,8 +367,7 @@
       break;
     }
     case UO_LNot: {
-      auto *SubExprVal =
-          dyn_cast_or_null<BoolValue>(Env.getValueStrict(*SubExpr));
+      auto *SubExprVal = dyn_cast_or_null<BoolValue>(Env.getValue(*SubExpr));
       if (SubExprVal == nullptr)
         break;
 
@@ -417,7 +416,7 @@
       return;
 
     if (Ret->isPRValue()) {
-      auto *Val = Env.getValueStrict(*Ret);
+      auto *Val = Env.getValue(*Ret);
       if (Val == nullptr)
         return;
 
@@ -491,8 +490,7 @@
 
       if (S->isElidable()) {
         Env.setStorageLocation(*S, *ArgLoc);
-      } else if (auto *ArgVal = cast_or_null<StructValue>(
-                     Env.getValue(*Arg, SkipPast::Reference))) {
+      } else if (auto *ArgVal = cast_or_null<StructValue>(Env.getValue(*Arg))) {
         auto &Val = *cast<StructValue>(Env.createValue(S->getType()));
         Env.setValueStrict(*S, Val);
         copyRecord(ArgVal->getAggregateLoc(), Val.getAggregateLoc(), Env);
@@ -587,7 +585,7 @@
     const Expr *SubExpr = S->getSubExpr();
     assert(SubExpr != nullptr);
 
-    Value *SubExprVal = Env.getValueStrict(*SubExpr);
+    Value *SubExprVal = Env.getValue(*SubExpr);
     if (SubExprVal == nullptr)
       return;
 
@@ -702,7 +700,7 @@
     // corresponding environment.
     if (const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr))
       if (auto *Val =
-              dyn_cast_or_null<BoolValue>(SubExprEnv->getValueStrict(SubExpr)))
+              dyn_cast_or_null<BoolValue>(SubExprEnv->getValue(SubExpr)))
         return *Val;
 
     // The sub-expression may lie within a basic block that isn't reachable,
@@ -710,9 +708,9 @@
     // (see https://discourse.llvm.org/t/70775). In this case, visit `SubExpr`
     // within the current environment and then try to get the value that gets
     // assigned to it.
-    if (Env.getValueStrict(SubExpr) == nullptr)
+    if (Env.getValue(SubExpr) == nullptr)
       Visit(&SubExpr);
-    if (auto *Val = dyn_cast_or_null<BoolValue>(Env.getValueStrict(SubExpr)))
+    if (auto *Val = dyn_cast_or_null<BoolValue>(Env.getValue(SubExpr)))
       return *Val;
 
     // If the value of `SubExpr` is still unknown, we create a fresh symbolic
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -238,7 +238,7 @@
 
 /// Ensures that `Expr` is mapped to a `BoolValue` and returns its formula.
 const Formula &forceBoolValue(Environment &Env, const Expr &Expr) {
-  auto *Value = cast_or_null<BoolValue>(Env.getValue(Expr, SkipPast::None));
+  auto *Value = cast_or_null<BoolValue>(Env.getValue(Expr));
   if (Value != nullptr)
     return Value->formula();
 
@@ -403,8 +403,7 @@
 void initializeOptionalReference(const Expr *OptionalExpr,
                                  const MatchFinder::MatchResult &,
                                  LatticeTransferState &State) {
-  if (auto *OptionalVal =
-          State.Env.getValue(*OptionalExpr, SkipPast::Reference)) {
+  if (auto *OptionalVal = State.Env.getValue(*OptionalExpr)) {
     if (OptionalVal->getProperty("has_value") == nullptr) {
       setHasValue(*OptionalVal, State.Env.makeAtomicBoolValue());
     }
@@ -430,7 +429,7 @@
 }
 
 Value *getValueBehindPossiblePointer(const Expr &E, const Environment &Env) {
-  Value *Val = Env.getValue(E, SkipPast::Reference);
+  Value *Val = Env.getValue(E);
   if (auto *PointerVal = dyn_cast_or_null<PointerValue>(Val))
     return Env.getValue(PointerVal->getPointeeLoc());
   return Val;
@@ -579,8 +578,7 @@
 
   // This is a constructor/assignment call for `optional<T>` with argument of
   // type `optional<U>` such that `T` is constructible from `U`.
-  if (auto *HasValueVal =
-          getHasValue(State.Env, State.Env.getValue(E, SkipPast::Reference)))
+  if (auto *HasValueVal = getHasValue(State.Env, State.Env.getValue(E)))
     return *HasValueVal;
   return State.Env.makeAtomicBoolValue();
 }
@@ -714,10 +712,8 @@
   Environment &Env = State.Env;
   auto &A = Env.arena();
   auto *CmpValue = &forceBoolValue(Env, *CmpExpr);
-  if (auto *LHasVal = getHasValue(
-          Env, Env.getValue(*CmpExpr->getArg(0), SkipPast::Reference)))
-    if (auto *RHasVal = getHasValue(
-            Env, Env.getValue(*CmpExpr->getArg(1), SkipPast::Reference))) {
+  if (auto *LHasVal = getHasValue(Env, Env.getValue(*CmpExpr->getArg(0))))
+    if (auto *RHasVal = getHasValue(Env, Env.getValue(*CmpExpr->getArg(1)))) {
       if (CmpExpr->getOperator() == clang::OO_ExclaimEqual)
         CmpValue = &A.makeNot(*CmpValue);
       Env.addToFlowCondition(evaluateEquality(A, *CmpValue, LHasVal->formula(),
@@ -729,7 +725,7 @@
                                  const clang::Expr *E, Environment &Env) {
   auto &A = Env.arena();
   auto *CmpValue = &forceBoolValue(Env, *CmpExpr);
-  if (auto *HasVal = getHasValue(Env, Env.getValue(*E, SkipPast::Reference))) {
+  if (auto *HasVal = getHasValue(Env, Env.getValue(*E))) {
     if (CmpExpr->getOperator() == clang::OO_ExclaimEqual)
       CmpValue = &A.makeNot(*CmpValue);
     Env.addToFlowCondition(
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -686,7 +686,7 @@
   assert(E.isPRValue());
 
   if (auto *StructVal = dyn_cast<StructValue>(&Val)) {
-    if (auto *ExistingVal = cast_or_null<StructValue>(getValueStrict(E)))
+    if (auto *ExistingVal = cast_or_null<StructValue>(getValue(E)))
       assert(&ExistingVal->getAggregateLoc() == &StructVal->getAggregateLoc());
     if (StorageLocation *ExistingLoc = getStorageLocation(E, SkipPast::None))
       assert(ExistingLoc == &StructVal->getAggregateLoc());
@@ -724,9 +724,7 @@
 
 Value *Environment::getValueStrict(const Expr &E) const {
   assert(E.isPRValue());
-  Value *Val = getValue(E, SkipPast::None);
-
-  return Val;
+  return getValue(E);
 }
 
 Value *Environment::createValue(QualType Type) {
@@ -859,7 +857,7 @@
     // assert that `InitExpr` is interpreted, rather than supplying a
     // default value (assuming we don't update the environment API to return
     // references).
-    Val = getValueStrict(*InitExpr);
+    Val = getValue(*InitExpr);
   if (!Val)
     Val = createValue(Ty);
 
@@ -964,8 +962,7 @@
   assert(Expr.getType()->isRecordType());
 
   if (Expr.isPRValue()) {
-    if (auto *ExistingVal =
-            cast_or_null<StructValue>(Env.getValueStrict(Expr))) {
+    if (auto *ExistingVal = cast_or_null<StructValue>(Env.getValue(Expr))) {
       auto &NewVal = Env.create<StructValue>(ExistingVal->getAggregateLoc());
       Env.setValueStrict(Expr, NewVal);
       return NewVal;
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -499,20 +499,14 @@
   /// Equivalent to `getValue(getStorageLocation(E, SP))` if `E` is assigned a
   /// storage location in the environment, otherwise returns null.
   ///
-  /// The `SP` parameter has no effect.
-  ///
-  /// This function is deprecated; prefer `getValueStrict()`. For details, see
-  /// https://discourse.llvm.org/t/70086.
-  Value *getValue(const Expr &E, SkipPast SP) const;
+  /// The `SP` parameter is deprecated and has no effect. New callers should
+  /// avoid passing this parameter.
+  Value *getValue(const Expr &E, SkipPast SP = SkipPast::None) const;
 
   /// Returns the `Value` assigned to the prvalue `E` in the environment, or
   /// null if `E` isn't assigned a value in the environment.
   ///
-  /// This function is the preferred alternative to
-  /// `getValue(const Expr &, SkipPast)`. Once the migration to strict handling
-  /// of value categories is complete (see https://discourse.llvm.org/t/70086),
-  /// `getValue()` will be removed and this function will be renamed to
-  /// `getValue()`.
+  /// This function is deprecated. Call `getValue(E)` instead.
   ///
   /// Requirements:
   ///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to