Revert "[clang][dataflow] Propagate locations from result objects to initializers." (#88315)

Reverts llvm/llvm-project#87320

This is causing buildbots to fail because
`isOriginalRecordConstructor()` is now unused.

3 files changed, 206 insertions, 388 deletions

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 6c796b4..1bfa7eb 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -15,7 +15,6 @@
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
@@ -27,8 +26,6 @@
 #include <cassert>
 #include <utility>
 
-#define DEBUG_TYPE "dataflow"
-
 namespace clang {
 namespace dataflow {
 
@@ -357,8 +354,6 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
   for (auto *Child : S.children())
     if (Child != nullptr)
       getFieldsGlobalsAndFuncs(*Child, Fields, Vars, Funcs);
-  if (const auto *DefaultArg = dyn_cast<CXXDefaultArgExpr>(&S))
-    getFieldsGlobalsAndFuncs(*DefaultArg->getExpr(), Fields, Vars, Funcs);
   if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(&S))
     getFieldsGlobalsAndFuncs(*DefaultInit->getExpr(), Fields, Vars, Funcs);
 
@@ -391,186 +386,6 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
   }
 }
 
-namespace {
-
-// Visitor that builds a map from record prvalues to result objects.
-// This traverses the body of the function to be analyzed; for each result
-// object that it encounters, it propagates the storage location of the result
-// object to all record prvalues that can initialize it.
-class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
-public:
-  // `ResultObjectMap` will be filled with a map from record prvalues to result
-  // object. If the function being analyzed returns a record by value,
-  // `LocForRecordReturnVal` is the location to which this record should be
-  // written; otherwise, it is null.
-  explicit ResultObjectVisitor(
-      llvm::DenseMap<const Expr *, RecordStorageLocation *> &ResultObjectMap,
-      RecordStorageLocation *LocForRecordReturnVal,
-      DataflowAnalysisContext &DACtx)
-      : ResultObjectMap(ResultObjectMap),
-        LocForRecordReturnVal(LocForRecordReturnVal), DACtx(DACtx) {}
-
-  bool shouldVisitImplicitCode() { return true; }
-
-  bool shouldVisitLambdaBody() const { return false; }
-
-  // Traverse all member and base initializers of `Ctor`. This function is not
-  // called by `RecursiveASTVisitor`; it should be called manually if we are
-  // analyzing a constructor. `ThisPointeeLoc` is the storage location that
-  // `this` points to.
-  void TraverseConstructorInits(const CXXConstructorDecl *Ctor,
-                                RecordStorageLocation *ThisPointeeLoc) {
-    assert(ThisPointeeLoc != nullptr);
-    for (const CXXCtorInitializer *Init : Ctor->inits()) {
-      Expr *InitExpr = Init->getInit();
-      if (FieldDecl *Field = Init->getMember();
-          Field != nullptr && Field->getType()->isRecordType()) {
-        PropagateResultObject(InitExpr, cast<RecordStorageLocation>(
-                                            ThisPointeeLoc->getChild(*Field)));
-      } else if (Init->getBaseClass()) {
-        PropagateResultObject(InitExpr, ThisPointeeLoc);
-      }
-
-      // Ensure that any result objects within `InitExpr` (e.g. temporaries)
-      // are also propagated to the prvalues that initialize them.
-      TraverseStmt(InitExpr);
-
-      // If this is a `CXXDefaultInitExpr`, also propagate any result objects
-      // within the default expression.
-      if (auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(InitExpr))
-        TraverseStmt(DefaultInit->getExpr());
-    }
-  }
-
-  bool TraverseBindingDecl(BindingDecl *BD) {
-    // `RecursiveASTVisitor` doesn't traverse holding variables for
-    // `BindingDecl`s by itself, so we need to tell it to.
-    if (VarDecl *HoldingVar = BD->getHoldingVar())
-      TraverseDecl(HoldingVar);
-    return RecursiveASTVisitor<ResultObjectVisitor>::TraverseBindingDecl(BD);
-  }
-
-  bool VisitVarDecl(VarDecl *VD) {
-    if (VD->getType()->isRecordType() && VD->hasInit())
-      PropagateResultObject(
-          VD->getInit(),
-          &cast<RecordStorageLocation>(DACtx.getStableStorageLocation(*VD)));
-    return true;
-  }
-
-  bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *MTE) {
-    if (MTE->getType()->isRecordType())
-      PropagateResultObject(
-          MTE->getSubExpr(),
-          &cast<RecordStorageLocation>(DACtx.getStableStorageLocation(*MTE)));
-    return true;
-  }
-
-  bool VisitReturnStmt(ReturnStmt *Return) {
-    Expr *RetValue = Return->getRetValue();
-    if (RetValue != nullptr && RetValue->getType()->isRecordType() &&
-        RetValue->isPRValue())
-      PropagateResultObject(RetValue, LocForRecordReturnVal);
-    return true;
-  }
-
-  bool VisitExpr(Expr *E) {
-    // Clang's AST can have record-type prvalues without a result object -- for
-    // example as full-expressions contained in a compound statement or as
-    // arguments of call expressions. We notice this if we get here and a
-    // storage location has not yet been associated with `E`. In this case,
-    // treat this as if it was a `MaterializeTemporaryExpr`.
-    if (E->isPRValue() && E->getType()->isRecordType() &&
-        !ResultObjectMap.contains(E))
-      PropagateResultObject(
-          E, &cast<RecordStorageLocation>(DACtx.getStableStorageLocation(*E)));
-    return true;
-  }
-
-  // Assigns `Loc` as the result object location of `E`, then propagates the
-  // location to all lower-level prvalues that initialize the same object as
-  // `E` (or one of its base classes or member variables).
-  void PropagateResultObject(Expr *E, RecordStorageLocation *Loc) {
-    if (!E->isPRValue() || !E->getType()->isRecordType()) {
-      assert(false);
-      // Ensure we don't propagate the result object if we hit this in a
-      // release build.
-      return;
-    }
-
-    ResultObjectMap[E] = Loc;
-
-    // The following AST node kinds are "original initializers": They are the
-    // lowest-level AST node that initializes a given object, and nothing
-    // below them can initialize the same object (or part of it).
-    if (isa<CXXConstructExpr>(E) || isa<CallExpr>(E) || isa<LambdaExpr>(E) ||
-        isa<CXXDefaultArgExpr>(E) || isa<CXXDefaultInitExpr>(E) ||
-        isa<CXXStdInitializerListExpr>(E)) {
-      return;
-    }
-
-    if (auto *InitList = dyn_cast<InitListExpr>(E)) {
-      if (!InitList->isSemanticForm())
-        return;
-      if (InitList->isTransparent()) {
-        PropagateResultObject(InitList->getInit(0), Loc);
-        return;
-      }
-
-      RecordInitListHelper InitListHelper(InitList);
-
-      for (auto [Base, Init] : InitListHelper.base_inits()) {
-        assert(Base->getType().getCanonicalType() ==
-               Init->getType().getCanonicalType());
-
-        // Storage location for the base class is the same as that of the
-        // derived class because we "flatten" the object hierarchy and put all
-        // fields in `RecordStorageLocation` of the derived class.
-        PropagateResultObject(Init, Loc);
-      }
-
-      for (auto [Field, Init] : InitListHelper.field_inits()) {
-        // Fields of non-record type are handled in
-        // `TransferVisitor::VisitInitListExpr()`.
-        if (!Field->getType()->isRecordType())
-          continue;
-        PropagateResultObject(
-            Init, cast<RecordStorageLocation>(Loc->getChild(*Field)));
-      }
-      return;
-    }
-
-    if (auto *Op = dyn_cast<BinaryOperator>(E); Op && Op->isCommaOp()) {
-      PropagateResultObject(Op->getRHS(), Loc);
-      return;
-    }
-
-    if (auto *Cond = dyn_cast<AbstractConditionalOperator>(E)) {
-      PropagateResultObject(Cond->getTrueExpr(), Loc);
-      PropagateResultObject(Cond->getFalseExpr(), Loc);
-      return;
-    }
-
-    // All other expression nodes that propagate a record prvalue should have
-    // exactly one child.
-    SmallVector<Stmt *, 1> Children(E->child_begin(), E->child_end());
-    LLVM_DEBUG({
-      if (Children.size() != 1)
-        E->dump();
-    });
-    assert(Children.size() == 1);
-    for (Stmt *S : Children)
-      PropagateResultObject(cast<Expr>(S), Loc);
-  }
-
-private:
-  llvm::DenseMap<const Expr *, RecordStorageLocation *> &ResultObjectMap;
-  RecordStorageLocation *LocForRecordReturnVal;
-  DataflowAnalysisContext &DACtx;
-};
-
-} // namespace
-
 Environment::Environment(DataflowAnalysisContext &DACtx)
     : DACtx(&DACtx),
       FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {}
@@ -586,23 +401,17 @@ void Environment::initialize() {
   if (DeclCtx == nullptr)
     return;
 
-  const auto *FuncDecl = dyn_cast<FunctionDecl>(DeclCtx);
-  if (FuncDecl == nullptr)
-    return;
-
-  assert(FuncDecl->doesThisDeclarationHaveABody());
+  if (const auto *FuncDecl = dyn_cast<FunctionDecl>(DeclCtx)) {
+    assert(FuncDecl->doesThisDeclarationHaveABody());
 
-  initFieldsGlobalsAndFuncs(FuncDecl);
+    initFieldsGlobalsAndFuncs(FuncDecl);
 
-  for (const auto *ParamDecl : FuncDecl->parameters()) {
-    assert(ParamDecl != nullptr);
-    setStorageLocation(*ParamDecl, createObject(*ParamDecl, nullptr));
+    for (const auto *ParamDecl : FuncDecl->parameters()) {
+      assert(ParamDecl != nullptr);
+      setStorageLocation(*ParamDecl, createObject(*ParamDecl, nullptr));
+    }
   }
 
-  if (FuncDecl->getReturnType()->isRecordType())
-    LocForRecordReturnVal = &cast<RecordStorageLocation>(
-        createStorageLocation(FuncDecl->getReturnType()));
-
   if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(DeclCtx)) {
     auto *Parent = MethodDecl->getParent();
     assert(Parent != nullptr);
@@ -635,12 +444,6 @@ void Environment::initialize() {
         initializeFieldsWithValues(ThisLoc);
     }
   }
-
-  // We do this below the handling of `CXXMethodDecl` above so that we can
-  // be sure that the storage location for `this` has been set.
-  ResultObjectMap = std::make_shared<PrValueToResultObject>(
-      buildResultObjectMap(DACtx, FuncDecl, getThisPointeeStorageLocation(),
-                           LocForRecordReturnVal));
 }
 
 // FIXME: Add support for resetting globals after function calls to enable
@@ -681,18 +484,13 @@ void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
     if (getStorageLocation(*D) != nullptr)
       continue;
 
-    // We don't run transfer functions on the initializers of global variables,
-    // so they won't be associated with a value or storage location. We
-    // therefore intentionally don't pass an initializer to `createObject()`;
-    // in particular, this ensures that `createObject()` will initialize the
-    // fields of record-type variables with values.
-    setStorageLocation(*D, createObject(*D, nullptr));
+    setStorageLocation(*D, createObject(*D));
   }
 
   for (const FunctionDecl *FD : Funcs) {
     if (getStorageLocation(*FD) != nullptr)
       continue;
-    auto &Loc = createStorageLocation(*FD);
+    auto &Loc = createStorageLocation(FD->getType());
     setStorageLocation(*FD, Loc);
   }
 }
@@ -721,9 +519,6 @@ Environment Environment::pushCall(const CallExpr *Call) const {
     }
   }
 
-  if (Call->getType()->isRecordType() && Call->isPRValue())
-    Env.LocForRecordReturnVal = &Env.getResultObjectLocation(*Call);
-
   Env.pushCallInternal(Call->getDirectCallee(),
                        llvm::ArrayRef(Call->getArgs(), Call->getNumArgs()));
 
@@ -734,7 +529,6 @@ Environment Environment::pushCall(const CXXConstructExpr *Call) const {
   Environment Env(*this);
 
   Env.ThisPointeeLoc = &Env.getResultObjectLocation(*Call);
-  Env.LocForRecordReturnVal = &Env.getResultObjectLocation(*Call);
 
   Env.pushCallInternal(Call->getConstructor(),
                        llvm::ArrayRef(Call->getArgs(), Call->getNumArgs()));
@@ -763,10 +557,6 @@ void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
     const VarDecl *Param = *ParamIt;
     setStorageLocation(*Param, createObject(*Param, Args[ArgIndex]));
   }
-
-  ResultObjectMap = std::make_shared<PrValueToResultObject>(
-      buildResultObjectMap(DACtx, FuncDecl, getThisPointeeStorageLocation(),
-                           LocForRecordReturnVal));
 }
 
 void Environment::popCall(const CallExpr *Call, const Environment &CalleeEnv) {
@@ -810,9 +600,6 @@ bool Environment::equivalentTo(const Environment &Other,
   if (ReturnLoc != Other.ReturnLoc)
     return false;
 
-  if (LocForRecordReturnVal != Other.LocForRecordReturnVal)
-    return false;
-
   if (ThisPointeeLoc != Other.ThisPointeeLoc)
     return false;
 
@@ -836,10 +623,8 @@ LatticeEffect Environment::widen(const Environment &PrevEnv,
   assert(DACtx == PrevEnv.DACtx);
   assert(ReturnVal == PrevEnv.ReturnVal);
   assert(ReturnLoc == PrevEnv.ReturnLoc);
-  assert(LocForRecordReturnVal == PrevEnv.LocForRecordReturnVal);
   assert(ThisPointeeLoc == PrevEnv.ThisPointeeLoc);
   assert(CallStack == PrevEnv.CallStack);
-  assert(ResultObjectMap == PrevEnv.ResultObjectMap);
 
   auto Effect = LatticeEffect::Unchanged;
 
@@ -871,16 +656,12 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB,
                               Environment::ValueModel &Model,
                               ExprJoinBehavior ExprBehavior) {
   assert(EnvA.DACtx == EnvB.DACtx);
-  assert(EnvA.LocForRecordReturnVal == EnvB.LocForRecordReturnVal);
   assert(EnvA.ThisPointeeLoc == EnvB.ThisPointeeLoc);
   assert(EnvA.CallStack == EnvB.CallStack);
-  assert(EnvA.ResultObjectMap == EnvB.ResultObjectMap);
 
   Environment JoinedEnv(*EnvA.DACtx);
 
   JoinedEnv.CallStack = EnvA.CallStack;
-  JoinedEnv.ResultObjectMap = EnvA.ResultObjectMap;
-  JoinedEnv.LocForRecordReturnVal = EnvA.LocForRecordReturnVal;
   JoinedEnv.ThisPointeeLoc = EnvA.ThisPointeeLoc;
 
   if (EnvA.ReturnVal == nullptr || EnvB.ReturnVal == nullptr) {
@@ -949,12 +730,6 @@ StorageLocation &Environment::createStorageLocation(const Expr &E) {
 
 void Environment::setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
   assert(!DeclToLoc.contains(&D));
-  // The only kinds of declarations that may have a "variable" storage location
-  // are declarations of reference type and `BindingDecl`. For all other
-  // declaration, the storage location should be the stable storage location
-  // returned by `createStorageLocation()`.
-  assert(D.getType()->isReferenceType() || isa<BindingDecl>(D) ||
-         &Loc == &createStorageLocation(D));
   DeclToLoc[&D] = &Loc;
 }
 
@@ -1016,29 +791,50 @@ Environment::getResultObjectLocation(const Expr &RecordPRValue) const {
   assert(RecordPRValue.getType()->isRecordType());
   assert(RecordPRValue.isPRValue());
 
-  assert(ResultObjectMap != nullptr);
-  RecordStorageLocation *Loc = ResultObjectMap->lookup(&RecordPRValue);
-  assert(Loc != nullptr);
-  // In release builds, use the "stable" storage location if the map lookup
-  // failed.
-  if (Loc == nullptr)
+  // Returns a storage location that we can use if assertions fail.
+  auto FallbackForAssertFailure =
+      [this, &RecordPRValue]() -> RecordStorageLocation & {
     return cast<RecordStorageLocation>(
         DACtx->getStableStorageLocation(RecordPRValue));
-  return *Loc;
+  };
+
+  if (isOriginalRecordConstructor(RecordPRValue)) {
+    auto *Val = cast_or_null<RecordValue>(getValue(RecordPRValue));
+    // The builtin transfer function should have created a `RecordValue` for all
+    // original record constructors.
+    assert(Val);
+    if (!Val)
+      return FallbackForAssertFailure();
+    return Val->getLoc();
+  }
+
+  if (auto *Op = dyn_cast<BinaryOperator>(&RecordPRValue);
+      Op && Op->isCommaOp()) {
+    return getResultObjectLocation(*Op->getRHS());
+  }
+
+  // All other expression nodes that propagate a record prvalue should have
+  // exactly one child.
+  llvm::SmallVector<const Stmt *> children(RecordPRValue.child_begin(),
+                                           RecordPRValue.child_end());
+  assert(children.size() == 1);
+  if (children.empty())
+    return FallbackForAssertFailure();
+
+  return getResultObjectLocation(*cast<Expr>(children[0]));
 }
 
 PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
   return DACtx->getOrCreateNullPointerValue(PointeeType);
 }
 
-void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
-                                             QualType Type) {
+void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc) {
   llvm::DenseSet<QualType> Visited;
   int CreatedValuesCount = 0;
-  initializeFieldsWithValues(Loc, Type, Visited, 0, CreatedValuesCount);
+  initializeFieldsWithValues(Loc, Visited, 0, CreatedValuesCount);
   if (CreatedValuesCount > MaxCompositeValueSize) {
-    llvm::errs() << "Attempting to initialize a huge value of type: " << Type
-                 << '\n';
+    llvm::errs() << "Attempting to initialize a huge value of type: "
+                 << Loc.getType() << '\n';
   }
 }
 
@@ -1052,7 +848,8 @@ 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));
+    assert(isOriginalRecordConstructor(CanonE) ||
+           &RecordVal->getLoc() == &getResultObjectLocation(CanonE));
     (void)RecordVal;
   }
 
@@ -1131,8 +928,7 @@ Value *Environment::createValueUnlessSelfReferential(
   if (Type->isRecordType()) {
     CreatedValuesCount++;
     auto &Loc = cast<RecordStorageLocation>(createStorageLocation(Type));
-    initializeFieldsWithValues(Loc, Loc.getType(), Visited, Depth,
-                               CreatedValuesCount);
+    initializeFieldsWithValues(Loc, Visited, Depth, CreatedValuesCount);
 
     return &refreshRecordValue(Loc, *this);
   }
@@ -1164,7 +960,6 @@ Environment::createLocAndMaybeValue(QualType Ty,
 }
 
 void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
-                                             QualType Type,
                                              llvm::DenseSet<QualType> &Visited,
                                              int Depth,
                                              int &CreatedValuesCount) {
@@ -1172,8 +967,8 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
     if (FieldType->isRecordType()) {
       auto &FieldRecordLoc = cast<RecordStorageLocation>(FieldLoc);
       setValue(FieldRecordLoc, create<RecordValue>(FieldRecordLoc));
-      initializeFieldsWithValues(FieldRecordLoc, FieldRecordLoc.getType(),
-                                 Visited, Depth + 1, CreatedValuesCount);
+      initializeFieldsWithValues(FieldRecordLoc, Visited, Depth + 1,
+                                 CreatedValuesCount);
     } else {
       if (!Visited.insert(FieldType.getCanonicalType()).second)
         return;
@@ -1184,7 +979,7 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
     }
   };
 
-  for (const FieldDecl *Field : DACtx->getModeledFields(Type)) {
+  for (const auto &[Field, FieldLoc] : Loc.children()) {
     assert(Field != nullptr);
     QualType FieldType = Field->getType();
 
@@ -1193,12 +988,14 @@ void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc,
                    &createLocAndMaybeValue(FieldType, Visited, Depth + 1,
                                            CreatedValuesCount));
     } else {
-      StorageLocation *FieldLoc = Loc.getChild(*Field);
       assert(FieldLoc != nullptr);
       initField(FieldType, *FieldLoc);
     }
   }
-  for (const auto &[FieldName, FieldType] : DACtx->getSyntheticFields(Type)) {
+  for (const auto &[FieldName, FieldLoc] : Loc.synthetic_fields()) {
+    assert(FieldLoc != nullptr);
+    QualType FieldType = FieldLoc->getType();
+
     // Synthetic fields cannot have reference type, so we don't need to deal
     // with this case.
     assert(!FieldType->isReferenceType());
@@ -1225,36 +1022,38 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D,
     return createObjectInternal(D, Ty.getNonReferenceType(), nullptr);
   }
 
+  Value *Val = nullptr;
+  if (InitExpr) {
+    // In the (few) cases where an expression is intentionally
+    // "uninterpreted", `InitExpr` is not associated with a value.  There are
+    // two ways to handle this situation: propagate the status, so that
+    // uninterpreted initializers result in uninterpreted variables, or
+    // provide a default value. We choose the latter so that later refinements
+    // of the variable can be used for reasoning about the surrounding code.
+    // For this reason, we let this case be handled by the `createValue()`
+    // call below.
+    //
+    // FIXME. If and when we interpret all language cases, change this to
+    // assert that `InitExpr` is interpreted, rather than supplying a
+    // default value (assuming we don't update the environment API to return
+    // references).
+    Val = getValue(*InitExpr);
+
+    if (!Val && isa<ImplicitValueInitExpr>(InitExpr) &&
+        InitExpr->getType()->isPointerType())
+      Val = &getOrCreateNullPointerValue(InitExpr->getType()->getPointeeType());
+  }
+  if (!Val)
+    Val = createValue(Ty);
+
+  if (Ty->isRecordType())
+    return cast<RecordValue>(Val)->getLoc();
+
   StorageLocation &Loc =
       D ? createStorageLocation(*D) : createStorageLocation(Ty);
 
-  if (Ty->isRecordType()) {
-    auto &RecordLoc = cast<RecordStorageLocation>(Loc);
-    if (!InitExpr)
-      initializeFieldsWithValues(RecordLoc);
-    refreshRecordValue(RecordLoc, *this);
-  } else {
-    Value *Val = nullptr;
-    if (InitExpr)
-      // In the (few) cases where an expression is intentionally
-      // "uninterpreted", `InitExpr` is not associated with a value.  There are
-      // two ways to handle this situation: propagate the status, so that
-      // uninterpreted initializers result in uninterpreted variables, or
-      // provide a default value. We choose the latter so that later refinements
-      // of the variable can be used for reasoning about the surrounding code.
-      // For this reason, we let this case be handled by the `createValue()`
-      // call below.
-      //
-      // FIXME. If and when we interpret all language cases, change this to
-      // assert that `InitExpr` is interpreted, rather than supplying a
-      // default value (assuming we don't update the environment API to return
-      // references).
-      Val = getValue(*InitExpr);
-    if (!Val)
-      Val = createValue(Ty);
-    if (Val)
-      setValue(Loc, *Val);
-  }
+  if (Val)
+    setValue(Loc, *Val);
 
   return Loc;
 }
@@ -1273,8 +1072,6 @@ bool Environment::allows(const Formula &F) const {
 
 void Environment::dump(raw_ostream &OS) const {
   llvm::DenseMap<const StorageLocation *, std::string> LocToName;
-  if (LocForRecordReturnVal != nullptr)
-    LocToName[LocForRecordReturnVal] = "(returned record)";
   if (ThisPointeeLoc != nullptr)
     LocToName[ThisPointeeLoc] = "this";
 
@@ -1305,9 +1102,6 @@ void Environment::dump(raw_ostream &OS) const {
       if (auto Iter = LocToName.find(ReturnLoc); Iter != LocToName.end())
         OS << " (" << Iter->second << ")";
       OS << "\n";
-    } else if (Func->getReturnType()->isRecordType() ||
-               isa<CXXConstructorDecl>(Func)) {
-      OS << "LocForRecordReturnVal: " << LocForRecordReturnVal << "\n";
     } else if (!Func->getReturnType()->isVoidType()) {
       if (ReturnVal == nullptr)
         OS << "ReturnVal: nullptr\n";
@@ -1328,22 +1122,6 @@ void Environment::dump() const {
   dump(llvm::dbgs());
 }
 
-Environment::PrValueToResultObject Environment::buildResultObjectMap(
-    DataflowAnalysisContext *DACtx, const FunctionDecl *FuncDecl,
-    RecordStorageLocation *ThisPointeeLoc,
-    RecordStorageLocation *LocForRecordReturnVal) {
-  assert(FuncDecl->doesThisDeclarationHaveABody());
-
-  PrValueToResultObject Map;
-
-  ResultObjectVisitor Visitor(Map, LocForRecordReturnVal, *DACtx);
-  if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FuncDecl))
-    Visitor.TraverseConstructorInits(Ctor, ThisPointeeLoc);
-  Visitor.TraverseStmt(FuncDecl->getBody());
-
-  return Map;
-}
-
 RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
                                                  const Environment &Env) {
   Expr *ImplicitObject = MCE.getImplicitObjectArgument();
@@ -1438,11 +1216,24 @@ RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env) {
 RecordValue &refreshRecordValue(const Expr &Expr, Environment &Env) {
   assert(Expr.getType()->isRecordType());
 
-  if (Expr.isPRValue())
-    refreshRecordValue(Env.getResultObjectLocation(Expr), Env);
+  if (Expr.isPRValue()) {
+    if (auto *ExistingVal = Env.get<RecordValue>(Expr)) {
+      auto &NewVal = Env.create<RecordValue>(ExistingVal->getLoc());
+      Env.setValue(Expr, NewVal);
+      Env.setValue(NewVal.getLoc(), NewVal);
+      return NewVal;
+    }
 
-  if (auto *Loc = Env.get<RecordStorageLocation>(Expr))
-    refreshRecordValue(*Loc, Env);
+    auto &NewVal = *cast<RecordValue>(Env.createValue(Expr.getType()));
+    Env.setValue(Expr, NewVal);
+    return NewVal;
+  }
+
+  if (auto *Loc = Env.get<RecordStorageLocation>(Expr)) {
+    auto &NewVal = Env.create<RecordValue>(*Loc);
+    Env.setValue(*Loc, NewVal);
+    return NewVal;
+  }
 
   auto &NewVal = *cast<RecordValue>(Env.createValue(Expr.getType()));
   Env.setStorageLocation(Expr, NewVal.getLoc());
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 88a9c0e..0a2e836 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -460,9 +460,11 @@ public:
     // 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);
+        Value *Val = Env.createValue(S->getType());
+        // We're guaranteed to always be able to create a value for record
+        // types.
+        assert(Val != nullptr);
+        Env.setValue(*S, *Val);
       }
     }
   }
@@ -470,13 +472,6 @@ public:
   void VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *S) {
     const Expr *InitExpr = S->getExpr();
     assert(InitExpr != nullptr);
-
-    // If this is a prvalue of record type, the handler for `*InitExpr` (if one
-    // exists) will initialize the result object; there is no value to propgate
-    // here.
-    if (S->getType()->isRecordType() && S->isPRValue())
-      return;
-
     propagateValueOrStorageLocation(*InitExpr, *S, Env);
   }
 
@@ -484,17 +479,6 @@ public:
     const CXXConstructorDecl *ConstructorDecl = S->getConstructor();
     assert(ConstructorDecl != nullptr);
 
-    // `CXXConstructExpr` can have array type if default-initializing an array
-    // of records. We don't handle this specifically beyond potentially inlining
-    // the call.
-    if (!S->getType()->isRecordType()) {
-      transferInlineCall(S, ConstructorDecl);
-      return;
-    }
-
-    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
       // parameters as long as they have default arguments defined for them.
@@ -507,14 +491,24 @@ public:
       if (ArgLoc == nullptr)
         return;
 
-      // Even if the copy/move constructor call is elidable, we choose to copy
-      // the record in all cases (which isn't wrong, just potentially not
-      // optimal).
-      copyRecord(*ArgLoc, Loc, Env);
+      if (S->isElidable()) {
+        if (Value *Val = Env.getValue(*ArgLoc))
+          Env.setValue(*S, *Val);
+      } else {
+        auto &Val = *cast<RecordValue>(Env.createValue(S->getType()));
+        Env.setValue(*S, Val);
+        copyRecord(*ArgLoc, Val.getLoc(), Env);
+      }
       return;
     }
 
-    Env.initializeFieldsWithValues(Loc, S->getType());
+    // `CXXConstructExpr` can have array type if default-initializing an array
+    // of records, and we currently can't create values for arrays. So check if
+    // we've got a record type.
+    if (S->getType()->isRecordType()) {
+      auto &InitialVal = *cast<RecordValue>(Env.createValue(S->getType()));
+      Env.setValue(*S, InitialVal);
+    }
 
     transferInlineCall(S, ConstructorDecl);
   }
@@ -557,15 +551,19 @@ public:
       if (S->isGLValue()) {
         Env.setStorageLocation(*S, *LocDst);
       } else if (S->getType()->isRecordType()) {
-        // Assume that the assignment returns the assigned value.
-        copyRecord(*LocDst, Env.getResultObjectLocation(*S), Env);
+        // Make sure that we have a `RecordValue` for this expression so that
+        // `Environment::getResultObjectLocation()` is able to return a location
+        // for it.
+        if (Env.getValue(*S) == nullptr)
+          refreshRecordValue(*S, Env);
       }
 
       return;
     }
 
-    // `CXXOperatorCallExpr` can be a prvalue. Call `VisitCallExpr`() to
-    // initialize the prvalue's fields with values.
+    // CXXOperatorCallExpr can be prvalues. Call `VisitCallExpr`() to create
+    // a `RecordValue` for them so that `Environment::getResultObjectLocation()`
+    // can return a value.
     VisitCallExpr(S);
   }
 
@@ -582,6 +580,11 @@ public:
     }
   }
 
+  void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) {
+    if (Value *Val = Env.createValue(S->getType()))
+      Env.setValue(*S, *Val);
+  }
+
   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
@@ -609,14 +612,13 @@ public:
     } else if (const FunctionDecl *F = S->getDirectCallee()) {
       transferInlineCall(S, F);
 
-      // If this call produces a prvalue of record type, initialize its fields
-      // with values.
+      // If this call produces a prvalue of record type, make sure that we have
+      // a `RecordValue` for it. This is required so that
+      // `Environment::getResultObjectLocation()` is able to return a location
+      // for this `CallExpr`.
       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 (Env.getValue(*S) == nullptr)
+          refreshRecordValue(*S, Env);
     }
   }
 
@@ -664,10 +666,8 @@ public:
     // `getLogicOperatorSubExprValue()`.
     if (S->isGLValue())
       Env.setStorageLocation(*S, Env.createObject(S->getType()));
-    else if (!S->getType()->isRecordType()) {
-      if (Value *Val = Env.createValue(S->getType()))
-        Env.setValue(*S, *Val);
-    }
+    else if (Value *Val = Env.createValue(S->getType()))
+      Env.setValue(*S, *Val);
   }
 
   void VisitInitListExpr(const InitListExpr *S) {
@@ -688,51 +688,71 @@ public:
       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
-    // or field. We therefore only need to deal with fields of non-record type
-    // here.
-
+    llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
     RecordInitListHelper InitListHelper(S);
 
+    for (auto [Base, Init] : InitListHelper.base_inits()) {
+      assert(Base->getType().getCanonicalType() ==
+             Init->getType().getCanonicalType());
+      auto *BaseVal = Env.get<RecordValue>(*Init);
+      if (!BaseVal)
+        BaseVal = cast<RecordValue>(Env.createValue(Init->getType()));
+      // Take ownership of the fields of the `RecordValue` for the base class
+      // and incorporate them into the "flattened" set of fields for the
+      // derived class.
+      auto Children = BaseVal->getLoc().children();
+      FieldLocs.insert(Children.begin(), Children.end());
+    }
+
     for (auto [Field, Init] : InitListHelper.field_inits()) {
-      if (Field->getType()->isRecordType())
-        continue;
-      if (Field->getType()->isReferenceType()) {
-        assert(Field->getType().getCanonicalType()->getPointeeType() ==
-               Init->getType().getCanonicalType());
-        Loc.setChild(*Field, &Env.createObject(Field->getType(), Init));
-        continue;
-      }
-      assert(Field->getType().getCanonicalType().getUnqualifiedType() ==
-             Init->getType().getCanonicalType().getUnqualifiedType());
-      StorageLocation *FieldLoc = Loc.getChild(*Field);
-      // Locations for non-reference fields must always be non-null.
-      assert(FieldLoc != nullptr);
-      Value *Val = Env.getValue(*Init);
-      if (Val == nullptr && isa<ImplicitValueInitExpr>(Init) &&
-          Init->getType()->isPointerType())
-        Val =
-            &Env.getOrCreateNullPointerValue(Init->getType()->getPointeeType());
-      if (Val == nullptr)
-        Val = Env.createValue(Field->getType());
-      if (Val != nullptr)
-        Env.setValue(*FieldLoc, *Val);
+      assert(
+          // The types are same, or
+          Field->getType().getCanonicalType().getUnqualifiedType() ==
+              Init->getType().getCanonicalType().getUnqualifiedType() ||
+          // The field's type is T&, and initializer is T
+          (Field->getType()->isReferenceType() &&
+           Field->getType().getCanonicalType()->getPointeeType() ==
+               Init->getType().getCanonicalType()));
+      auto& Loc = Env.createObject(Field->getType(), Init);
+      FieldLocs.insert({Field, &Loc});
     }
 
-    for (const auto &[FieldName, FieldLoc] : Loc.synthetic_fields()) {
-      QualType FieldType = FieldLoc->getType();
-      if (FieldType->isRecordType()) {
-        Env.initializeFieldsWithValues(*cast<RecordStorageLocation>(FieldLoc));
-      } else {
-        if (Value *Val = Env.createValue(FieldType))
-          Env.setValue(*FieldLoc, *Val);
+    // In the case of a union, we don't in general have initializers for all
+    // of the fields. Create storage locations for the remaining fields (but
+    // don't associate them with values).
+    if (Type->isUnionType()) {
+      for (const FieldDecl *Field :
+           Env.getDataflowAnalysisContext().getModeledFields(Type)) {
+        if (auto [it, inserted] = FieldLocs.insert({Field, nullptr}); inserted)
+          it->second = &Env.createStorageLocation(Field->getType());
       }
     }
 
+    // Check that we satisfy the invariant that a `RecordStorageLoation`
+    // contains exactly the set of modeled fields for that type.
+    // `ModeledFields` includes fields from all the bases, but only the
+    // modeled ones. However, if a class type is initialized with an
+    // `InitListExpr`, all fields in the class, including those from base
+    // classes, are included in the set of modeled fields. The code above
+    // should therefore populate exactly the modeled fields.
+    assert(containsSameFields(
+        Env.getDataflowAnalysisContext().getModeledFields(Type), FieldLocs));
+
+    RecordStorageLocation::SyntheticFieldMap SyntheticFieldLocs;
+    for (const auto &Entry :
+         Env.getDataflowAnalysisContext().getSyntheticFields(Type)) {
+      SyntheticFieldLocs.insert(
+          {Entry.getKey(), &Env.createObject(Entry.getValue())});
+    }
+
+    auto &Loc = Env.getDataflowAnalysisContext().createRecordStorageLocation(
+        Type, std::move(FieldLocs), std::move(SyntheticFieldLocs));
+    RecordValue &RecordVal = Env.create<RecordValue>(Loc);
+
+    Env.setValue(Loc, RecordVal);
+
+    Env.setValue(*S, RecordVal);
+
     // FIXME: Implement array initialization.
   }
 
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 1b73c5d..595f70f 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -369,10 +369,17 @@ builtinTransferInitializer(const CFGInitializer &Elt,
     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())
+    if (Member->getType()->isRecordType()) {
+      auto *InitValStruct = cast<RecordValue>(InitExprVal);
+      // FIXME: Rather than performing a copy here, we should really be
+      // initializing the field in place. This would require us to propagate the
+      // storage location of the field to the AST node that creates the
+      // `RecordValue`.
+      copyRecord(InitValStruct->getLoc(),
+                 *cast<RecordStorageLocation>(MemberLoc), Env);
+    } else {
       Env.setValue(*MemberLoc, *InitExprVal);
+    }
   }
 }