[clang][nullability] Remove `RecordValue`. (#89052)

This class no longer serves any purpose; see also the discussion here:
https://reviews.llvm.org/D155204#inline-1503204

A lot of existing tests in TransferTest.cpp check for the existence of
`RecordValue`s. Some of these checks are now simply redundant and have
been
removed. In other cases, tests were checking for the existence of a
`RecordValue` as a way of testing whether a record has been initialized.
I have
typically changed these test to instead check whether a field of the
record has
a value.

13 files changed, 130 insertions, 396 deletions

diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 4277792..d50dba3 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -84,7 +84,7 @@ public:
     virtual ComparisonResult compare(QualType Type, const Value &Val1,
                                      const Environment &Env1, const Value &Val2,
                                      const Environment &Env2) {
-      // FIXME: Consider adding QualType to RecordValue and removing the Type
+      // FIXME: Consider adding `QualType` to `Value` and removing the `Type`
       // argument here.
       return ComparisonResult::Unknown;
     }
@@ -407,20 +407,15 @@ public:
   /// storage locations and values for indirections until it finds a
   /// non-pointer/non-reference type.
   ///
-  /// If `Type` is a class, struct, or union type, creates values for all
-  /// modeled fields (including synthetic fields) and calls `setValue()` to
-  /// associate the `RecordValue` with its storage location
-  /// (`RecordValue::getLoc()`).
-  ///
   /// If `Type` is one of the following types, this function will always return
   /// a non-null pointer:
   /// - `bool`
   /// - Any integer type
-  /// - Any class, struct, or union type
   ///
   /// Requirements:
   ///
-  ///  `Type` must not be null.
+  ///  - `Type` must not be null.
+  ///  - `Type` must not be a reference type or record type.
   Value *createValue(QualType Type);
 
   /// Creates an object (i.e. a storage location with an associated value) of
@@ -452,6 +447,7 @@ public:
   /// Initializes the fields (including synthetic fields) of `Loc` with values,
   /// unless values of the field type are not supported or we hit one of the
   /// limits at which we stop producing values.
+  /// If a field already has a value, that value is preserved.
   /// If `Type` is provided, initializes only those fields that are modeled for
   /// `Type`; this is intended for use in cases where `Loc` is a derived type
   /// and we only want to initialize the fields of a base type.
@@ -461,6 +457,10 @@ public:
   }
 
   /// Assigns `Val` as the value of `Loc` in the environment.
+  ///
+  /// Requirements:
+  ///
+  ///  `Loc` must not be a `RecordStorageLocation`.
   void setValue(const StorageLocation &Loc, Value &Val);
 
   /// Clears any association between `Loc` and a value in the environment.
@@ -470,20 +470,24 @@ public:
   ///
   /// Requirements:
   ///
-  ///  - `E` must be a prvalue
-  ///  - If `Val` is a `RecordValue`, its `RecordStorageLocation` must be
-  ///    `getResultObjectLocation(E)`. An exception to this is if `E` is an
-  ///    expression that originally creates a `RecordValue` (such as a
-  ///    `CXXConstructExpr` or `CallExpr`), as these establish the location of
-  ///    the result object in the first place.
+  ///  - `E` must be a prvalue.
+  ///  - `E` must not have record type.
   void setValue(const Expr &E, Value &Val);
 
   /// Returns the value assigned to `Loc` in the environment or null if `Loc`
   /// isn't assigned a value in the environment.
+  ///
+  /// Requirements:
+  ///
+  ///  `Loc` must not be a `RecordStorageLocation`.
   Value *getValue(const StorageLocation &Loc) const;
 
   /// Equivalent to `getValue(getStorageLocation(D))` if `D` is assigned a
   /// storage location in the environment, otherwise returns null.
+  ///
+  /// Requirements:
+  ///
+  ///  `D` must not have record type.
   Value *getValue(const ValueDecl &D) const;
 
   /// Equivalent to `getValue(getStorageLocation(E, SP))` if `E` is assigned a
@@ -775,12 +779,6 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
 RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
                                              const Environment &Env);
 
-/// Associates a new `RecordValue` with `Loc` and returns the new value.
-RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env);
-
-/// Associates a new `RecordValue` with `Expr` and returns the new value.
-RecordValue &refreshRecordValue(const Expr &Expr, Environment &Env);
-
 } // namespace dataflow
 } // namespace clang
 
diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h
index be1bf93..97efa3a 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Value.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -35,7 +35,6 @@ public:
   enum class Kind {
     Integer,
     Pointer,
-    Record,
 
     // TODO: Top values should not be need to be type-specific.
     TopBool,
@@ -67,7 +66,6 @@ public:
   /// Properties may not be set on `RecordValue`s; use synthetic fields instead
   /// (for details, see documentation for `RecordStorageLocation`).
   void setProperty(llvm::StringRef Name, Value &Val) {
-    assert(getKind() != Kind::Record);
     Properties.insert_or_assign(Name, &Val);
   }
 
@@ -184,45 +182,6 @@ private:
   StorageLocation &PointeeLoc;
 };
 
-/// Models a value of `struct` or `class` type.
-/// In C++, prvalues of class type serve only a limited purpose: They can only
-/// be used to initialize a result object. It is not possible to access member
-/// variables or call member functions on a prvalue of class type.
-/// Correspondingly, `RecordValue` also serves only a limited purpose: It
-/// conveys a prvalue of class type from the place where the object is
-/// constructed to the result object that it initializes.
-///
-/// When creating a prvalue of class type, we already need a storage location
-/// for `this`, even though prvalues are otherwise not associated with storage
-/// locations. `RecordValue` is therefore essentially a wrapper for a storage
-/// location, which is then used to set the storage location for the result
-/// object when we process the AST node for that result object.
-///
-/// For example:
-///    MyStruct S = MyStruct(3);
-///
-/// In this example, `MyStruct(3) is a prvalue, which is modeled as a
-/// `RecordValue` that wraps a `RecordStorageLocation`. This
-/// `RecordStorageLocation` is then used as the storage location for `S`.
-///
-/// Over time, we may eliminate `RecordValue` entirely. See also the discussion
-/// here: https://reviews.llvm.org/D155204#inline-1503204
-class RecordValue final : public Value {
-public:
-  explicit RecordValue(RecordStorageLocation &Loc)
-      : Value(Kind::Record), Loc(Loc) {}
-
-  static bool classof(const Value *Val) {
-    return Val->getKind() == Kind::Record;
-  }
-
-  /// Returns the storage location that this `RecordValue` is associated with.
-  RecordStorageLocation &getLoc() const { return Loc; }
-
-private:
-  RecordStorageLocation &Loc;
-};
-
 raw_ostream &operator<<(raw_ostream &OS, const Value &Val);
 
 } // namespace dataflow
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 1387734..6998e6d 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -24,6 +24,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <utility>
@@ -80,7 +81,6 @@ static bool equateUnknownValues(Value::Kind K) {
   switch (K) {
   case Value::Kind::Integer:
   case Value::Kind::Pointer:
-  case Value::Kind::Record:
     return true;
   default:
     return false;
@@ -145,25 +145,7 @@ static Value *joinDistinctValues(QualType Type, Value &Val1,
     return &A.makeBoolValue(JoinedVal);
   }
 
-  Value *JoinedVal = nullptr;
-  if (auto *RecordVal1 = dyn_cast<RecordValue>(&Val1)) {
-    auto *RecordVal2 = cast<RecordValue>(&Val2);
-
-    if (&RecordVal1->getLoc() == &RecordVal2->getLoc())
-      // `RecordVal1` and `RecordVal2` may have different properties associated
-      // with them. Create a new `RecordValue` with the same location but
-      // without any properties so that we soundly approximate both values. If a
-      // particular analysis needs to join properties, it should do so in
-      // `DataflowAnalysis::join()`.
-      JoinedVal = &JoinedEnv.create<RecordValue>(RecordVal1->getLoc());
-    else
-      // If the locations for the two records are different, need to create a
-      // completely new value.
-      JoinedVal = JoinedEnv.createValue(Type);
-  } else {
-    JoinedVal = JoinedEnv.createValue(Type);
-  }
-
+  Value *JoinedVal = JoinedEnv.createValue(Type);
   if (JoinedVal)
     Model.join(Type, Val1, Env1, Val2, Env2, *JoinedVal, JoinedEnv);
 
@@ -565,7 +547,6 @@ void Environment::initialize() {
       auto &ThisLoc =
           cast<RecordStorageLocation>(createStorageLocation(ThisPointeeType));
       setThisPointeeStorageLocation(ThisLoc);
-      refreshRecordValue(ThisLoc, *this);
       // Initialize fields of `*this` with values, but only if we're not
       // analyzing a constructor; after all, it's the constructor's job to do
       // this (and we want to be able to test that).
@@ -709,10 +690,6 @@ void Environment::popCall(const CXXConstructExpr *Call,
   // See also comment in `popCall(const CallExpr *, const Environment &)` above.
   this->LocToVal = std::move(CalleeEnv.LocToVal);
   this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
-
-  if (Value *Val = CalleeEnv.getValue(*CalleeEnv.ThisPointeeLoc)) {
-    setValue(*Call, *Val);
-  }
 }
 
 bool Environment::equivalentTo(const Environment &Other,
@@ -936,24 +913,23 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
 }
 
 void Environment::setValue(const StorageLocation &Loc, Value &Val) {
-  assert(!isa<RecordValue>(&Val) || &cast<RecordValue>(&Val)->getLoc() == &Loc);
-
+  // Records should not be associated with values.
+  assert(!isa<RecordStorageLocation>(Loc));
   LocToVal[&Loc] = &Val;
 }
 
 void Environment::setValue(const Expr &E, Value &Val) {
   const Expr &CanonE = ignoreCFGOmittedNodes(E);
 
-  if (auto *RecordVal = dyn_cast<RecordValue>(&Val)) {
-    assert(&RecordVal->getLoc() == &getResultObjectLocation(CanonE));
-    (void)RecordVal;
-  }
-
   assert(CanonE.isPRValue());
+  // Records should not be associated with values.
+  assert(!CanonE.getType()->isRecordType());
   ExprToVal[&CanonE] = &Val;
 }
 
 Value *Environment::getValue(const StorageLocation &Loc) const {
+  // Records should not be associated with values.
+  assert(!isa<RecordStorageLocation>(Loc));
   return LocToVal.lookup(&Loc);
 }
 
@@ -965,6 +941,9 @@ Value *Environment::getValue(const ValueDecl &D) const {
 }
 
 Value *Environment::getValue(const Expr &E) const {
+  // Records should not be associated with values.
+  assert(!E.getType()->isRecordType());
+
   if (E.isPRValue()) {
     auto It = ExprToVal.find(&ignoreCFGOmittedNodes(E));
     return It == ExprToVal.end() ? nullptr : It->second;
@@ -993,6 +972,7 @@ Value *Environment::createValueUnlessSelfReferential(
     int &CreatedValuesCount) {
   assert(!Type.isNull());
   assert(!Type->isReferenceType());
+  assert(!Type->isRecordType());
 
   // Allow unlimited fields at depth 1; only cap at deeper nesting levels.
   if ((Depth > 1 && CreatedValuesCount > MaxCompositeValueSize) ||
@@ -1021,15 +1001,6 @@ Value *Environment::createValueUnlessSelfReferential(
     return &arena().create<PointerValue>(PointeeLoc);
   }
 
-  if (Type->isRecordType()) {
-    CreatedValuesCount++;
-    auto &Loc = cast<RecordStorageLocation>(createStorageLocation(Type));
-    initializeFieldsWithValues(Loc, Loc.getType(), Visited, Depth,
-                               CreatedValuesCount);
-
-    return &refreshRecordValue(Loc, *this);
-  }
-
   return nullptr;
 }
 
@@ -1039,20 +1010,23 @@ Environment::createLocAndMaybeValue(QualType Ty,
                                     int Depth, int &CreatedValuesCount) {
   if (!Visited.insert(Ty.getCanonicalType()).second)
     return createStorageLocation(Ty.getNonReferenceType());
-  Value *Val = createValueUnlessSelfReferential(
-      Ty.getNonReferenceType(), Visited, Depth, CreatedValuesCount);
-  Visited.erase(Ty.getCanonicalType());
+  auto EraseVisited = llvm::make_scope_exit(
+      [&Visited, Ty] { Visited.erase(Ty.getCanonicalType()); });
 
   Ty = Ty.getNonReferenceType();
 
-  if (Val == nullptr)
-    return createStorageLocation(Ty);
-
-  if (Ty->isRecordType())
-    return cast<RecordValue>(Val)->getLoc();
+  if (Ty->isRecordType()) {
+    auto &Loc = cast<RecordStorageLocation>(createStorageLocation(Ty));
+    initializeFieldsWithValues(Loc, Ty, Visited, Depth, CreatedValuesCount);
+    return Loc;
+  }
 
   StorageLocation &Loc = createStorageLocation(Ty);
-  setValue(Loc, *Val);
+
+  if (Value *Val = createValueUnlessSelfReferential(Ty, Visited, Depth,
+                                                    CreatedValuesCount))
+    setValue(Loc, *Val);
+
   return Loc;
 }
 
@@ -1064,10 +1038,11 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
   auto initField = [&](QualType FieldType, StorageLocation &FieldLoc) {
     if (FieldType->isRecordType()) {
       auto &FieldRecordLoc = cast<RecordStorageLocation>(FieldLoc);
-      setValue(FieldRecordLoc, create<RecordValue>(FieldRecordLoc));
       initializeFieldsWithValues(FieldRecordLoc, FieldRecordLoc.getType(),
                                  Visited, Depth + 1, CreatedValuesCount);
     } else {
+      if (getValue(FieldLoc) != nullptr)
+        return;
       if (!Visited.insert(FieldType.getCanonicalType()).second)
         return;
       if (Value *Val = createValueUnlessSelfReferential(
@@ -1125,7 +1100,6 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D,
     auto &RecordLoc = cast<RecordStorageLocation>(Loc);
     if (!InitExpr)
       initializeFieldsWithValues(RecordLoc);
-    refreshRecordValue(RecordLoc, *this);
   } else {
     Value *Val = nullptr;
     if (InitExpr)
@@ -1264,25 +1238,5 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
   return Env.get<RecordStorageLocation>(*Base);
 }
 
-RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env) {
-  auto &NewVal = Env.create<RecordValue>(Loc);
-  Env.setValue(Loc, NewVal);
-  return NewVal;
-}
-
-RecordValue &refreshRecordValue(const Expr &Expr, Environment &Env) {
-  assert(Expr.getType()->isRecordType());
-
-  if (Expr.isPRValue())
-    refreshRecordValue(Env.getResultObjectLocation(Expr), Env);
-
-  if (auto *Loc = Env.get<RecordStorageLocation>(Expr))
-    refreshRecordValue(*Loc, Env);
-
-  auto &NewVal = *cast<RecordValue>(Env.createValue(Expr.getType()));
-  Env.setStorageLocation(Expr, NewVal.getLoc());
-  return NewVal;
-}
-
 } // namespace dataflow
 } // namespace clang
diff --git a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
index 573c4b1..d40aab7 100644
--- a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -28,8 +28,6 @@ llvm::StringRef debugString(Value::Kind Kind) {
     return "Integer";
   case Value::Kind::Pointer:
     return "Pointer";
-  case Value::Kind::Record:
-    return "Record";
   case Value::Kind::AtomicBool:
     return "AtomicBool";
   case Value::Kind::TopBool:
diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
index 397a8d8..a36cb41 100644
--- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -95,7 +95,6 @@ public:
 
     switch (V.getKind()) {
     case Value::Kind::Integer:
-    case Value::Kind::Record:
     case Value::Kind::TopBool:
     case Value::Kind::AtomicBool:
     case Value::Kind::FormulaBool:
@@ -126,8 +125,9 @@ public:
       return;
 
     JOS.attribute("type", L.getType().getAsString());
-    if (auto *V = Env.getValue(L))
-      dump(*V);
+    if (!L.getType()->isRecordType())
+      if (auto *V = Env.getValue(L))
+        dump(*V);
 
     if (auto *RLoc = dyn_cast<RecordStorageLocation>(&L)) {
       for (const auto &Child : RLoc->children())
@@ -281,9 +281,10 @@ public:
             Iters.back().Block->Elements[ElementIndex - 1].getAs<CFGStmt>();
         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); });
+            if (!E->getType()->isRecordType())
+              if (auto *V = State.Env.getValue(*E))
+                JOS.attributeObject(
+                    "value", [&] { ModelDumper(JOS, State.Env).dump(*V); });
           } else {
             if (auto *Loc = State.Env.getStorageLocation(*E))
               JOS.attributeObject(
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index cadb1ce..0707aa6 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -339,17 +339,6 @@ void setHasValue(RecordStorageLocation &OptionalLoc, BoolValue &HasValueVal,
   Env.setValue(locForHasValue(OptionalLoc), HasValueVal);
 }
 
-/// Creates a symbolic value for an `optional` value at an existing storage
-/// location. Uses `HasValueVal` as the symbolic value of the "has_value"
-/// property.
-RecordValue &createOptionalValue(RecordStorageLocation &Loc,
-                                 BoolValue &HasValueVal, Environment &Env) {
-  auto &OptionalVal = Env.create<RecordValue>(Loc);
-  Env.setValue(Loc, OptionalVal);
-  setHasValue(Loc, HasValueVal, Env);
-  return OptionalVal;
-}
-
 /// Returns the symbolic value that represents the "has_value" property of the
 /// optional at `OptionalLoc`. Returns null if `OptionalLoc` is null.
 BoolValue *getHasValue(Environment &Env, RecordStorageLocation *OptionalLoc) {
@@ -413,9 +402,8 @@ void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
 void transferMakeOptionalCall(const CallExpr *E,
                               const MatchFinder::MatchResult &,
                               LatticeTransferState &State) {
-  State.Env.setValue(
-      *E, createOptionalValue(State.Env.getResultObjectLocation(*E),
-                              State.Env.getBoolLiteralValue(true), State.Env));
+  setHasValue(State.Env.getResultObjectLocation(*E),
+              State.Env.getBoolLiteralValue(true), State.Env);
 }
 
 void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
@@ -483,9 +471,6 @@ void transferValueOrNotEqX(const Expr *ComparisonExpr,
 void transferCallReturningOptional(const CallExpr *E,
                                    const MatchFinder::MatchResult &Result,
                                    LatticeTransferState &State) {
-  if (State.Env.getValue(*E) != nullptr)
-    return;
-
   RecordStorageLocation *Loc = nullptr;
   if (E->isPRValue()) {
     Loc = &State.Env.getResultObjectLocation(*E);
@@ -497,16 +482,16 @@ void transferCallReturningOptional(const CallExpr *E,
     }
   }
 
-  RecordValue &Val =
-      createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
-  if (E->isPRValue())
-    State.Env.setValue(*E, Val);
+  if (State.Env.getValue(locForHasValue(*Loc)) != nullptr)
+    return;
+
+  setHasValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
 }
 
 void constructOptionalValue(const Expr &E, Environment &Env,
                             BoolValue &HasValueVal) {
   RecordStorageLocation &Loc = Env.getResultObjectLocation(E);
-  Env.setValue(E, createOptionalValue(Loc, HasValueVal, Env));
+  setHasValue(Loc, HasValueVal, Env);
 }
 
 /// Returns a symbolic value for the "has_value" property of an `optional<T>`
@@ -555,7 +540,7 @@ void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal,
   assert(E->getNumArgs() > 0);
 
   if (auto *Loc = State.Env.get<RecordStorageLocation>(*E->getArg(0))) {
-    createOptionalValue(*Loc, HasValueVal, State.Env);
+    setHasValue(*Loc, HasValueVal, State.Env);
 
     // Assign a storage location for the whole expression.
     State.Env.setStorageLocation(*E, *Loc);
@@ -587,11 +572,11 @@ void transferSwap(RecordStorageLocation *Loc1, RecordStorageLocation *Loc2,
 
   if (Loc1 == nullptr) {
     if (Loc2 != nullptr)
-      createOptionalValue(*Loc2, Env.makeAtomicBoolValue(), Env);
+      setHasValue(*Loc2, Env.makeAtomicBoolValue(), Env);
     return;
   }
   if (Loc2 == nullptr) {
-    createOptionalValue(*Loc1, Env.makeAtomicBoolValue(), Env);
+    setHasValue(*Loc1, Env.makeAtomicBoolValue(), Env);
     return;
   }
 
@@ -609,8 +594,8 @@ void transferSwap(RecordStorageLocation *Loc1, RecordStorageLocation *Loc2,
   if (BoolVal2 == nullptr)
     BoolVal2 = &Env.makeAtomicBoolValue();
 
-  createOptionalValue(*Loc1, *BoolVal2, Env);
-  createOptionalValue(*Loc2, *BoolVal1, Env);
+  setHasValue(*Loc1, *BoolVal2, Env);
+  setHasValue(*Loc2, *BoolVal1, Env);
 }
 
 void transferSwapCall(const CXXMemberCallExpr *E,
@@ -806,8 +791,7 @@ auto buildTransferMatchSwitch() {
              LatticeTransferState &State) {
             if (RecordStorageLocation *Loc =
                     getImplicitObjectLocation(*E, State.Env)) {
-              createOptionalValue(*Loc, State.Env.getBoolLiteralValue(true),
-                                  State.Env);
+              setHasValue(*Loc, State.Env.getBoolLiteralValue(true), State.Env);
             }
           })
 
@@ -818,8 +802,8 @@ auto buildTransferMatchSwitch() {
              LatticeTransferState &State) {
             if (RecordStorageLocation *Loc =
                     getImplicitObjectLocation(*E, State.Env)) {
-              createOptionalValue(*Loc, State.Env.getBoolLiteralValue(false),
-                                  State.Env);
+              setHasValue(*Loc, State.Env.getBoolLiteralValue(false),
+                          State.Env);
             }
           })
 
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index 2f0b0e5..b840123 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -83,9 +83,6 @@ void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
       copySyntheticField(SrcFieldLoc->getType(), *SrcFieldLoc,
                          Dst.getSyntheticField(Name), Env);
   }
-
-  RecordValue *DstVal = &Env.create<RecordValue>(Dst);
-  Env.setValue(Dst, *DstVal);
 }
 
 bool recordsEqual(const RecordStorageLocation &Loc1, const Environment &Env1,
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 1e03477..2771c8b 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -96,6 +96,8 @@ static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
 }
 
 static void propagateValue(const Expr &From, const Expr &To, Environment &Env) {
+  if (From.getType()->isRecordType())
+    return;
   if (auto *Val = Env.getValue(From))
     Env.setValue(To, *Val);
 }
@@ -403,6 +405,9 @@ public:
       return;
 
     if (Ret->isPRValue()) {
+      if (Ret->getType()->isRecordType())
+        return;
+
       auto *Val = Env.getValue(*Ret);
       if (Val == nullptr)
         return;
@@ -457,15 +462,9 @@ public:
     assert(ArgExpr != nullptr);
     propagateValueOrStorageLocation(*ArgExpr, *S, Env);
 
-    // If this is a prvalue of record type, we consider it to be an "original
-    // record constructor", which we always require to have a `RecordValue`.
-    // So make sure we have a value if we didn't propagate one above.
     if (S->isPRValue() && S->getType()->isRecordType()) {
-      if (Env.getValue(*S) == nullptr) {
-        auto &Loc = Env.getResultObjectLocation(*S);
-        Env.initializeFieldsWithValues(Loc);
-        refreshRecordValue(Loc, Env);
-      }
+      auto &Loc = Env.getResultObjectLocation(*S);
+      Env.initializeFieldsWithValues(Loc);
     }
   }
 
@@ -495,7 +494,6 @@ public:
     }
 
     RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
-    Env.setValue(*S, refreshRecordValue(Loc, Env));
 
     if (ConstructorDecl->isCopyOrMoveConstructor()) {
       // It is permissible for a copy/move constructor to have additional
@@ -542,8 +540,7 @@ public:
 
       RecordStorageLocation *LocSrc = nullptr;
       if (Arg1->isPRValue()) {
-        if (auto *Val = Env.get<RecordValue>(*Arg1))
-          LocSrc = &Val->getLoc();
+        LocSrc = &Env.getResultObjectLocation(*Arg1);
       } else {
         LocSrc = Env.get<RecordStorageLocation>(*Arg1);
       }
@@ -575,15 +572,6 @@ public:
     propagateValue(*RBO->getSemanticForm(), *RBO, Env);
   }
 
-  void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *S) {
-    if (S->getCastKind() == CK_ConstructorConversion) {
-      const Expr *SubExpr = S->getSubExpr();
-      assert(SubExpr != nullptr);
-
-      propagateValue(*SubExpr, *S, Env);
-    }
-  }
-
   void VisitCallExpr(const CallExpr *S) {
     // Of clang's builtins, only `__builtin_expect` is handled explicitly, since
     // others (like trap, debugtrap, and unreachable) are handled by CFG
@@ -613,12 +601,10 @@ public:
 
       // If this call produces a prvalue of record type, initialize its fields
       // with values.
-      if (S->getType()->isRecordType() && S->isPRValue())
-        if (Env.getValue(*S) == nullptr) {
-          RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
-          Env.initializeFieldsWithValues(Loc);
-          Env.setValue(*S, refreshRecordValue(Loc, Env));
-        }
+      if (S->getType()->isRecordType() && S->isPRValue()) {
+        RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
+        Env.initializeFieldsWithValues(Loc);
+      }
     }
   }
 
@@ -626,18 +612,16 @@ public:
     const Expr *SubExpr = S->getSubExpr();
     assert(SubExpr != nullptr);
 
-    Value *SubExprVal = Env.getValue(*SubExpr);
-    if (SubExprVal == nullptr)
-      return;
+    StorageLocation &Loc = Env.createStorageLocation(*S);
+    Env.setStorageLocation(*S, Loc);
 
-    if (RecordValue *RecordVal = dyn_cast<RecordValue>(SubExprVal)) {
-      Env.setStorageLocation(*S, RecordVal->getLoc());
+    if (SubExpr->getType()->isRecordType())
+      // Nothing else left to do -- we initialized the record when transferring
+      // `SubExpr`.
       return;
-    }
 
-    StorageLocation &Loc = Env.createStorageLocation(*S);
-    Env.setValue(Loc, *SubExprVal);
-    Env.setStorageLocation(*S, Loc);
+    if (Value *SubExprVal = Env.getValue(*SubExpr))
+      Env.setValue(Loc, *SubExprVal);
   }
 
   void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
@@ -683,15 +667,11 @@ public:
       return;
     }
 
-    // In case the initializer list is transparent, we just need to propagate
-    // the value that it contains.
-    if (S->isSemanticForm() && S->isTransparent()) {
-      propagateValue(*S->getInit(0), *S, Env);
+    // If the initializer list is transparent, there's nothing to do.
+    if (S->isSemanticForm() && S->isTransparent())
       return;
-    }
 
     RecordStorageLocation &Loc = Env.getResultObjectLocation(*S);
-    Env.setValue(*S, refreshRecordValue(Loc, Env));
 
     // Initialization of base classes and fields of record type happens when we
     // visit the nested `CXXConstructExpr` or `InitListExpr` for that base class
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 1b73c5d..71d5c1a 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -367,11 +367,11 @@ builtinTransferInitializer(const CFGInitializer &Elt,
       return;
 
     ParentLoc->setChild(*Member, InitExprLoc);
-  } else if (auto *InitExprVal = Env.getValue(*InitExpr)) {
-    assert(MemberLoc != nullptr);
     // Record-type initializers construct themselves directly into the result
     // object, so there is no need to handle them here.
-    if (!Member->getType()->isRecordType())
+  } else if (!Member->getType()->isRecordType()) {
+    assert(MemberLoc != nullptr);
+    if (auto *InitExprVal = Env.getValue(*InitExpr))
       Env.setValue(*MemberLoc, *InitExprVal);
   }
 }
diff --git a/clang/lib/Analysis/FlowSensitive/Value.cpp b/clang/lib/Analysis/FlowSensitive/Value.cpp
index 7fad6de..d70e5a8 100644
--- a/clang/lib/Analysis/FlowSensitive/Value.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Value.cpp
@@ -46,8 +46,6 @@ raw_ostream &operator<<(raw_ostream &OS, const Value &Val) {
     return OS << "Integer(@" << &Val << ")";
   case Value::Kind::Pointer:
     return OS << "Pointer(" << &cast<PointerValue>(Val).getPointeeLoc() << ")";
-  case Value::Kind::Record:
-    return OS << "Record(" << &cast<RecordValue>(Val).getLoc() << ")";
   case Value::Kind::TopBool:
     return OS << "TopBool(" << cast<TopBoolValue>(Val).getAtom() << ")";
   case Value::Kind::AtomicBool:
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index cc20623..4195648 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -150,56 +150,6 @@ TEST_F(EnvironmentTest, CreateValueRecursiveType) {
   EXPECT_THAT(PV, NotNull());
 }
 
-TEST_F(EnvironmentTest, JoinRecords) {
-  using namespace ast_matchers;
-
-  std::string Code = R"cc(
-    struct S {};
-    // Need to use the type somewhere so that the `QualType` gets created;
-    S s;
-  )cc";
-
-  auto Unit =
-      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
-  auto &Context = Unit->getASTContext();
-
-  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
-  auto Results =
-      match(qualType(hasDeclaration(recordDecl(hasName("S")))).bind("SType"),
-            Context);
-  const QualType *TyPtr = selectFirst<QualType>("SType", Results);
-  ASSERT_THAT(TyPtr, NotNull());
-  QualType Ty = *TyPtr;
-  ASSERT_FALSE(Ty.isNull());
-
-  auto *ConstructExpr = CXXConstructExpr::CreateEmpty(Context, 0);
-  ConstructExpr->setType(Ty);
-  ConstructExpr->setValueKind(VK_PRValue);
-
-  // Two different `RecordValue`s with the same location are joined into a
-  // third `RecordValue` with that same location.
-  {
-    Environment Env1(DAContext);
-    auto &Val1 = *cast<RecordValue>(Env1.createValue(Ty));
-    RecordStorageLocation &Loc = Val1.getLoc();
-    Env1.setValue(Loc, Val1);
-
-    Environment Env2(DAContext);
-    auto &Val2 = Env2.create<RecordValue>(Loc);
-    Env2.setValue(Loc, Val2);
-    Env2.setValue(Loc, Val2);
-
-    Environment::ValueModel Model;
-    Environment EnvJoined =
-        Environment::join(Env1, Env2, Model, Environment::DiscardExprState);
-    auto *JoinedVal = cast<RecordValue>(EnvJoined.getValue(Loc));
-    EXPECT_NE(JoinedVal, &Val1);
-    EXPECT_NE(JoinedVal, &Val2);
-    EXPECT_EQ(&JoinedVal->getLoc(), &Loc);
-  }
-}
-
 TEST_F(EnvironmentTest, DifferentReferenceLocInJoin) {
   // This tests the case where the storage location for a reference-type
   // variable is different for two states being joined. We used to believe this
@@ -453,35 +403,4 @@ TEST_F(EnvironmentTest,
               Contains(Member));
 }
 
-TEST_F(EnvironmentTest, RefreshRecordValue) {
-  using namespace ast_matchers;
-
-  std::string Code = R"cc(
-     struct S {};
-     void target () {
-       S s;
-       s;
-     }
-  )cc";
-
-  auto Unit =
-      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
-  auto &Context = Unit->getASTContext();
-
-  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
-  auto Results = match(functionDecl(hasName("target")).bind("target"), Context);
-  const auto *Target = selectFirst<FunctionDecl>("target", Results);
-  ASSERT_THAT(Target, NotNull());
-
-  Results = match(declRefExpr(to(varDecl(hasName("s")))).bind("s"), Context);
-  const auto *DRE = selectFirst<DeclRefExpr>("s", Results);
-  ASSERT_THAT(DRE, NotNull());
-
-  Environment Env(DAContext, *Target);
-  EXPECT_THAT(Env.getStorageLocation(*DRE), IsNull());
-  refreshRecordValue(*DRE, Env);
-  EXPECT_THAT(Env.getStorageLocation(*DRE), NotNull());
-}
-
 } // namespace
diff --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index 55baa4e..88b9266 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -85,10 +85,6 @@ TEST(RecordOpsTest, CopyRecord) {
         EXPECT_NE(Env.getValue(S1.getSyntheticField("synth_int")),
                   Env.getValue(S2.getSyntheticField("synth_int")));
 
-        auto *S1Val = cast<RecordValue>(Env.getValue(S1));
-        auto *S2Val = cast<RecordValue>(Env.getValue(S2));
-        EXPECT_NE(S1Val, S2Val);
-
         copyRecord(S1, S2, Env);
 
         EXPECT_EQ(getFieldValue(&S1, *OuterIntDecl, Env),
@@ -98,10 +94,6 @@ TEST(RecordOpsTest, CopyRecord) {
                   getFieldValue(&Inner2, *InnerIntDecl, Env));
         EXPECT_EQ(Env.getValue(S1.getSyntheticField("synth_int")),
                   Env.getValue(S2.getSyntheticField("synth_int")));
-
-        S1Val = cast<RecordValue>(Env.getValue(S1));
-        S2Val = cast<RecordValue>(Env.getValue(S2));
-        EXPECT_NE(S1Val, S2Val);
       });
 }
 
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 01b8fd0..bb16138 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -260,9 +260,6 @@ TEST(TransferTest, StructIncomplete) {
         ASSERT_THAT(FooValue, NotNull());
 
         EXPECT_TRUE(isa<RecordStorageLocation>(FooValue->getPointeeLoc()));
-        auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
-        ASSERT_THAT(FooPointeeValue, NotNull());
-        EXPECT_TRUE(isa<RecordValue>(FooPointeeValue));
       });
 }
 
@@ -311,9 +308,10 @@ TEST(TransferTest, StructFieldUnmodeled) {
 
         const auto *FooLoc =
             cast<RecordStorageLocation>(Env.getStorageLocation(*FooDecl));
-        const auto *UnmodeledLoc = FooLoc->getChild(*UnmodeledDecl);
-        ASSERT_TRUE(isa<RecordStorageLocation>(UnmodeledLoc));
-        EXPECT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
+        const auto &UnmodeledLoc =
+            *cast<RecordStorageLocation>(FooLoc->getChild(*UnmodeledDecl));
+        StorageLocation &UnmodeledXLoc = getFieldLoc(UnmodeledLoc, "X", ASTCtx);
+        EXPECT_EQ(Env.getValue(UnmodeledXLoc), nullptr);
 
         const ValueDecl *ZabDecl = findValueDecl(ASTCtx, "Zab");
         ASSERT_THAT(ZabDecl, NotNull());
@@ -492,9 +490,6 @@ TEST(TransferTest, ReferenceVarDecl) {
 
         const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
         ASSERT_TRUE(isa_and_nonnull<RecordStorageLocation>(FooLoc));
-
-        const Value *FooReferentVal = Env.getValue(*FooLoc);
-        EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooReferentVal));
       });
 }
 
@@ -585,22 +580,21 @@ TEST(TransferTest, SelfReferentialReferenceVarDecl) {
 
     const auto &FooReferentLoc =
         *cast<RecordStorageLocation>(BarLoc.getChild(*FooRefDecl));
-    EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
-    EXPECT_THAT(getFieldValue(&FooReferentLoc, *BarDecl, Env), IsNull());
+    EXPECT_EQ(Env.getValue(*cast<RecordStorageLocation>(
+                                FooReferentLoc.getChild(*BarDecl))
+                                ->getChild(*FooPtrDecl)),
+              nullptr);
 
     const auto &FooPtrVal =
         *cast<PointerValue>(getFieldValue(&BarLoc, *FooPtrDecl, Env));
     const auto &FooPtrPointeeLoc =
         cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
-    EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
-    EXPECT_THAT(getFieldValue(&FooPtrPointeeLoc, *BarDecl, Env), IsNull());
-
-    EXPECT_THAT(getFieldValue(&BarLoc, *BazRefDecl, Env), NotNull());
+    EXPECT_EQ(Env.getValue(*cast<RecordStorageLocation>(
+                                FooPtrPointeeLoc.getChild(*BarDecl))
+                                ->getChild(*FooPtrDecl)),
+              nullptr);
 
-    const auto &BazPtrVal =
-        *cast<PointerValue>(getFieldValue(&BarLoc, *BazPtrDecl, Env));
-    const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
-    EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+    EXPECT_TRUE(isa<PointerValue>(getFieldValue(&BarLoc, *BazPtrDecl, Env)));
   });
 }
 
@@ -631,9 +625,6 @@ TEST(TransferTest, PointerVarDecl) {
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
         const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
         EXPECT_TRUE(isa<RecordStorageLocation>(&FooPointeeLoc));
-
-        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-        EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooPointeeVal));
       });
 }
 
@@ -740,20 +731,14 @@ TEST(TransferTest, SelfReferentialPointerVarDecl) {
         const auto &BarPointeeLoc =
             cast<RecordStorageLocation>(BarVal.getPointeeLoc());
 
-        EXPECT_THAT(getFieldValue(&BarPointeeLoc, *FooRefDecl, Env), NotNull());
-
         const auto &FooPtrVal = *cast<PointerValue>(
             getFieldValue(&BarPointeeLoc, *FooPtrDecl, Env));
         const auto &FooPtrPointeeLoc =
             cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
-        EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+        EXPECT_EQ(Env.getValue(*FooPtrPointeeLoc.getChild(*BarDecl)), nullptr);
 
-        EXPECT_THAT(getFieldValue(&BarPointeeLoc, *BazRefDecl, Env), NotNull());
-
-        const auto &BazPtrVal = *cast<PointerValue>(
-            getFieldValue(&BarPointeeLoc, *BazPtrDecl, Env));
-        const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
-        EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+        EXPECT_TRUE(
+            isa<PointerValue>(getFieldValue(&BarPointeeLoc, *BazPtrDecl, Env)));
       });
 }
 
@@ -1165,9 +1150,6 @@ TEST(TransferTest, ReferenceParamDecl) {
 
         const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
         ASSERT_TRUE(isa_and_nonnull<RecordStorageLocation>(FooLoc));
-
-        const Value *FooReferentVal = Env.getValue(*FooLoc);
-        EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooReferentVal));
       });
 }
 
@@ -1196,9 +1178,6 @@ TEST(TransferTest, PointerParamDecl) {
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
         const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
         EXPECT_TRUE(isa<RecordStorageLocation>(&FooPointeeLoc));
-
-        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-        EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooPointeeVal));
       });
 }
 
@@ -1400,8 +1379,7 @@ static void derivedBaseMemberExpectations(
 
   const auto &FooLoc =
       *cast<RecordStorageLocation>(Env.getStorageLocation(*FooDecl));
-  const auto &FooVal = *cast<RecordValue>(Env.getValue(FooLoc));
-  EXPECT_EQ(&FooVal.getLoc(), &FooLoc);
+  EXPECT_NE(Env.getValue(*FooLoc.getChild(*BarDecl)), nullptr);
 }
 
 TEST(TransferTest, DerivedBaseMemberStructDefault) {
@@ -1850,7 +1828,6 @@ TEST(TransferTest, StructThisMember) {
 
         const auto *QuxLoc =
             cast<RecordStorageLocation>(ThisLoc->getChild(*QuxDecl));
-        EXPECT_THAT(dyn_cast<RecordValue>(Env.getValue(*QuxLoc)), NotNull());
 
         const auto *BazVal =
             cast<IntegerValue>(getFieldValue(QuxLoc, *BazDecl, Env));
@@ -1921,7 +1898,6 @@ TEST(TransferTest, ClassThisMember) {
 
         const auto *QuxLoc =
             cast<RecordStorageLocation>(ThisLoc->getChild(*QuxDecl));
-        EXPECT_THAT(dyn_cast<RecordValue>(Env.getValue(*QuxLoc)), NotNull());
 
         const auto *BazVal =
             cast<IntegerValue>(getFieldValue(QuxLoc, *BazDecl, Env));
@@ -2297,10 +2273,6 @@ TEST(TransferTest, AssignmentOperator) {
           const auto *BarLoc2 =
               cast<RecordStorageLocation>(Env2.getStorageLocation(*BarDecl));
 
-          const auto *FooVal2 = cast<RecordValue>(Env2.getValue(*FooLoc2));
-          const auto *BarVal2 = cast<RecordValue>(Env2.getValue(*BarLoc2));
-          EXPECT_NE(FooVal2, BarVal2);
-
           EXPECT_TRUE(recordsEqual(*FooLoc2, *BarLoc2, Env2));
 
           const auto *FooBazVal2 =
@@ -2652,12 +2624,7 @@ TEST(TransferTest, CopyConstructor) {
           const auto *BarLoc =
               cast<RecordStorageLocation>(Env.getStorageLocation(*BarDecl));
 
-          // `Foo` and `Bar` have different `RecordValue`s associated with them.
-          const auto *FooVal = cast<RecordValue>(Env.getValue(*FooLoc));
-          const auto *BarVal = cast<RecordValue>(Env.getValue(*BarLoc));
-          EXPECT_NE(FooVal, BarVal);
-
-          // But the records compare equal.
+          // The records compare equal.
           EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
 
           // In particular, the value of `Baz` in both records is the same.
@@ -2878,10 +2845,6 @@ TEST(TransferTest, MoveConstructor) {
 
         EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
 
-        const auto *FooVal1 = cast<RecordValue>(Env1.getValue(*FooLoc1));
-        const auto *BarVal1 = cast<RecordValue>(Env1.getValue(*BarLoc1));
-        EXPECT_NE(FooVal1, BarVal1);
-
         const auto *FooBazVal1 =
             cast<IntegerValue>(getFieldValue(FooLoc1, *BazDecl, Env1));
         const auto *BarBazVal1 =
@@ -2890,8 +2853,6 @@ TEST(TransferTest, MoveConstructor) {
 
         const auto *FooLoc2 =
             cast<RecordStorageLocation>(Env2.getStorageLocation(*FooDecl));
-        const auto *FooVal2 = cast<RecordValue>(Env2.getValue(*FooLoc2));
-        EXPECT_NE(FooVal2, BarVal1);
         EXPECT_TRUE(recordsEqual(*FooLoc2, Env2, *BarLoc1, Env1));
 
         const auto *FooBazVal2 =
@@ -3516,7 +3477,8 @@ TEST(TransferTest, NullToPointerCast) {
 
         const StorageLocation &BazPointeeLoc = BazVal->getPointeeLoc();
         EXPECT_TRUE(isa<RecordStorageLocation>(BazPointeeLoc));
-        EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+        EXPECT_EQ(BazVal, &Env.fork().getOrCreateNullPointerValue(
+                              BazPointeeLoc.getType()));
 
         const StorageLocation &NullPointeeLoc = NullVal->getPointeeLoc();
         EXPECT_TRUE(isa<ScalarStorageLocation>(NullPointeeLoc));
@@ -3670,10 +3632,14 @@ TEST(TransferTest, CannotAnalyzeMethodOfClassTemplate) {
 
 TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
   std::string Code = R"(
-    struct A {};
+    struct A {
+      int i;
+    };
 
     void target(A Foo, A Bar, bool Cond) {
       A Baz = Cond ?  Foo : Bar;
+      // Make sure A::i is modeled.
+      Baz.i;
       /*[[p]]*/
     }
   )";
@@ -3681,26 +3647,20 @@ TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
       Code,
       [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
          ASTContext &ASTCtx) {
-        ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
         const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
 
-        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-        ASSERT_THAT(FooDecl, NotNull());
-
-        const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
-        ASSERT_THAT(BarDecl, NotNull());
+        auto *FooIVal = cast<IntegerValue>(getFieldValue(
+            &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Foo"), "i",
+            ASTCtx, Env));
+        auto *BarIVal = cast<IntegerValue>(getFieldValue(
+            &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Bar"), "i",
+            ASTCtx, Env));
+        auto *BazIVal = cast<IntegerValue>(getFieldValue(
+            &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Baz"), "i",
+            ASTCtx, Env));
 
-        const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
-        ASSERT_THAT(BazDecl, NotNull());
-
-        const auto *FooVal = cast<RecordValue>(Env.getValue(*FooDecl));
-        const auto *BarVal = cast<RecordValue>(Env.getValue(*BarDecl));
-
-        const auto *BazVal = dyn_cast<RecordValue>(Env.getValue(*BazDecl));
-        ASSERT_THAT(BazVal, NotNull());
-
-        EXPECT_NE(BazVal, FooVal);
-        EXPECT_NE(BazVal, BarVal);
+        EXPECT_NE(BazIVal, FooIVal);
+        EXPECT_NE(BazIVal, BarIVal);
       });
 }
 
@@ -3967,7 +3927,6 @@ TEST(TransferTest, AssignToUnionMember) {
         const auto *BazLoc = dyn_cast_or_null<RecordStorageLocation>(
             Env.getStorageLocation(*BazDecl));
         ASSERT_THAT(BazLoc, NotNull());
-        ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
 
         const auto *FooVal =
             cast<IntegerValue>(getFieldValue(BazLoc, *FooDecl, Env));
@@ -5558,17 +5517,15 @@ TEST(TransferTest, ContextSensitiveReturnReferenceWithConditionalOperator) {
         ASSERT_THAT(SDecl, NotNull());
 
         auto *SLoc = Env.getStorageLocation(*SDecl);
-        ASSERT_THAT(SLoc, NotNull());
-        EXPECT_THAT(Env.getValue(*SLoc), NotNull());
+        EXPECT_THAT(SLoc, NotNull());
 
         auto *Loc = Env.getReturnStorageLocation();
-        ASSERT_THAT(Loc, NotNull());
-        EXPECT_THAT(Env.getValue(*Loc), NotNull());
+        EXPECT_THAT(Loc, NotNull());
 
         // TODO: We would really like to make this stronger assertion, but that
         // doesn't work because we don't propagate values correctly through
         // the conditional operator yet.
-        // ASSERT_THAT(Loc, Eq(SLoc));
+        // EXPECT_EQ(Loc, SLoc);
       },
       {BuiltinOptions{ContextSensitiveOptions{}}});
 }
@@ -6634,7 +6591,7 @@ TEST(TransferTest, NewExpressions_Structs) {
     void target() {
       Outer *p = new Outer;
       // Access the fields to make sure the analysis actually generates children
-      // for them in the `RecordStorageLocation` and `RecordValue`.
+      // for them in the `RecordStorageLocation`.
       p->OuterField.InnerField;
       // [[after_new]]
     }
@@ -6656,9 +6613,6 @@ TEST(TransferTest, NewExpressions_Structs) {
             *cast<RecordStorageLocation>(OuterLoc.getChild(*OuterField));
         auto &InnerFieldLoc = *OuterFieldLoc.getChild(*InnerField);
 
-        // Values for the struct and all fields exist after the new.
-        EXPECT_THAT(Env.getValue(OuterLoc), NotNull());
-        EXPECT_THAT(Env.getValue(OuterFieldLoc), NotNull());
         EXPECT_THAT(Env.getValue(InnerFieldLoc), NotNull());
       });
 }