[clang][dataflow] Replace initValueInStorageLocation with createValue

[sgatev]

Summary

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.

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

public:
   /// assigned a storage location in the environment.
   StorageLocation *getStorageLocation(const Expr &E, SkipPast SP) const;
 
   /// Returns the storage location assigned to the `this` pointee in the
   /// environment or null if the `this` pointee has no assigned storage location
   /// 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);
 
   /// Returns the value assigned to `Loc` in the environment or null if `Loc`
   /// isn't assigned a value in the environment.
   Value *getValue(const StorageLocation &Loc) const;
 
public:
   /// reference to `Loc`.
   StorageLocation &takeOwnership(std::unique_ptr<StorageLocation> Loc);
 
   /// Transfers ownership of `Val` to the analysis context and returns a
   /// reference to `Val`.
   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
   /// which types appeared in the reference/pointer chain in order to avoid
   /// creating a cyclic dependency with self-referential pointers/references.
   ///
   /// Requirements:
   ///
   ///  `Type` must not be null.
-  Value *initValueInStorageLocationUnlessSelfReferential(
-      const StorageLocation &Loc, QualType Type,
+  Value *createValueUnlessSelfReferential(QualType Type,
                                           llvm::DenseSet<QualType> &Visited);
 
   StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
   const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
 
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
   // Maps from program declarations and statements to storage locations that are

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

 Environment::Environment(DataflowAnalysisContext &DACtx,
                          const DeclContext &DeclCtx)
     : Environment(DACtx) {
   if (const auto *FuncDecl = dyn_cast<FunctionDecl>(&DeclCtx)) {
     for (const auto *ParamDecl : FuncDecl->parameters()) {
       assert(ParamDecl != nullptr);
       auto &ParamLoc = createStorageLocation(*ParamDecl);
       setStorageLocation(*ParamDecl, ParamLoc);
-      initValueInStorageLocation(ParamLoc, ParamDecl->getType());
+      if (Value *ParamVal = createValue(ParamDecl->getType()))
+        setValue(ParamLoc, *ParamVal);
     }
   }
 
   if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(&DeclCtx)) {
     if (!MethodDecl->isStatic()) {
       QualType ThisPointeeType = MethodDecl->getThisObjectType();
       // FIXME: Add support for union types.
       if (!ThisPointeeType->isUnionType()) {
         auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
         DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
-        initValueInStorageLocation(ThisPointeeLoc, ThisPointeeType);
+        if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+          setValue(ThisPointeeLoc, *ThisPointeeVal);
       }
     }
   }
 }
 
 bool Environment::operator==(const Environment &Other) const {
   assert(DACtx == Other.DACtx);
   return DeclToLoc == Other.DeclToLoc && LocToVal == Other.LocToVal;

 
 Value *Environment::getValue(const Expr &E, SkipPast SP) const {
   auto *Loc = getStorageLocation(E, SP);
   if (Loc == nullptr)
     return nullptr;
   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()) {
     QualType PointeeType = Type->getAs<ReferenceType>()->getPointeeType();
     auto &PointeeLoc = createStorageLocation(PointeeType);
 
     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()) {
     QualType PointeeType = Type->getAs<PointerType>()->getPointeeType();
     auto &PointeeLoc = createStorageLocation(PointeeType);
 
     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;
     for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) {
       assert(Field != nullptr);
 
       QualType FieldType = Field->getType();
       if (Visited.contains(FieldType.getCanonicalType()))
         continue;
 
       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;
 }
 
 StorageLocation &
 Environment::takeOwnership(std::unique_ptr<StorageLocation> Loc) {
   return DACtx->takeOwnership(std::move(Loc));

clang/lib/Analysis/FlowSensitive/Transfer.cpp

void VisitDeclStmt(const DeclStmt *S) {
     // is safe.
     const auto &D = *cast<VarDecl>(S->getSingleDecl());
     auto &Loc = Env.createStorageLocation(D);
     Env.setStorageLocation(D, Loc);
 
     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;
     }
 
     // The CFG does not contain `ParenExpr` as top-level statements in basic
     // blocks, however sub-expressions can still be of that type.
     InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
     assert(InitExpr != nullptr);
 
     if (D.getType()->isReferenceType()) {
       // Initializing a reference variable - do not create a reference to
       // reference.
       if (auto *InitExprLoc =
               Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
         auto &Val =
             Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
         Env.setValue(Loc, Val);
       } else {
         // 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;
     }
 
     if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
       Env.setValue(Loc, *InitExprVal);
     } else if (!D.getType()->isStructureOrClassType()) {
       // 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");
     }
   }
 
   void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
     // The CFG does not contain `ParenExpr` as top-level statements in basic
     // blocks, however sub-expressions can still be of that type.
if (ConstructorDecl->isCopyOrMoveConstructor()) {
         Env.setStorageLocation(*S, Loc);
         Env.setValue(Loc, *ArgVal);
       }
       return;
     }
 
     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) {
     if (S->getOperator() == OO_Equal) {
       assert(S->getNumArgs() == 2);
 
       const Expr *Arg0 = S->getArg(0);
       assert(Arg0 != nullptr);
if (S->getCastKind() == CK_ConstructorConversion) {
 
       Env.setStorageLocation(*S, *SubExprLoc);
     }
   }
 
   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) {
     if (S->isCallToStdMove()) {
       assert(S->getNumArgs() == 1);
 
       const Expr *Arg = S->getArg(0);
       assert(Arg != nullptr);