[clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.

[mboehme]

Summary

These methods provide a less verbose way of allocating StorageLocations and
Values than the existing takeOwnership(make_unique(...)) pattern.

In addition, because allocation of StorageLocations and Values now happens
within the DataflowAnalysisContext, the create<T>() open up the possibility
of using BumpPtrAllocator to allocate these objects if it turns out this
helps performance.

[xazax.hun]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h

100

Would emplace back work? That returns a reference to the just emplaced element saving us the call to back making the code a bit more concise.

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

328

Just curious, what is the reason for repeating the enable_if here in addition to the one in the called function? Do we get better error messages?

[mboehme]

Changes in response to review comments

[mboehme]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h

100

Nice -- thanks for the suggestion! Done.

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

328

Yes, the idea is:

• We get error messages from the interface of create() rather than the implementation
• The enable_if documents intent. (The comment above states this intent too, but it's always nicer to have the code itself state the intent if possible.)

[ymandel]

Thanks!

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h

215–217

Is there any reason for this function anymore or should we deprecate it (and the Top version below)? Especially in this object which only used internally and not by clients -- the extra two characters don't seem to justify a function.

[mboehme]

Changes in response to review comments

[mboehme]

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h

215–217

I had wondered the same thing. I've added LLVM_DEPRECATED to both.

(I'm not changing callers in this patch because I'd like to avoid expanding the size of the patch.)

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h

public:
   ///
   ///  `S` must not be null.
   DataflowAnalysisContext(std::unique_ptr<Solver> S,
                           Options Opts = Options{
                               /*ContextSensitiveOpts=*/std::nullopt,
                               /*Logger=*/nullptr});
   ~DataflowAnalysisContext();
 
+  /// Creates a `T` (some subclass of `StorageLocation`), forwarding `args` to
+  /// the constructor, and returns a reference to it.
+  ///
+  /// The `DataflowAnalysisContext` takes ownership of the created object. The
+  /// object will be destroyed when the `DataflowAnalysisContext` is destroyed.
+  template <typename T, typename... Args>
+  std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
+  create(Args &&...args) {
+    // Note: If allocation of individual `StorageLocation`s turns out to be
+    // costly, consider creating specializations of `create<T>` for commonly
+    // used `StorageLocation` subclasses and make them use a `BumpPtrAllocator`.
+    return *cast<T>(
+        Locs.emplace_back(std::make_unique<T>(std::forward<Args>(args)...))
+            .get());
+  }
+
+  /// Creates a `T` (some subclass of `Value`), forwarding `args` to the
+  /// constructor, and returns a reference to it.
+  ///
+  /// The `DataflowAnalysisContext` takes ownership of the created object. The
+  /// object will be destroyed when the `DataflowAnalysisContext` is destroyed.
+  template <typename T, typename... Args>
+  std::enable_if_t<std::is_base_of<Value, T>::value, T &>
+  create(Args &&...args) {
+    // Note: If allocation of individual `Value`s turns out to be costly,
+    // consider creating specializations of `create<T>` for commonly used
+    // `Value` subclasses and make them use a `BumpPtrAllocator`.
+    return *cast<T>(
+        Vals.emplace_back(std::make_unique<T>(std::forward<Args>(args)...))
+            .get());
+  }
+
   /// Takes ownership of `Loc` and returns a reference to it.
   ///
+  /// This function is deprecated. Instead of
+  /// `takeOwnership(std::make_unique<SomeStorageLocation>(args))`, prefer
+  /// `create<SomeStorageLocation>(args)`.
+  ///
   /// Requirements:
   ///
   ///  `Loc` must not be null.
   template <typename T>
-  std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
-  takeOwnership(std::unique_ptr<T> Loc) {
+  LLVM_DEPRECATED("use create<T> instead", "")
+  std::enable_if_t<std::is_base_of<StorageLocation, T>::value,
+                   T &> takeOwnership(std::unique_ptr<T> Loc) {
     assert(Loc != nullptr);
     Locs.push_back(std::move(Loc));
     return *cast<T>(Locs.back().get());
   }
 
   /// Takes ownership of `Val` and returns a reference to it.
   ///
+  /// This function is deprecated. Instead of
+  /// `takeOwnership(std::make_unique<SomeValue>(args))`, prefer
+  /// `create<SomeValue>(args)`.
+  ///
   /// Requirements:
   ///
   ///  `Val` must not be null.
   template <typename T>
-  std::enable_if_t<std::is_base_of<Value, T>::value, T &>
-  takeOwnership(std::unique_ptr<T> Val) {
+  LLVM_DEPRECATED("use create<T> instead", "")
+  std::enable_if_t<std::is_base_of<Value, T>::value, T &> takeOwnership(
+      std::unique_ptr<T> Val) {
     assert(Val != nullptr);
     Vals.push_back(std::move(Val));
     return *cast<T>(Vals.back().get());
   }
 
   /// Returns a new storage location appropriate for `Type`.
   ///
   /// A null `Type` is interpreted as the pointee type of `std::nullptr_t`.
public:
 
   /// Returns a symbolic boolean value that models a boolean literal equal to
   /// `Value`.
   AtomicBoolValue &getBoolLiteralValue(bool Value) const {
     return Value ? TrueVal : FalseVal;
   }
 
   /// Creates an atomic boolean value.
-  AtomicBoolValue &createAtomicBoolValue() {
-    return takeOwnership(std::make_unique<AtomicBoolValue>());
-  }
+  LLVM_DEPRECATED("use create<AtomicBoolValue> instead",
+                  "create<AtomicBoolValue>")
+  AtomicBoolValue &createAtomicBoolValue() { return create<AtomicBoolValue>(); }
 
   /// Creates a Top value for booleans. Each instance is unique and can be
   /// assigned a distinct truth value during solving.
   ///
   /// FIXME: `Top iff Top` is true when both Tops are identical (by pointer
   /// equality), but not when they are distinct values. We should improve the
   /// implementation so that `Top iff Top` has a consistent meaning, regardless
   /// of the identity of `Top`. Moreover, I think the meaning should be
   /// `false`.
-  TopBoolValue &createTopBoolValue() {
-    return takeOwnership(std::make_unique<TopBoolValue>());
-  }
+  LLVM_DEPRECATED("use create<TopBoolValue> instead", "create<TopBoolValue>")
+  TopBoolValue &createTopBoolValue() { return create<TopBoolValue>(); }
 
   /// Returns a boolean value that represents the conjunction of `LHS` and
   /// `RHS`. Subsequent calls with the same arguments, regardless of their
   /// order, will return the same result. If the given boolean values represent
   /// the same value, the result will be the value itself.
   BoolValue &getOrCreateConjunction(BoolValue &LHS, BoolValue &RHS);
 
   /// Returns a boolean value that represents the disjunction of `LHS` and

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

public:
   /// Equivalent to `getValue(getStorageLocation(D, SP), SkipPast::None)` if `D`
   /// is assigned a storage location in the environment, otherwise returns null.
   Value *getValue(const ValueDecl &D, SkipPast SP) const;
 
   /// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if `E`
   /// is assigned a storage location in the environment, otherwise returns null.
   Value *getValue(const Expr &E, SkipPast SP) const;
 
+  /// Creates a `T` (some subclass of `StorageLocation`), forwarding `args` to
+  /// the constructor, and returns a reference to it.
+  ///
+  /// The analysis context takes ownership of the created object. The object
+  /// will be destroyed when the analysis context is destroyed.
+  template <typename T, typename... Args>
+  std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
+  create(Args &&...args) {
+    return DACtx->create<T>(std::forward<Args>(args)...);
+  }
+
+  /// Creates a `T` (some subclass of `Value`), forwarding `args` to the
+  /// constructor, and returns a reference to it.
+  ///
+  /// The analysis context takes ownership of the created object. The object
+  /// will be destroyed when the analysis context is destroyed.
+  template <typename T, typename... Args>
+  std::enable_if_t<std::is_base_of<Value, T>::value, T &>
+  create(Args &&...args) {
+    return DACtx->create<T>(std::forward<Args>(args)...);
+  }
+
   /// Transfers ownership of `Loc` to the analysis context and returns a
   /// reference to it.
   ///
+  /// This function is deprecated. Instead of
+  /// `takeOwnership(std::make_unique<SomeStorageLocation>(args))`, prefer
+  /// `create<SomeStorageLocation>(args)`.
+  ///
   /// Requirements:
   ///
   ///  `Loc` must not be null.
   template <typename T>
-  std::enable_if_t<std::is_base_of<StorageLocation, T>::value, T &>
-  takeOwnership(std::unique_ptr<T> Loc) {
+  LLVM_DEPRECATED("use create<T> instead", "")
+  std::enable_if_t<std::is_base_of<StorageLocation, T>::value,
+                   T &> takeOwnership(std::unique_ptr<T> Loc) {
     return DACtx->takeOwnership(std::move(Loc));
   }
 
   /// Transfers ownership of `Val` to the analysis context and returns a
   /// reference to it.
   ///
+  /// This function is deprecated. Instead of
+  /// `takeOwnership(std::make_unique<SomeValue>(args))`, prefer
+  /// `create<SomeValue>(args)`.
+  ///
   /// Requirements:
   ///
   ///  `Val` must not be null.
   template <typename T>
-  std::enable_if_t<std::is_base_of<Value, T>::value, T &>
-  takeOwnership(std::unique_ptr<T> Val) {
+  LLVM_DEPRECATED("use create<T> instead", "")
+  std::enable_if_t<std::is_base_of<Value, T>::value, T &> takeOwnership(
+      std::unique_ptr<T> Val) {
     return DACtx->takeOwnership(std::move(Val));
   }
 
   /// Returns a symbolic boolean value that models a boolean literal equal to
   /// `Value`
   AtomicBoolValue &getBoolLiteralValue(bool Value) const {
     return DACtx->getBoolLiteralValue(Value);
   }

clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

if (!Type.isNull() &&
     // the allocation occurs, we can't apply that filter when performing
     // context-sensitive analysis. But, this only applies to storage locations,
     // since field access it not allowed to fail. In contrast, field *values*
     // don't need this allowance, since the API allows for uninitialized fields.
     auto Fields = Opts.ContextSensitiveOpts ? getObjectFields(Type)
                                             : getReferencedFields(Type);
     for (const FieldDecl *Field : Fields)
       FieldLocs.insert({Field, &createStorageLocation(Field->getType())});
-    return takeOwnership(
-        std::make_unique<AggregateStorageLocation>(Type, std::move(FieldLocs)));
+    return create<AggregateStorageLocation>(Type, std::move(FieldLocs));
   }
-  return takeOwnership(std::make_unique<ScalarStorageLocation>(Type));
+  return create<ScalarStorageLocation>(Type);
 }
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) {
   if (auto *Loc = getStorageLocation(D))
     return *Loc;
   auto &Loc = createStorageLocation(D.getType());
   setStorageLocation(D, Loc);

 
 PointerValue &
 DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
   auto CanonicalPointeeType =
       PointeeType.isNull() ? PointeeType : PointeeType.getCanonicalType();
   auto Res = NullPointerVals.try_emplace(CanonicalPointeeType, nullptr);
   if (Res.second) {
     auto &PointeeLoc = createStorageLocation(CanonicalPointeeType);
-    Res.first->second =
-        &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
+    Res.first->second = &create<PointerValue>(PointeeLoc);
   }
   return *Res.first->second;
 }
 
 static std::pair<BoolValue *, BoolValue *>
 makeCanonicalBoolValuePair(BoolValue &LHS, BoolValue &RHS) {
   auto Res = std::make_pair(&LHS, &RHS);
   if (&RHS < &LHS)
     std::swap(Res.first, Res.second);
   return Res;
 }
 
 BoolValue &DataflowAnalysisContext::getOrCreateConjunction(BoolValue &LHS,
                                                            BoolValue &RHS) {
   if (&LHS == &RHS)
     return LHS;
 
   auto Res = ConjunctionVals.try_emplace(makeCanonicalBoolValuePair(LHS, RHS),
                                          nullptr);
   if (Res.second)
-    Res.first->second =
-        &takeOwnership(std::make_unique<ConjunctionValue>(LHS, RHS));
+    Res.first->second = &create<ConjunctionValue>(LHS, RHS);
   return *Res.first->second;
 }
 
 BoolValue &DataflowAnalysisContext::getOrCreateDisjunction(BoolValue &LHS,
                                                            BoolValue &RHS) {
   if (&LHS == &RHS)
     return LHS;
 
   auto Res = DisjunctionVals.try_emplace(makeCanonicalBoolValuePair(LHS, RHS),
                                          nullptr);
   if (Res.second)
-    Res.first->second =
-        &takeOwnership(std::make_unique<DisjunctionValue>(LHS, RHS));
+    Res.first->second = &create<DisjunctionValue>(LHS, RHS);
   return *Res.first->second;
 }
 
 BoolValue &DataflowAnalysisContext::getOrCreateNegation(BoolValue &Val) {
   auto Res = NegationVals.try_emplace(&Val, nullptr);
   if (Res.second)
-    Res.first->second = &takeOwnership(std::make_unique<NegationValue>(Val));
+    Res.first->second = &create<NegationValue>(Val);
   return *Res.first->second;
 }
 
 BoolValue &DataflowAnalysisContext::getOrCreateImplication(BoolValue &LHS,
                                                            BoolValue &RHS) {
   if (&LHS == &RHS)
     return getBoolLiteralValue(true);
 
   auto Res = ImplicationVals.try_emplace(std::make_pair(&LHS, &RHS), nullptr);
   if (Res.second)
-    Res.first->second =
-        &takeOwnership(std::make_unique<ImplicationValue>(LHS, RHS));
+    Res.first->second = &create<ImplicationValue>(LHS, RHS);
   return *Res.first->second;
 }
 
 BoolValue &DataflowAnalysisContext::getOrCreateIff(BoolValue &LHS,
                                                    BoolValue &RHS) {
   if (&LHS == &RHS)
     return getBoolLiteralValue(true);
 
   auto Res = BiconditionalVals.try_emplace(makeCanonicalBoolValuePair(LHS, RHS),
                                            nullptr);
   if (Res.second)
-    Res.first->second =
-        &takeOwnership(std::make_unique<BiconditionalValue>(LHS, RHS));
+    Res.first->second = &create<BiconditionalValue>(LHS, RHS);
   return *Res.first->second;
 }
 
 AtomicBoolValue &DataflowAnalysisContext::makeFlowConditionToken() {
   return createAtomicBoolValue();
 }
 
 void DataflowAnalysisContext::addFlowConditionConstraint(

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (ArgLoc == nullptr)
       continue;
 
     const VarDecl *Param = *ParamIt;
     auto &Loc = createStorageLocation(*Param);
     setStorageLocation(*Param, Loc);
 
     QualType ParamType = Param->getType();
     if (ParamType->isReferenceType()) {
-      auto &Val = takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
+      auto &Val = create<ReferenceValue>(*ArgLoc);
       setValue(Loc, Val);
     } else if (auto *ArgVal = getValue(*ArgLoc)) {
       setValue(Loc, *ArgVal);
     } else if (Value *Val = createValue(ParamType)) {
       setValue(Loc, *Val);
     }
   }
 }
if (Type->isBooleanType()) {
     return &makeAtomicBoolValue();
   }
 
   if (Type->isIntegerType()) {
     // FIXME: consider instead `return nullptr`, given that we do nothing useful
     // with integers, and so distinguishing them serves no purpose, but could
     // prevent convergence.
     CreatedValuesCount++;
-    return &takeOwnership(std::make_unique<IntegerValue>());
+    return &create<IntegerValue>();
   }
 
   if (Type->isReferenceType()) {
     CreatedValuesCount++;
     QualType PointeeType = Type->castAs<ReferenceType>()->getPointeeType();
     auto &PointeeLoc = createStorageLocation(PointeeType);
 
     if (Visited.insert(PointeeType.getCanonicalType()).second) {
       Value *PointeeVal = createValueUnlessSelfReferential(
           PointeeType, Visited, Depth, CreatedValuesCount);
       Visited.erase(PointeeType.getCanonicalType());
 
       if (PointeeVal != nullptr)
         setValue(PointeeLoc, *PointeeVal);
     }
 
-    return &takeOwnership(std::make_unique<ReferenceValue>(PointeeLoc));
+    return &create<ReferenceValue>(PointeeLoc);
   }
 
   if (Type->isPointerType()) {
     CreatedValuesCount++;
     QualType PointeeType = Type->castAs<PointerType>()->getPointeeType();
     auto &PointeeLoc = createStorageLocation(PointeeType);
 
     if (Visited.insert(PointeeType.getCanonicalType()).second) {
       Value *PointeeVal = createValueUnlessSelfReferential(
           PointeeType, Visited, Depth, CreatedValuesCount);
       Visited.erase(PointeeType.getCanonicalType());
 
       if (PointeeVal != nullptr)
         setValue(PointeeLoc, *PointeeVal);
     }
 
-    return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
+    return &create<PointerValue>(PointeeLoc);
   }
 
   if (Type->isStructureOrClassType() || Type->isUnionType()) {
     CreatedValuesCount++;
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
     for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
       assert(Field != nullptr);
 
       QualType FieldType = Field->getType();
       if (Visited.contains(FieldType.getCanonicalType()))
         continue;
 
       Visited.insert(FieldType.getCanonicalType());
       if (auto *FieldValue = createValueUnlessSelfReferential(
               FieldType, Visited, Depth + 1, CreatedValuesCount))
         FieldValues.insert({Field, FieldValue});
       Visited.erase(FieldType.getCanonicalType());
     }
 
-    return &takeOwnership(
-        std::make_unique<StructValue>(std::move(FieldValues)));
+    return &create<StructValue>(std::move(FieldValues));
   }
 
   return nullptr;
 }
 
 StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const {
   switch (SP) {
   case SkipPast::None:

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

 /// property of the optional value `OptionalVal`.
 void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {
   OptionalVal.setProperty("has_value", HasValueVal);
 }
 
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
-  auto OptionalVal = std::make_unique<StructValue>();
-  setHasValue(*OptionalVal, HasValueVal);
-  return Env.takeOwnership(std::move(OptionalVal));
+  auto &OptionalVal = Env.create<StructValue>();
+  setHasValue(OptionalVal, HasValueVal);
+  return OptionalVal;
 }
 
 /// Returns the symbolic value that represents the "has_value" property of the
 /// optional value `OptionalVal`. Returns null if `OptionalVal` is null.
 BoolValue *getHasValue(Environment &Env, Value *OptionalVal) {
   if (OptionalVal != nullptr) {
     auto *HasValueVal =
         cast_or_null<BoolValue>(OptionalVal->getProperty("has_value"));
StorageLocation *maybeInitializeOptionalValueMember(QualType Q,
   }
 
   auto Ty = stripReference(Q);
   auto *ValueVal = Env.createValue(Ty);
   if (ValueVal == nullptr)
     return nullptr;
   auto &ValueLoc = Env.createStorageLocation(Ty);
   Env.setValue(ValueLoc, *ValueVal);
-  auto ValueRef = std::make_unique<ReferenceValue>(ValueLoc);
-  OptionalVal.setProperty("value", Env.takeOwnership(std::move(ValueRef)));
+  auto &ValueRef = Env.create<ReferenceValue>(ValueLoc);
+  OptionalVal.setProperty("value", ValueRef);
   return &ValueLoc;
 }
 
 void initializeOptionalReference(const Expr *OptionalExpr,
                                  const MatchFinder::MatchResult &,
                                  LatticeTransferState &State) {
   if (auto *OptionalVal =
           State.Env.getValue(*OptionalExpr, SkipPast::Reference)) {

clang/lib/Analysis/FlowSensitive/Transfer.cpp

if (isa_and_nonnull<ReferenceValue>(Env.getValue(*DeclLoc))) {
       // depend on its truth to proceed. But, it verifies our assumptions,
       // which, if violated, probably indicate a problem elsewhere.
       assert((VD->getType()->isReferenceType() || isa<BindingDecl>(VD)) &&
              "Only reference-typed declarations or `BindingDecl`s should map "
              "to `ReferenceValue`s");
       Env.setStorageLocation(*S, *DeclLoc);
     } else {
       auto &Loc = Env.createStorageLocation(*S);
-      auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*DeclLoc));
+      auto &Val = Env.create<ReferenceValue>(*DeclLoc);
       Env.setStorageLocation(*S, Loc);
       Env.setValue(Loc, Val);
     }
   }
 
   void VisitDeclStmt(const DeclStmt *S) {
     // Group decls are converted into single decls in the CFG so the cast below
     // is safe.
void VisitDeclStmt(const DeclStmt *S) {
     }
 
     if (D.getType()->isReferenceType()) {
       // Initializing a reference variable - do not create a reference to
       // reference.
       // FIXME: reuse the ReferenceValue instead of creating a new one.
       if (auto *InitExprLoc =
               Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
-        auto &Val =
-            Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
+        auto &Val = Env.create<ReferenceValue>(*InitExprLoc);
         Env.setValue(Loc, Val);
       }
     } else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
       Env.setValue(Loc, *InitExprVal);
     }
 
     if (Env.getValue(Loc) == nullptr) {
       // We arrive here in (the few) cases where an expression is intentionally
case UO_Deref: {
       // Skip past a reference to handle dereference of a dependent pointer.
       const auto *SubExprVal = cast_or_null<PointerValue>(
           Env.getValue(*SubExpr, SkipPast::Reference));
       if (SubExprVal == nullptr)
         break;
 
       auto &Loc = Env.createStorageLocation(*S);
       Env.setStorageLocation(*S, Loc);
-      Env.setValue(Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(
-                            SubExprVal->getPointeeLoc())));
+      Env.setValue(Loc,
+                   Env.create<ReferenceValue>(SubExprVal->getPointeeLoc()));
       break;
     }
     case UO_AddrOf: {
       // Do not form a pointer to a reference. If `SubExpr` is assigned a
       // `ReferenceValue` then form a value that points to the location of its
       // pointee.
       StorageLocation *PointeeLoc =
           Env.getStorageLocation(*SubExpr, SkipPast::Reference);
       if (PointeeLoc == nullptr)
         break;
 
       auto &PointerLoc = Env.createStorageLocation(*S);
-      auto &PointerVal =
-          Env.takeOwnership(std::make_unique<PointerValue>(*PointeeLoc));
+      auto &PointerVal = Env.create<PointerValue>(*PointeeLoc);
       Env.setStorageLocation(*S, PointerLoc);
       Env.setValue(PointerLoc, PointerVal);
       break;
     }
     case UO_LNot: {
       auto *SubExprVal =
           dyn_cast_or_null<BoolValue>(Env.getValue(*SubExpr, SkipPast::None));
       if (SubExprVal == nullptr)
void VisitCXXThisExpr(const CXXThisExpr *S) {
     auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
     if (ThisPointeeLoc == nullptr)
       // Unions are not supported yet, and will not have a location for the
       // `this` expression's pointee.
       return;
 
     auto &Loc = Env.createStorageLocation(*S);
     Env.setStorageLocation(*S, Loc);
-    Env.setValue(Loc, Env.takeOwnership(
-                          std::make_unique<PointerValue>(*ThisPointeeLoc)));
+    Env.setValue(Loc, Env.create<PointerValue>(*ThisPointeeLoc));
   }
 
   void VisitReturnStmt(const ReturnStmt *S) {
     if (!Env.getAnalysisOptions().ContextSensitiveOpts)
       return;
 
     auto *Ret = S->getRetValue();
     if (Ret == nullptr)
if (auto *D = dyn_cast<VarDecl>(Member)) {
 
         if (VarDeclLoc->getType()->isReferenceType()) {
           assert(isa_and_nonnull<ReferenceValue>(Env.getValue((*VarDeclLoc))) &&
                  "reference-typed declarations map to `ReferenceValue`s");
           Env.setStorageLocation(*S, *VarDeclLoc);
         } else {
           auto &Loc = Env.createStorageLocation(*S);
           Env.setStorageLocation(*S, Loc);
-          Env.setValue(Loc, Env.takeOwnership(
-                                std::make_unique<ReferenceValue>(*VarDeclLoc)));
+          Env.setValue(Loc, Env.create<ReferenceValue>(*VarDeclLoc));
         }
         return;
       }
     }
 
     // The receiver can be either a value or a pointer to a value. Skip past the
     // indirection to handle both cases.
     auto *BaseLoc = cast_or_null<AggregateStorageLocation>(
if (MemberLoc.getType()->isReferenceType()) {
       if (auto *V = Env.getValue(MemberLoc)) {
         assert(isa<ReferenceValue>(V) &&
                "reference-typed declarations map to `ReferenceValue`s");
         Env.setStorageLocation(*S, MemberLoc);
       }
     } else {
       auto &Loc = Env.createStorageLocation(*S);
       Env.setStorageLocation(*S, Loc);
-      Env.setValue(
-          Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(MemberLoc)));
+      Env.setValue(Loc, Env.create<ReferenceValue>(MemberLoc));
     }
   }
 
   void VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *S) {
     const Expr *InitExpr = S->getExpr();
     assert(InitExpr != nullptr);
 
     Value *InitExprVal = Env.getValue(*InitExpr, SkipPast::None);

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

if (InitStmtLoc == nullptr)
     return;
 
   auto *InitStmtVal = Env.getValue(*InitStmtLoc);
   if (InitStmtVal == nullptr)
     return;
 
   if (Member->getType()->isReferenceType()) {
     auto &MemberLoc = ThisLoc.getChild(*Member);
-    Env.setValue(MemberLoc, Env.takeOwnership(std::make_unique<ReferenceValue>(
-                                *InitStmtLoc)));
+    Env.setValue(MemberLoc, Env.create<ReferenceValue>(*InitStmtLoc));
   } else {
     auto &MemberLoc = ThisLoc.getChild(*Member);
     Env.setValue(MemberLoc, *InitStmtVal);
   }
 }
 
 void builtinTransfer(const CFGElement &Elt,
                      TypeErasedDataflowAnalysisState &State,