[clang] f76f667 - [clang][dataflow] Use `Strict` accessors where we weren't using them yet.

Martin Braenne via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 31 12:40:18 PDT 2023


Author: Martin Braenne
Date: 2023-07-31T19:40:04Z
New Revision: f76f6674d8221f59f9e515e3cc03ad07fa72fe46

URL: https://github.com/llvm/llvm-project/commit/f76f6674d8221f59f9e515e3cc03ad07fa72fe46
DIFF: https://github.com/llvm/llvm-project/commit/f76f6674d8221f59f9e515e3cc03ad07fa72fe46.diff

LOG: [clang][dataflow] Use `Strict` accessors where we weren't using them yet.

This eliminates all uses of the deprecated accessors.

Reviewed By: ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D156672

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    clang/lib/Analysis/FlowSensitive/Transfer.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 214e0c061ed7bc..c1cd00a73491ae 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -336,8 +336,8 @@ Environment Environment::pushCall(const CallExpr *Call) const {
   if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
     if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
       if (!isa<CXXThisExpr>(Arg))
-        Env.ThisPointeeLoc = cast<AggregateStorageLocation>(
-            getStorageLocation(*Arg, SkipPast::Reference));
+          Env.ThisPointeeLoc =
+              cast<AggregateStorageLocation>(getStorageLocationStrict(*Arg));
       // Otherwise (when the argument is `this`), retain the current
       // environment's `ThisPointeeLoc`.
     }
@@ -832,9 +832,8 @@ StorageLocation &Environment::createObjectInternal(const VarDecl *D,
     // can happen that we can't see the initializer, so `InitExpr` may still
     // be null.
     if (InitExpr) {
-      if (auto *InitExprLoc =
-              getStorageLocation(*InitExpr, SkipPast::Reference))
-        return *InitExprLoc;
+      if (auto *InitExprLoc = getStorageLocationStrict(*InitExpr))
+          return *InitExprLoc;
     }
 
     // Even though we have an initializer, we might not get an
@@ -913,16 +912,13 @@ getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
   Expr *ImplicitObject = MCE.getImplicitObjectArgument();
   if (ImplicitObject == nullptr)
     return nullptr;
-  StorageLocation *Loc =
-      Env.getStorageLocation(*ImplicitObject, SkipPast::Reference);
-  if (Loc == nullptr)
-    return nullptr;
   if (ImplicitObject->getType()->isPointerType()) {
-    if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Loc)))
+    if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*ImplicitObject)))
       return &cast<AggregateStorageLocation>(Val->getPointeeLoc());
     return nullptr;
   }
-  return cast<AggregateStorageLocation>(Loc);
+  return cast_or_null<AggregateStorageLocation>(
+      Env.getStorageLocationStrict(*ImplicitObject));
 }
 
 AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
@@ -930,15 +926,13 @@ AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
   Expr *Base = ME.getBase();
   if (Base == nullptr)
     return nullptr;
-  StorageLocation *Loc = Env.getStorageLocation(*Base, SkipPast::Reference);
-  if (Loc == nullptr)
-    return nullptr;
   if (ME.isArrow()) {
-    if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Loc)))
+    if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Base)))
       return &cast<AggregateStorageLocation>(Val->getPointeeLoc());
     return nullptr;
   }
-  return cast<AggregateStorageLocation>(Loc);
+  return cast_or_null<AggregateStorageLocation>(
+      Env.getStorageLocationStrict(*Base));
 }
 
 std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {

diff  --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index ef51402e171ed6..9b0daf2d95183f 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -254,10 +254,17 @@ class HTMLLogger : public Logger {
       if (ElementIndex > 0) {
         auto S =
             Iters.back().first->Elements[ElementIndex - 1].getAs<CFGStmt>();
-        if (const Expr *E = S ? llvm::dyn_cast<Expr>(S->getStmt()) : nullptr)
-          if (auto *Loc = State.Env.getStorageLocation(*E, SkipPast::None))
-            JOS->attributeObject(
-                "value", [&] { ModelDumper(*JOS, State.Env).dump(*Loc); });
+        if (const Expr *E = S ? llvm::dyn_cast<Expr>(S->getStmt()) : nullptr) {
+          if (E->isPRValue()) {
+            if (auto *V = State.Env.getValue(*E))
+              JOS->attributeObject(
+                  "value", [&] { ModelDumper(*JOS, State.Env).dump(*V); });
+          } else {
+            if (auto *Loc = State.Env.getStorageLocationStrict(*E))
+              JOS->attributeObject(
+                  "value", [&] { ModelDumper(*JOS, State.Env).dump(*Loc); });
+          }
+        }
       }
       if (!ContextLogs.empty()) {
         JOS->attribute("logs", ContextLogs);

diff  --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 14afb3f7a669ca..1edadfd7189302 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -242,10 +242,8 @@ const Formula &forceBoolValue(Environment &Env, const Expr &Expr) {
   if (Value != nullptr)
     return Value->formula();
 
-  auto &Loc = Env.createStorageLocation(Expr);
   Value = &Env.makeAtomicBoolValue();
-  Env.setValue(Loc, *Value);
-  Env.setStorageLocation(Expr, Loc);
+  Env.setValueStrict(Expr, *Value);
   return Value->formula();
 }
 
@@ -439,10 +437,10 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
                         LatticeTransferState &State) {
   if (auto *OptionalVal =
           getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
-    if (State.Env.getStorageLocation(*UnwrapExpr, SkipPast::None) == nullptr)
+    if (State.Env.getStorageLocationStrict(*UnwrapExpr) == nullptr)
       if (auto *Loc = maybeInitializeOptionalValueMember(
               UnwrapExpr->getType(), *OptionalVal, State.Env))
-        State.Env.setStorageLocation(*UnwrapExpr, *Loc);
+        State.Env.setStorageLocationStrict(*UnwrapExpr, *Loc);
   }
 }
 
@@ -471,9 +469,7 @@ void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
   if (auto *HasValueVal = getHasValue(
           State.Env, getValueBehindPossiblePointer(
                          *CallExpr->getImplicitObjectArgument(), State.Env))) {
-    auto &CallExprLoc = State.Env.createStorageLocation(*CallExpr);
-    State.Env.setValue(CallExprLoc, *HasValueVal);
-    State.Env.setStorageLocation(*CallExpr, CallExprLoc);
+    State.Env.setValueStrict(*CallExpr, *HasValueVal);
   }
 }
 
@@ -534,15 +530,20 @@ void transferValueOrNotEqX(const Expr *ComparisonExpr,
 void transferCallReturningOptional(const CallExpr *E,
                                    const MatchFinder::MatchResult &Result,
                                    LatticeTransferState &State) {
-  if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
+  if (State.Env.getValue(*E) != nullptr)
     return;
 
   AggregateStorageLocation *Loc = nullptr;
   if (E->isPRValue()) {
     Loc = &State.Env.getResultObjectLocation(*E);
   } else {
-    Loc = &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
-    State.Env.setStorageLocationStrict(*E, *Loc);
+    Loc = cast_or_null<AggregateStorageLocation>(
+        State.Env.getStorageLocationStrict(*E));
+    if (Loc == nullptr) {
+      Loc =
+          &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
+      State.Env.setStorageLocationStrict(*E, *Loc);
+    }
   }
 
   createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);

diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 87a4fb801cc173..66220553c66d47 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -141,30 +141,26 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       Env.setValue(*LHSLoc, *RHSVal);
 
       // Assign a storage location for the whole expression.
-      Env.setStorageLocation(*S, *LHSLoc);
+      Env.setStorageLocationStrict(*S, *LHSLoc);
       break;
     }
     case BO_LAnd:
     case BO_LOr: {
-      auto &Loc = Env.createStorageLocation(*S);
-      Env.setStorageLocation(*S, Loc);
-
       BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS);
       BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS);
 
       if (S->getOpcode() == BO_LAnd)
-        Env.setValue(Loc, Env.makeAnd(LHSVal, RHSVal));
+        Env.setValueStrict(*S, Env.makeAnd(LHSVal, RHSVal));
       else
-        Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
+        Env.setValueStrict(*S, Env.makeOr(LHSVal, RHSVal));
       break;
     }
     case BO_NE:
     case BO_EQ: {
       auto &LHSEqRHSValue = evaluateBooleanEquality(*LHS, *RHS, Env);
-      auto &Loc = Env.createStorageLocation(*S);
-      Env.setStorageLocation(*S, Loc);
-      Env.setValue(Loc, S->getOpcode() == BO_EQ ? LHSEqRHSValue
-                                                : Env.makeNot(LHSEqRHSValue));
+      Env.setValueStrict(*S, S->getOpcode() == BO_EQ
+                                 ? LHSEqRHSValue
+                                 : Env.makeNot(LHSEqRHSValue));
       break;
     }
     case BO_Comma: {
@@ -238,7 +234,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
           VisitDeclRefExpr(DE);
           VisitMemberExpr(ME);
 
-          if (auto *Loc = Env.getStorageLocation(*ME, SkipPast::Reference))
+          if (auto *Loc = Env.getStorageLocationStrict(*ME))
             Env.setStorageLocation(*B, *Loc);
         } else if (auto *VD = B->getHoldingVar()) {
           // Holding vars are used to back the `BindingDecl`s of tuple-like
@@ -283,9 +279,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       if (SubExprVal == nullptr)
         break;
 
-      auto &ExprLoc = Env.createStorageLocation(*S);
-      Env.setStorageLocation(*S, ExprLoc);
-      Env.setValue(ExprLoc, *SubExprVal);
+      Env.setValueStrict(*S, *SubExprVal);
       break;
     }
 
@@ -308,12 +302,9 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       break;
     }
     case CK_NullToPointer: {
-      auto &Loc = Env.createStorageLocation(S->getType());
-      Env.setStorageLocation(*S, Loc);
-
       auto &NullPointerVal =
           Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
-      Env.setValue(Loc, NullPointerVal);
+      Env.setValueStrict(*S, NullPointerVal);
       break;
     }
     case CK_NullToMemberPointer:
@@ -321,15 +312,11 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       // with this expression.
       break;
     case CK_FunctionToPointerDecay: {
-      StorageLocation *PointeeLoc =
-          Env.getStorageLocation(*SubExpr, SkipPast::Reference);
+      StorageLocation *PointeeLoc = Env.getStorageLocationStrict(*SubExpr);
       if (PointeeLoc == nullptr)
         break;
 
-      auto &PointerLoc = Env.createStorageLocation(*S);
-      auto &PointerVal = Env.create<PointerValue>(*PointeeLoc);
-      Env.setStorageLocation(*S, PointerLoc);
-      Env.setValue(PointerLoc, PointerVal);
+      Env.setValueStrict(*S, Env.create<PointerValue>(*PointeeLoc));
       break;
     }
     case CK_BuiltinFnToFnPtr:
@@ -371,9 +358,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       if (SubExprVal == nullptr)
         break;
 
-      auto &ExprLoc = Env.createStorageLocation(*S);
-      Env.setStorageLocation(*S, ExprLoc);
-      Env.setValue(ExprLoc, Env.makeNot(*SubExprVal));
+      Env.setValueStrict(*S, Env.makeNot(*SubExprVal));
       break;
     }
     default:
@@ -388,16 +373,12 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       // `this` expression's pointee.
       return;
 
-    auto &Loc = Env.createStorageLocation(*S);
-    Env.setStorageLocation(*S, Loc);
-    Env.setValue(Loc, Env.create<PointerValue>(*ThisPointeeLoc));
+    Env.setValueStrict(*S, Env.create<PointerValue>(*ThisPointeeLoc));
   }
 
   void VisitCXXNewExpr(const CXXNewExpr *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 VisitCXXDeleteExpr(const CXXDeleteExpr *S) {
@@ -450,7 +431,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
         if (VarDeclLoc == nullptr)
           return;
 
-        Env.setStorageLocation(*S, *VarDeclLoc);
+        Env.setStorageLocationStrict(*S, *VarDeclLoc);
         return;
       }
     }
@@ -484,16 +465,17 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       assert(Arg != nullptr);
 
       auto *ArgLoc = cast_or_null<AggregateStorageLocation>(
-          Env.getStorageLocation(*Arg, SkipPast::Reference));
+          Env.getStorageLocationStrict(*Arg));
       if (ArgLoc == nullptr)
         return;
 
       if (S->isElidable()) {
-        Env.setStorageLocation(*S, *ArgLoc);
-      } else if (auto *ArgVal = cast_or_null<StructValue>(Env.getValue(*Arg))) {
+        if (Value *Val = Env.getValue(*ArgLoc))
+          Env.setValueStrict(*S, *Val);
+      } else {
         auto &Val = *cast<StructValue>(Env.createValue(S->getType()));
         Env.setValueStrict(*S, Val);
-        copyRecord(ArgVal->getAggregateLoc(), Val.getAggregateLoc(), Env);
+        copyRecord(*ArgLoc, Val.getAggregateLoc(), Env);
       }
       return;
     }
@@ -565,22 +547,20 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       const Expr *Arg = S->getArg(0);
       assert(Arg != nullptr);
 
-      auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::None);
+      auto *ArgLoc = Env.getStorageLocationStrict(*Arg);
       if (ArgLoc == nullptr)
         return;
 
-      Env.setStorageLocation(*S, *ArgLoc);
+      Env.setStorageLocationStrict(*S, *ArgLoc);
     } else if (S->getDirectCallee() != nullptr &&
                S->getDirectCallee()->getBuiltinID() ==
                    Builtin::BI__builtin_expect) {
       assert(S->getNumArgs() > 0);
       assert(S->getArg(0) != nullptr);
-      // `__builtin_expect` returns by-value, so strip away any potential
-      // references in the argument.
-      auto *ArgLoc = Env.getStorageLocation(*S->getArg(0), SkipPast::Reference);
-      if (ArgLoc == nullptr)
+      auto *ArgVal = Env.getValue(*S->getArg(0));
+      if (ArgVal == nullptr)
         return;
-      Env.setStorageLocation(*S, *ArgLoc);
+      Env.setValueStrict(*S, *ArgVal);
     } else if (const FunctionDecl *F = S->getDirectCallee()) {
       transferInlineCall(S, F);
     }
@@ -595,13 +575,13 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       return;
 
     if (StructValue *StructVal = dyn_cast<StructValue>(SubExprVal)) {
-      Env.setStorageLocation(*S, StructVal->getAggregateLoc());
+      Env.setStorageLocationStrict(*S, StructVal->getAggregateLoc());
       return;
     }
 
     StorageLocation &Loc = Env.createStorageLocation(*S);
     Env.setValue(Loc, *SubExprVal);
-    Env.setStorageLocation(*S, Loc);
+    Env.setStorageLocationStrict(*S, Loc);
   }
 
   void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {


        


More information about the cfe-commits mailing list