[clang][dataflow] Replace initValueInStorageLocation with createValue

Since Environment's setValue method already does part of the work that
initValueInStorageLocation does, we can factor out a new createValue
method to reduce the duplication.

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed-by: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D117493

diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 9613b29..5082181 100644 (file)
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -116,15 +116,15 @@ public:
   /// in the environment.
   StorageLocation *getThisPointeeStorageLocation() const;
 
-  /// Creates a value appropriate for `Type`, assigns it to `Loc`, and returns
-  /// it, if `Type` is supported, otherwise return null. If `Type` is a pointer
-  /// or reference type, creates all the necessary storage locations and values
-  /// for indirections until it finds a non-pointer/non-reference type.
+  /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
+  /// return null. If `Type` is a pointer or reference type, creates all the
+  /// necessary storage locations and values for indirections until it finds a
+  /// non-pointer/non-reference type.
   ///
   /// Requirements:
   ///
   ///  `Type` must not be null.
-  Value *initValueInStorageLocation(const StorageLocation &Loc, QualType Type);
+  Value *createValue(QualType Type);
 
   /// Assigns `Val` as the value of `Loc` in the environment.
   void setValue(const StorageLocation &Loc, Value &Val);
@@ -150,8 +150,8 @@ public:
   Value &takeOwnership(std::unique_ptr<Value> Val);
 
 private:
-  /// Returns the value assigned to `Loc` in the environment or null if `Type`
-  /// isn't supported.
+  /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
+  /// return null.
   ///
   /// Recursively initializes storage locations and values until it sees a
   /// self-referential pointer or reference type. `Visited` is used to track
@@ -161,9 +161,8 @@ private:
   /// Requirements:
   ///
   ///  `Type` must not be null.
-  Value *initValueInStorageLocationUnlessSelfReferential(
-      const StorageLocation &Loc, QualType Type,
-      llvm::DenseSet<QualType> &Visited);
+  Value *createValueUnlessSelfReferential(QualType Type,
+                                          llvm::DenseSet<QualType> &Visited);
 
   StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
   const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 7d0cbf3..e1d420f 100644 (file)
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -49,7 +49,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
       assert(ParamDecl != nullptr);
       auto &ParamLoc = createStorageLocation(*ParamDecl);
       setStorageLocation(*ParamDecl, ParamLoc);
-      initValueInStorageLocation(ParamLoc, ParamDecl->getType());
+      if (Value *ParamVal = createValue(ParamDecl->getType()))
+        setValue(ParamLoc, *ParamVal);
     }
   }
 
@@ -60,7 +61,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
       if (!ThisPointeeType->isUnionType()) {
         auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
         DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
-        initValueInStorageLocation(ThisPointeeLoc, ThisPointeeType);
+        if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+          setValue(ThisPointeeLoc, *ThisPointeeVal);
       }
     }
   }
@@ -189,21 +191,17 @@ Value *Environment::getValue(const Expr &E, SkipPast SP) const {
   return getValue(*Loc);
 }
 
-Value *Environment::initValueInStorageLocation(const StorageLocation &Loc,
-                                               QualType Type) {
+Value *Environment::createValue(QualType Type) {
   llvm::DenseSet<QualType> Visited;
-  return initValueInStorageLocationUnlessSelfReferential(Loc, Type, Visited);
+  return createValueUnlessSelfReferential(Type, Visited);
 }
 
-Value *Environment::initValueInStorageLocationUnlessSelfReferential(
-    const StorageLocation &Loc, QualType Type,
-    llvm::DenseSet<QualType> &Visited) {
+Value *Environment::createValueUnlessSelfReferential(
+    QualType Type, llvm::DenseSet<QualType> &Visited) {
   assert(!Type.isNull());
 
   if (Type->isIntegerType()) {
-    auto &Val = takeOwnership(std::make_unique<IntegerValue>());
-    setValue(Loc, Val);
-    return &Val;
+    return &takeOwnership(std::make_unique<IntegerValue>());
   }
 
   if (Type->isReferenceType()) {
@@ -212,14 +210,15 @@ Value *Environment::initValueInStorageLocationUnlessSelfReferential(
 
     if (!Visited.contains(PointeeType.getCanonicalType())) {
       Visited.insert(PointeeType.getCanonicalType());
-      initValueInStorageLocationUnlessSelfReferential(PointeeLoc, PointeeType,
-                                                      Visited);
+      Value *PointeeVal =
+          createValueUnlessSelfReferential(PointeeType, Visited);
       Visited.erase(PointeeType.getCanonicalType());
+
+      if (PointeeVal != nullptr)
+        setValue(PointeeLoc, *PointeeVal);
     }
 
-    auto &Val = takeOwnership(std::make_unique<ReferenceValue>(PointeeLoc));
-    setValue(Loc, Val);
-    return &Val;
+    return &takeOwnership(std::make_unique<ReferenceValue>(PointeeLoc));
   }
 
   if (Type->isPointerType()) {
@@ -228,19 +227,18 @@ Value *Environment::initValueInStorageLocationUnlessSelfReferential(
 
     if (!Visited.contains(PointeeType.getCanonicalType())) {
       Visited.insert(PointeeType.getCanonicalType());
-      initValueInStorageLocationUnlessSelfReferential(PointeeLoc, PointeeType,
-                                                      Visited);
+      Value *PointeeVal =
+          createValueUnlessSelfReferential(PointeeType, Visited);
       Visited.erase(PointeeType.getCanonicalType());
+
+      if (PointeeVal != nullptr)
+        setValue(PointeeLoc, *PointeeVal);
     }
 
-    auto &Val = takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
-    setValue(Loc, Val);
-    return &Val;
+    return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
   }
 
   if (Type->isStructureOrClassType()) {
-    auto *AggregateLoc = cast<AggregateStorageLocation>(&Loc);
-
     // FIXME: Initialize only fields that are accessed in the context that is
     // being analyzed.
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
@@ -253,15 +251,12 @@ Value *Environment::initValueInStorageLocationUnlessSelfReferential(
 
       Visited.insert(FieldType.getCanonicalType());
       FieldValues.insert(
-          {Field, initValueInStorageLocationUnlessSelfReferential(
-                      AggregateLoc->getChild(*Field), FieldType, Visited)});
+          {Field, createValueUnlessSelfReferential(FieldType, Visited)});
       Visited.erase(FieldType.getCanonicalType());
     }
 
-    auto &Val =
-        takeOwnership(std::make_unique<StructValue>(std::move(FieldValues)));
-    setValue(Loc, Val);
-    return &Val;
+    return &takeOwnership(
+        std::make_unique<StructValue>(std::move(FieldValues)));
   }
 
   return nullptr;

diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index cc73fb0..6ae4573 100644 (file)
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -96,7 +96,8 @@ public:
     const Expr *InitExpr = D.getInit();
     if (InitExpr == nullptr) {
       // No initializer expression - associate `Loc` with a new value.
-      Env.initValueInStorageLocation(Loc, D.getType());
+      if (Value *Val = Env.createValue(D.getType()))
+        Env.setValue(Loc, *Val);
       return;
     }
 
@@ -117,7 +118,8 @@ public:
         // FIXME: The initializer expression must always be assigned a value.
         // Replace this with an assert when we have sufficient coverage of
         // language features.
-        Env.initValueInStorageLocation(Loc, D.getType());
+        if (Value *Val = Env.createValue(D.getType()))
+          Env.setValue(Loc, *Val);
       }
       return;
     }
@@ -128,7 +130,8 @@ public:
       // FIXME: The initializer expression must always be assigned a value.
       // Replace this with an assert when we have sufficient coverage of
       // language features.
-      Env.initValueInStorageLocation(Loc, D.getType());
+      if (Value *Val = Env.createValue(D.getType()))
+        Env.setValue(Loc, *Val);
     } else {
       llvm_unreachable("structs and classes must always be assigned values");
     }
@@ -269,7 +272,8 @@ public:
 
     auto &Loc = Env.createStorageLocation(*S);
     Env.setStorageLocation(*S, Loc);
-    Env.initValueInStorageLocation(Loc, S->getType());
+    if (Value *Val = Env.createValue(S->getType()))
+      Env.setValue(Loc, *Val);
   }
 
   void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -319,7 +323,8 @@ public:
   void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) {
     auto &Loc = Env.createStorageLocation(*S);
     Env.setStorageLocation(*S, Loc);
-    Env.initValueInStorageLocation(Loc, S->getType());
+    if (Value *Val = Env.createValue(S->getType()))
+      Env.setValue(Loc, *Val);
   }
 
   void VisitCallExpr(const CallExpr *S) {