[clang][dataflow] Perform deep copies in copy and move operations.

[mboehme]

Summary

This serves two purposes:

• Because, today, we only copy the StructValue, modifying the destination of the copy also modifies the source. This is demonstrated by the new checks added to CopyConstructor and MoveConstructor, which fail without the deep copy.

• It lays the groundwork for eliminating the redundancy between AggregateStorageLocation and StructValue, which will happen as part of the ongoing migration to strict handling of value categories (seeo https://discourse.llvm.org/t/70086 for details). This will involve turning StructValue into essentially just a wrapper for AggregateStorageLocation; under this scheme, the current "shallow" copy (copying a StructValue from one AggregateStorageLocation to another) will no longer be possible.

Because we now perform deep copies, tests need to perform a deep equality
comparison instead of just comparing for equal identity of the StructValues.
The new function recordsEqual() provides such a deep equality comparison.

[mboehme]

Avoid including llvm/Testing/Support/Error.h in clang/unittests/Analysis/FlowSensitive/TestingSupport.h.

This can lead to version conflicts in projects that want to include
TestingSupport.h but use a different version of gtest than the one vendored with
LLVM.

[xazax.hun]

I am not opposed to this solution, but I do anticipate some performance problems in the future. I wonder if copy-on-write, or some persistent data structures where copying is cheap would be a better strategy long term. But getting it right first and fast after sounds like a good strategy :)

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

745–746

[xazax.hun]

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

57–58

I know we usually like to mention types at least once, but wonder whether ranges/iterators are actually exceptions.

[mboehme]

Minor style changes in response to review comments.

[mboehme]

In D153006#4424980, @xazax.hun wrote:

I am not opposed to this solution, but I do anticipate some performance problems in the future. I wonder if copy-on-write, or some persistent data structures where copying is cheap would be a better strategy long term. But getting it right first and fast after sounds like a good strategy :)

Agreed - I think "right first and fast after" is the way to go here.

Also, we should wait and see whether this really does turn out to be a performance issue. We'll only be doing this for types that are copyable, and by their nature, these tend to have relatively few fields, as programmers try to avoid making types copyable that are expensive to copy at runtime.

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

57–58

I've looked at the style guide, and it seems to support this usage of auto, so done!

[mboehme]

Use cast_or_null instead of cast in VisitCXXOperatorCallExpr. Added a test
that crashes without this change.

[mboehme]

Now that integer literals are associated with Values, use them in tests
instead of using a helper variable.

[MaskRay]

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

20

See https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions

I'll change this.

[mboehme]

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

20

Thanks for pointing this out!

https://reviews.llvm.org/D153833

[RKSimon]

@mboehme https://lab.llvm.org/buildbot/#/builders/124 is still broken - please can you revert the patch series?

[mboehme]

In D153006#4463759, @RKSimon wrote:

@mboehme https://lab.llvm.org/buildbot/#/builders/124 is still broken - please can you revert the patch series?

Sorry, I did not see this in time to revert.

It seems this has been fixed in the meantime by https://github.com/llvm/llvm-project/commit/cec30e2b190bd58a26f54b5fd4232d3828632466. Thank you to @donat.nagy for the fix, and apologies for the breakage.

[gribozavr2]

clang/include/clang/Analysis/FlowSensitive/RecordOps.h

26

Shouldn't it go the other way, from Src to Dst?

53

Could you add a caveat that only 'true' return values from this function matter?

That is, "false" means "might or might not be equal at runtime, we don't know".

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

391

Could you change VerifyResultsT to a std::function (like in other overloads above) so that the type signature is clear?

I'm also unsure about the name - why start a new overload set? It seems like another variant of the checkDataflow() functions above.

[mboehme]

https://reviews.llvm.org/D154339 has changes in response to post-commit comments.

clang/include/clang/Analysis/FlowSensitive/RecordOps.h

26

Oops. Fixed in https://reviews.llvm.org/D154339.

53

Good point. Done in https://reviews.llvm.org/D154339.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

391

Could you change VerifyResultsT to a std::function (like in other overloads above) so that the type signature is clear?

Done in https://reviews.llvm.org/D154339.

I could have sworn that when I moved this from TransferTest.cpp to here that I tried making this change and realized I couldn't for some reason. But obviously I was wrong.

I'm also unsure about the name - why start a new overload set? It seems like another variant of the checkDataflow() functions above.

This is really just moved from TransferTest.cpp -- and it's more closely related to the runDataflow() functions there and in the newly added RecordOps.cpp. (I can't call it runDataflow() because then it would differ only in return type from one of the functions in the overload set.)

Let's continue the discussion on https://reviews.llvm.org/D154339 if necessary.

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

public:
   /// Requirements:
   ///
   ///  `Type` must not be null.
   Value *createValue(QualType Type);
 
   /// Assigns `Val` as the value of `Loc` in the environment.
   void setValue(const StorageLocation &Loc, Value &Val);
 
+  /// Clears any association between `Loc` and a value in the environment.
+  void clearValue(const StorageLocation &Loc);
+
   /// Assigns `Val` as the value of the prvalue `E` in the environment.
   ///
   /// If `E` is not yet associated with a storage location, associates it with
   /// a newly created storage location. In any case, associates the storage
   /// location of `E` with `Val`.
   ///
   /// Once the migration to strict handling of value categories is complete
   /// (see https://discourse.llvm.org/t/70086), this function will be renamed to

clang/include/clang/Analysis/FlowSensitive/RecordOps.h

+//===-- RecordOps.h ---------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  Operations on records (structs, classes, and unions).
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
+
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+
+namespace clang {
+namespace dataflow {
+
+/// Copies a record (struct, class, or union) from `Src` to `Dst`.
+///
+/// This performs a deep copy, i.e. it copies every field and recurses on
+/// fields of record type. It also copies properties from the `StructValue`
+/// associated with `Dst` to the `StructValue` associated with `Src` (if these
+/// `StructValue`s exist).
+///
+/// If there is a `StructValue` associated with `Dst` in the environment, this
+/// function creates a new `StructValue` and associates it with `Dst`; clients
+/// need to be aware of this and must not assume that the `StructValue`
+/// associated with `Dst` remains the same after the call.
+///
+/// We create a new `StructValue` rather than modifying properties on the old
+/// `StructValue` because the old `StructValue` may be shared with other
+/// `Environment`s, and we don't want changes to properties to be visible there.
+///
+/// Requirements:
+///
+///  `Src` and `Dst` must have the same canonical unqualified type.
+void copyRecord(AggregateStorageLocation &Src, AggregateStorageLocation &Dst,
+                Environment &Env);
+
+/// Returns whether the records `Loc1` and `Loc2` are equal.
+///
+/// Values for `Loc1` are retrieved from `Env1`, and values for `Loc2` are
+/// retrieved from `Env2`. A convenience overload retrieves values for `Loc1`
+/// and `Loc2` from the same environment.
+///
+/// This performs a deep comparison, i.e. it compares every field and recurses
+/// on fields of record type. Fields of reference type compare equal if they
+/// refer to the same storage location. If `StructValue`s are associated with
+/// `Loc1` and `Loc2`, it also compares the properties on those `StructValue`s.
+///
+/// Requirements:
+///
+///  `Src` and `Dst` must have the same canonical unqualified type.
+bool recordsEqual(const AggregateStorageLocation &Loc1, const Environment &Env1,
+                  const AggregateStorageLocation &Loc2,
+                  const Environment &Env2);
+
+inline bool recordsEqual(const AggregateStorageLocation &Loc1,
+                         const AggregateStorageLocation &Loc2,
+                         const Environment &Env) {
+  return recordsEqual(Loc1, Env, Loc2, Env);
+}
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H

clang/include/clang/Analysis/FlowSensitive/StorageLocation.h

 /// struct with public members. The child map is flat, so when used for a struct
 /// or class type, all accessible members of base struct and class types are
 /// directly accesible as children of this location.
 /// FIXME: Currently, the storage location of unions is modelled the same way as
 /// that of structs or classes. Eventually, we need to change this modelling so
 /// that all of the members of a given union have the same storage location.
 class AggregateStorageLocation final : public StorageLocation {
 public:
+  using FieldToLoc = llvm::DenseMap<const ValueDecl *, StorageLocation *>;
+
   explicit AggregateStorageLocation(QualType Type)
-      : AggregateStorageLocation(
-            Type, llvm::DenseMap<const ValueDecl *, StorageLocation *>()) {}
+      : AggregateStorageLocation(Type, FieldToLoc()) {}
 
-  AggregateStorageLocation(
-      QualType Type,
-      llvm::DenseMap<const ValueDecl *, StorageLocation *> Children)
+  AggregateStorageLocation(QualType Type, FieldToLoc Children)
       : StorageLocation(Kind::Aggregate, Type), Children(std::move(Children)) {}
 
   static bool classof(const StorageLocation *Loc) {
     return Loc->getKind() == Kind::Aggregate;
   }
 
   /// Returns the child storage location for `D`.
   StorageLocation &getChild(const ValueDecl &D) const {
     auto It = Children.find(&D);
     assert(It != Children.end());
     return *It->second;
   }
 
+  llvm::iterator_range<FieldToLoc::const_iterator> children() const {
+    return {Children.begin(), Children.end()};
+  }
+
 private:
-  llvm::DenseMap<const ValueDecl *, StorageLocation *> Children;
+  FieldToLoc Children;
 };
 
 } // namespace dataflow
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_STORAGELOCATION_H

clang/include/clang/Analysis/FlowSensitive/Value.h

public:
 
   /// Returns the child value that is assigned for `D` or null if the child is
   /// not initialized.
   Value *getChild(const ValueDecl &D) const { return Children.lookup(&D); }
 
   /// Assigns `Val` as the child value for `D`.
   void setChild(const ValueDecl &D, Value &Val) { Children[&D] = &Val; }
 
+  /// Clears any value assigned as the child value for `D`.
+  void clearChild(const ValueDecl &D) { Children.erase(&D); }
+
   llvm::iterator_range<
       llvm::DenseMap<const ValueDecl *, Value *>::const_iterator>
   children() const {
     return {Children.begin(), Children.end()};
   }
 
 private:
   llvm::DenseMap<const ValueDecl *, Value *> Children;
 };
 
 raw_ostream &operator<<(raw_ostream &OS, const Value &Val);
 
 } // namespace dataflow
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_VALUE_H

clang/lib/Analysis/FlowSensitive/CMakeLists.txt

 add_clang_library(clangAnalysisFlowSensitive
   Arena.cpp
   ControlFlowContext.cpp
   DataflowAnalysisContext.cpp
   DataflowEnvironment.cpp
   HTMLLogger.cpp
   Logger.cpp
+  RecordOps.cpp
   Transfer.cpp
   TypeErasedDataflowAnalysis.cpp
   Value.cpp
   WatchedLiteralsSolver.cpp
   DebugSupport.cpp
 
   DEPENDS
   clangAnalysisFlowSensitiveResources

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (It != MemberLocToStruct.end()) {
 
     assert(It->second.second != nullptr);
     const ValueDecl &Member = *It->second.second;
 
     StructVal.setChild(Member, Val);
   }
 }
 
+void Environment::clearValue(const StorageLocation &Loc) {
+  LocToVal.erase(&Loc);
+
+  if (auto It = MemberLocToStruct.find(&Loc); It != MemberLocToStruct.end()) {
+    // `Loc` is the location of a struct member so we need to also clear the
+    // member in the corresponding `StructValue`.
+
+    assert(It->second.first != nullptr);
+    StructValue &StructVal = *It->second.first;
+
+    assert(It->second.second != nullptr);
+    const ValueDecl &Member = *It->second.second;
+
+    StructVal.clearChild(Member);
+  }
+}
+
 void Environment::setValueStrict(const Expr &E, Value &Val) {
   assert(E.isPRValue());
   assert(!isa<ReferenceValue>(Val));
 
   StorageLocation *Loc = getStorageLocation(E, SkipPast::None);
   if (Loc == nullptr) {
     Loc = &createStorageLocation(E);
     setStorageLocation(E, *Loc);

clang/lib/Analysis/FlowSensitive/RecordOps.cpp

+//===-- RecordOps.cpp -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  Operations on records (structs, classes, and unions).
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
+
+#define DEBUG_TYPE "dataflow"
+
+namespace clang {
+namespace dataflow {
+
+void copyRecord(AggregateStorageLocation &Src, AggregateStorageLocation &Dst,
+                Environment &Env) {
+  QualType Type = Src.getType();
+
+  LLVM_DEBUG({
+    if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
+        Type.getCanonicalType().getUnqualifiedType()) {
+      llvm::dbgs() << "Source type " << Type << "\n";
+      llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
+    }
+  });
+  assert(Dst.getType().getCanonicalType().getUnqualifiedType() ==
+         Type.getCanonicalType().getUnqualifiedType());
+
+  for (auto [Field, SrcFieldLoc] : Src.children()) {
+    assert(SrcFieldLoc != nullptr);
+
+    StorageLocation &DstFieldLoc = Dst.getChild(*Field);
+
+    if (Field->getType()->isRecordType()) {
+      copyRecord(cast<AggregateStorageLocation>(*SrcFieldLoc),
+                 cast<AggregateStorageLocation>(DstFieldLoc), Env);
+    } else {
+      if (Value *Val = Env.getValue(*SrcFieldLoc))
+        Env.setValue(DstFieldLoc, *Val);
+      else
+        Env.clearValue(DstFieldLoc);
+    }
+  }
+
+  StructValue *SrcVal = cast_or_null<StructValue>(Env.getValue(Src));
+  StructValue *DstVal = cast_or_null<StructValue>(Env.getValue(Dst));
+
+  if (SrcVal == nullptr || DstVal == nullptr)
+    return;
+
+  auto DstChildren = DstVal->children();
+  DstVal = &Env.create<StructValue>(llvm::DenseMap<const ValueDecl *, Value *>(
+      DstChildren.begin(), DstChildren.end()));
+  Env.setValue(Dst, *DstVal);
+
+  for (const auto &[Name, Value] : SrcVal->properties()) {
+    if (Value != nullptr)
+      DstVal->setProperty(Name, *Value);
+  }
+}
+
+bool recordsEqual(const AggregateStorageLocation &Loc1, const Environment &Env1,
+                  const AggregateStorageLocation &Loc2,
+                  const Environment &Env2) {
+  QualType Type = Loc1.getType();
+
+  LLVM_DEBUG({
+    if (Loc2.getType().getCanonicalType().getUnqualifiedType() !=
+        Type.getCanonicalType().getUnqualifiedType()) {
+      llvm::dbgs() << "Loc1 type " << Type << "\n";
+      llvm::dbgs() << "Loc2 type " << Loc2.getType() << "\n";
+    }
+  });
+  assert(Loc2.getType().getCanonicalType().getUnqualifiedType() ==
+         Type.getCanonicalType().getUnqualifiedType());
+
+  for (auto [Field, FieldLoc1] : Loc1.children()) {
+    assert(FieldLoc1 != nullptr);
+
+    StorageLocation &FieldLoc2 = Loc2.getChild(*Field);
+
+    if (Field->getType()->isRecordType()) {
+      if (!recordsEqual(cast<AggregateStorageLocation>(*FieldLoc1), Env1,
+                        cast<AggregateStorageLocation>(FieldLoc2), Env2))
+        return false;
+    } else if (Field->getType()->isReferenceType()) {
+      auto *RefVal1 = cast_or_null<ReferenceValue>(Env1.getValue(*FieldLoc1));
+      auto *RefVal2 = cast_or_null<ReferenceValue>(Env1.getValue(FieldLoc2));
+      if (RefVal1 && RefVal2) {
+        if (&RefVal1->getReferentLoc() != &RefVal2->getReferentLoc())
+          return false;
+      } else {
+        // If either of `RefVal1` and `RefVal2` is null, we only consider them
+        // equal if they're both null.
+        if (RefVal1 || RefVal2)
+          return false;
+      }
+    } else {
+      if (Env1.getValue(*FieldLoc1) != Env2.getValue(FieldLoc2))
+        return false;
+    }
+  }
+
+  llvm::StringMap<Value *> Props1, Props2;
+
+  if (StructValue *Val1 = cast_or_null<StructValue>(Env1.getValue(Loc1)))
+    for (const auto &[Name, Value] : Val1->properties())
+      Props1[Name] = Value;
+  if (StructValue *Val2 = cast_or_null<StructValue>(Env2.getValue(Loc2)))
+    for (const auto &[Name, Value] : Val2->properties())
+      Props2[Name] = Value;
+
+  if (Props1.size() != Props2.size())
+    return false;
+
+  for (const auto &[Name, Value] : Props1) {
+    auto It = Props2.find(Name);
+    if (It == Props2.end())
+      return false;
+    if (Value != It->second)
+      return false;
+  }
+
+  return true;
+}
+
+} // namespace dataflow
+} // namespace clang

clang/lib/Analysis/FlowSensitive/Transfer.cpp

 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <memory>
void VisitCXXConstructExpr(const CXXConstructExpr *S) {
     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.
       assert(S->getNumArgs() != 0);
 
       const Expr *Arg = S->getArg(0);
       assert(Arg != nullptr);
 
-      if (S->isElidable()) {
-        auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
-        if (ArgLoc == nullptr)
-          return;
+      auto *ArgLoc = cast<AggregateStorageLocation>(
+          Env.getStorageLocation(*Arg, SkipPast::Reference));
 
+      if (S->isElidable()) {
         Env.setStorageLocation(*S, *ArgLoc);
-      } else if (auto *ArgVal = Env.getValue(*Arg, SkipPast::Reference)) {
-        auto &Loc = Env.createStorageLocation(*S);
+      } else {
+        auto &Loc =
+            cast<AggregateStorageLocation>(Env.createStorageLocation(*S));
         Env.setStorageLocation(*S, Loc);
-        Env.setValue(Loc, *ArgVal);
+        if (Value *Val = Env.createValue(S->getType())) {
+          Env.setValue(Loc, *Val);
+          copyRecord(*ArgLoc, Loc, Env);
+        }
       }
       return;
     }
 
     auto &Loc = Env.createStorageLocation(*S);
     Env.setStorageLocation(*S, Loc);
     if (Value *Val = Env.createValue(S->getType()))
       Env.setValue(Loc, *Val);
if (S->getOperator() == OO_Equal) {
       const auto *Method =
           dyn_cast_or_null<CXXMethodDecl>(S->getDirectCallee());
       if (!Method)
         return;
       if (!Method->isCopyAssignmentOperator() &&
           !Method->isMoveAssignmentOperator())
         return;
 
-      auto *ObjectLoc = Env.getStorageLocation(*Arg0, SkipPast::Reference);
+      auto *ObjectLoc = cast_or_null<AggregateStorageLocation>(
+          Env.getStorageLocation(*Arg0, SkipPast::Reference));
       if (ObjectLoc == nullptr)
         return;
 
-      auto *Val = Env.getValue(*Arg1, SkipPast::Reference);
-      if (Val == nullptr)
+      auto *ArgLoc = cast_or_null<AggregateStorageLocation>(
+          Env.getStorageLocation(*Arg1, SkipPast::Reference));
+      if (ArgLoc == nullptr)
         return;
 
-      // Assign a value to the storage location of the object.
-      Env.setValue(*ObjectLoc, *Val);
+      copyRecord(*ArgLoc, *ObjectLoc, Env);
 
       // FIXME: Add a test for the value of the whole expression.
       // Assign a storage location for the whole expression.
       Env.setStorageLocation(*S, *ObjectLoc);
     }
   }
 
   void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *S) {

clang/unittests/Analysis/FlowSensitive/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
   ArenaTest.cpp
   CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
   DebugSupportTest.cpp
   LoggerTest.cpp
   MapLatticeTest.cpp
   MatchSwitchTest.cpp
   MultiVarConstantPropagationTest.cpp
+  RecordOpsTest.cpp
   SignAnalysisTest.cpp
   SingleVarConstantPropagationTest.cpp
   SolverTest.cpp
   TestingSupport.cpp
   TestingSupportTest.cpp
   TransferBranchTest.cpp
   TransferTest.cpp
   TypeErasedDataflowAnalysisTest.cpp

clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp

+//===- unittests/Analysis/FlowSensitive/RecordOpsTest.cpp -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
+#include "TestingSupport.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace dataflow {
+namespace test {
+namespace {
+
+template <typename VerifyResultsT>
+void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
+                 LangStandard::Kind Std = LangStandard::lang_cxx17,
+                 llvm::StringRef TargetFun = "target") {
+  ASSERT_THAT_ERROR(
+      runDataflowReturnError(Code, VerifyResults,
+                             DataflowAnalysisOptions{BuiltinOptions{}}, Std,
+                             TargetFun),
+      llvm::Succeeded());
+}
+
+TEST(RecordOpsTest, CopyRecord) {
+  std::string Code = R"(
+    struct S {
+      int outer_int;
+      int &ref;
+      struct {
+        int inner_int;
+      } inner;
+    };
+    void target(S s1, S s2) {
+      (void)s1.outer_int;
+      (void)s1.ref;
+      (void)s1.inner.inner_int;
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        Environment Env = getEnvironmentAtAnnotation(Results, "p");
+
+        const ValueDecl *OuterIntDecl = findValueDecl(ASTCtx, "outer_int");
+        const ValueDecl *RefDecl = findValueDecl(ASTCtx, "ref");
+        const ValueDecl *InnerDecl = findValueDecl(ASTCtx, "inner");
+        const ValueDecl *InnerIntDecl = findValueDecl(ASTCtx, "inner_int");
+
+        auto &S1 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s1");
+        auto &S2 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s2");
+        auto &Inner1 = cast<AggregateStorageLocation>(S1.getChild(*InnerDecl));
+        auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
+
+        EXPECT_NE(Env.getValue(S1.getChild(*OuterIntDecl)),
+                  Env.getValue(S2.getChild(*OuterIntDecl)));
+        EXPECT_NE(Env.getValue(S1.getChild(*RefDecl)),
+                  Env.getValue(S2.getChild(*RefDecl)));
+        EXPECT_NE(Env.getValue(Inner1.getChild(*InnerIntDecl)),
+                  Env.getValue(Inner2.getChild(*InnerIntDecl)));
+
+        auto *S1Val = cast<StructValue>(Env.getValue(S1));
+        auto *S2Val = cast<StructValue>(Env.getValue(S2));
+        EXPECT_NE(S1Val, S2Val);
+
+        S1Val->setProperty("prop", Env.getBoolLiteralValue(true));
+
+        copyRecord(S1, S2, Env);
+
+        EXPECT_EQ(Env.getValue(S1.getChild(*OuterIntDecl)),
+                  Env.getValue(S2.getChild(*OuterIntDecl)));
+        EXPECT_EQ(Env.getValue(S1.getChild(*RefDecl)),
+                  Env.getValue(S2.getChild(*RefDecl)));
+        EXPECT_EQ(Env.getValue(Inner1.getChild(*InnerIntDecl)),
+                  Env.getValue(Inner2.getChild(*InnerIntDecl)));
+
+        S1Val = cast<StructValue>(Env.getValue(S1));
+        S2Val = cast<StructValue>(Env.getValue(S2));
+        EXPECT_NE(S1Val, S2Val);
+
+        EXPECT_EQ(S2Val->getProperty("prop"), &Env.getBoolLiteralValue(true));
+      });
+}
+
+TEST(RecordOpsTest, RecordsEqual) {
+  std::string Code = R"(
+    struct S {
+      int outer_int;
+      int &ref;
+      struct {
+        int inner_int;
+      } inner;
+    };
+    void target(S s1, S s2) {
+      (void)s1.outer_int;
+      (void)s1.ref;
+      (void)s1.inner.inner_int;
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        Environment Env = getEnvironmentAtAnnotation(Results, "p");
+
+        const ValueDecl *OuterIntDecl = findValueDecl(ASTCtx, "outer_int");
+        const ValueDecl *RefDecl = findValueDecl(ASTCtx, "ref");
+        const ValueDecl *InnerDecl = findValueDecl(ASTCtx, "inner");
+        const ValueDecl *InnerIntDecl = findValueDecl(ASTCtx, "inner_int");
+
+        auto &S1 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s1");
+        auto &S2 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s2");
+        auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
+
+        cast<StructValue>(Env.getValue(S1))
+            ->setProperty("prop", Env.getBoolLiteralValue(true));
+
+        // Strategy: Create two equal records, then verify each of the various
+        // ways in which records can differ causes recordsEqual to return false.
+        // changes we can make to the record.
+
+        // This test reuses the same objects for multiple checks, which isn't
+        // great, but seems better than duplicating the setup code for every
+        // check.
+
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S2 has a different outer_int.
+        Env.setValue(S2.getChild(*OuterIntDecl), Env.create<IntegerValue>());
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S2 doesn't have outer_int at all.
+        Env.clearValue(S2.getChild(*OuterIntDecl));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S2 has a different ref.
+        Env.setValue(S2.getChild(*RefDecl),
+                     Env.create<ReferenceValue>(Env.createStorageLocation(
+                         RefDecl->getType().getNonReferenceType())));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S2 doesn't have ref at all.
+        Env.clearValue(S2.getChild(*RefDecl));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S2 as a different inner_int.
+        Env.setValue(Inner2.getChild(*InnerIntDecl),
+                     Env.create<IntegerValue>());
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S1 and S2 have the same property with different values.
+        cast<StructValue>(Env.getValue(S2))
+            ->setProperty("prop", Env.getBoolLiteralValue(false));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S1 has a property that S2 doesn't have.
+        cast<StructValue>(Env.getValue(S1))
+            ->setProperty("other_prop", Env.getBoolLiteralValue(false));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        // We modified S1 this time, so need to copy back the other way.
+        copyRecord(S2, S1, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S2 has a property that S1 doesn't have.
+        cast<StructValue>(Env.getValue(S2))
+            ->setProperty("other_prop", Env.getBoolLiteralValue(false));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+        copyRecord(S1, S2, Env);
+        EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+        // S1 and S2 have the same number of properties, but with different
+        // names.
+        cast<StructValue>(Env.getValue(S1))
+            ->setProperty("prop1", Env.getBoolLiteralValue(false));
+        cast<StructValue>(Env.getValue(S2))
+            ->setProperty("prop2", Env.getBoolLiteralValue(false));
+        EXPECT_FALSE(recordsEqual(S1, S2, Env));
+      });
+}
+
+} // namespace
+} // namespace test
+} // namespace dataflow
+} // namespace clang

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
return checkDataflow<AnalysisT>(
         VerifyResults(AnnotationStates, AO);
 
         // `checkDataflow()` can analyze more than one function.  Reset the
         // variables to prepare for analyzing the next function.
         AnnotationStates.clear();
       });
 }
 
+using BuiltinOptions = DataflowAnalysisContext::Options;
+
+/// Runs dataflow on `Code` with a `NoopAnalysis` and calls `VerifyResults` to
+/// verify the results.
+template <typename VerifyResultsT>
+llvm::Error
+runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
+                       DataflowAnalysisOptions Options,
+                       LangStandard::Kind Std = LangStandard::lang_cxx17,
+                       llvm::StringRef TargetFun = "target") {
+  using ast_matchers::hasName;
+  llvm::SmallVector<std::string, 3> ASTBuildArgs = {
+      // -fnodelayed-template-parsing is the default everywhere but on Windows.
+      // Set it explicitly so that tests behave the same on Windows as on other
+      // platforms.
+      "-fsyntax-only", "-fno-delayed-template-parsing",
+      "-std=" +
+          std::string(LangStandard::getLangStandardForKind(Std).getName())};
+  AnalysisInputs<NoopAnalysis> AI(
+      Code, hasName(TargetFun),
+      [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
+                                                          Environment &Env) {
+        return NoopAnalysis(
+            C,
+            DataflowAnalysisOptions{
+                UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
+                                : std::optional<BuiltinOptions>()});
+      });
+  AI.ASTBuildArgs = ASTBuildArgs;
+  if (Options.BuiltinOpts)
+    AI.BuiltinOptions = *Options.BuiltinOpts;
+  return checkDataflow<NoopAnalysis>(
+      std::move(AI),
+      /*VerifyResults=*/
+      [&VerifyResults](
+          const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+          const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
+}
+
 /// Returns the `ValueDecl` for the given identifier.
 ///
 /// Requirements:
 ///
 ///   `Name` must be unique in `ASTCtx`.
 const ValueDecl *findValueDecl(ASTContext &ASTCtx, llvm::StringRef Name);
 
+/// Returns the storage location (of type `LocT`) for the given identifier.
+/// `LocT` must be a subclass of `StorageLocation` and must be of the
+/// appropriate type.
+///
+/// Requirements:
+///
+///   `Name` must be unique in `ASTCtx`.
+template <class LocT>
+LocT &getLocForDecl(ASTContext &ASTCtx, const Environment &Env,
+                    llvm::StringRef Name) {
+  const ValueDecl *VD = findValueDecl(ASTCtx, Name);
+  assert(VD != nullptr);
+  return *cast<LocT>(Env.getStorageLocation(*VD));
+}
+
 /// Returns the value (of type `ValueT`) for the given identifier.
 /// `ValueT` must be a subclass of `Value` and must be of the appropriate type.
 ///
 /// Requirements:
 ///
 ///   `Name` must be unique in `ASTCtx`.
 template <class ValueT>
 ValueT &getValueForDecl(ASTContext &ASTCtx, const Environment &Env,

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

 //===- unittests/Analysis/FlowSensitive/TransferTest.cpp ------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Testing/Support/Error.h"

 using namespace dataflow;
 using namespace test;
 using ::testing::Eq;
 using ::testing::IsNull;
 using ::testing::Ne;
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
-using BuiltinOptions = DataflowAnalysisContext::Options;
-
-template <typename Matcher>
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, Matcher Match,
-                       DataflowAnalysisOptions Options,
-                       LangStandard::Kind Std = LangStandard::lang_cxx17,
-                       llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector<std::string, 3> ASTBuildArgs = {
-      // -fnodelayed-template-parsing is the default everywhere but on Windows.
-      // Set it explicitly so that tests behave the same on Windows as on other
-      // platforms.
-      "-fsyntax-only", "-fno-delayed-template-parsing",
-      "-std=" +
-          std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs<NoopAnalysis> AI(
-      Code, hasName(TargetFun),
-      [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-                                                          Environment &Env) {
-        return NoopAnalysis(
-            C,
-            DataflowAnalysisOptions{
-                UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-                                : std::optional<BuiltinOptions>()});
-      });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-    AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow<NoopAnalysis>(
-      std::move(AI),
-      /*VerifyResults=*/
-      [&Match](
-          const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
-          const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); });
-}
-
-template <typename Matcher>
-void runDataflow(llvm::StringRef Code, Matcher Match,
+template <typename VerifyResultsT>
+void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
                  DataflowAnalysisOptions Options,
                  LangStandard::Kind Std = LangStandard::lang_cxx17,
                  llvm::StringRef TargetFun = "target") {
   ASSERT_THAT_ERROR(
-      runDataflowReturnError(Code, Match, Options, Std, TargetFun),
+      runDataflowReturnError(Code, VerifyResults, Options, Std, TargetFun),
       llvm::Succeeded());
 }
 
-template <typename Matcher>
-void runDataflow(llvm::StringRef Code, Matcher Match,
+template <typename VerifyResultsT>
+void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
                  LangStandard::Kind Std = LangStandard::lang_cxx17,
                  bool ApplyBuiltinTransfer = true,
                  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, std::move(Match),
+  runDataflow(Code, std::move(VerifyResults),
               {ApplyBuiltinTransfer ? BuiltinOptions{}
                                     : std::optional<BuiltinOptions>()},
               Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
   std::string Code = R"(
     void target() {

 
 TEST(TransferTest, AssignmentOperator) {
   std::string Code = R"(
     struct A {
       int Baz;
     };
 
     void target() {
-      A Foo;
-      A Bar;
-      (void)Foo.Baz;
+      A Foo = { 1 };
+      A Bar = { 2 };
       // [[p1]]
       Foo = Bar;
       // [[p2]]
+      Foo.Baz = 3;
+      // [[p3]]
     }
   )";
   runDataflow(
       Code,
       [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
          ASTContext &ASTCtx) {
-        ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
-        const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
-        const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
-
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
 
+        // Before copy assignment.
+        {
+          const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
+
           const auto *FooLoc1 =
               cast<AggregateStorageLocation>(Env1.getStorageLocation(*FooDecl));
           const auto *BarLoc1 =
               cast<AggregateStorageLocation>(Env1.getStorageLocation(*BarDecl));
-
-        const auto *FooVal1 = cast<StructValue>(Env1.getValue(*FooLoc1));
-        const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1));
-        EXPECT_NE(FooVal1, BarVal1);
+          EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
 
           const auto *FooBazVal1 =
               cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl)));
           const auto *BarBazVal1 =
               cast<IntegerValue>(Env1.getValue(BarLoc1->getChild(*BazDecl)));
           EXPECT_NE(FooBazVal1, BarBazVal1);
+        }
+
+        // After copy assignment.
+        {
+          const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
 
           const auto *FooLoc2 =
               cast<AggregateStorageLocation>(Env2.getStorageLocation(*FooDecl));
           const auto *BarLoc2 =
               cast<AggregateStorageLocation>(Env2.getStorageLocation(*BarDecl));
 
           const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2));
           const auto *BarVal2 = cast<StructValue>(Env2.getValue(*BarLoc2));
-        EXPECT_EQ(FooVal2, BarVal2);
+          EXPECT_NE(FooVal2, BarVal2);
+
+          EXPECT_TRUE(recordsEqual(*FooLoc2, *BarLoc2, Env2));
 
           const auto *FooBazVal2 =
-            cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl)));
+              cast<IntegerValue>(Env2.getValue(FooLoc2->getChild(*BazDecl)));
           const auto *BarBazVal2 =
-            cast<IntegerValue>(Env2.getValue(BarLoc1->getChild(*BazDecl)));
+              cast<IntegerValue>(Env2.getValue(BarLoc2->getChild(*BazDecl)));
           EXPECT_EQ(FooBazVal2, BarBazVal2);
+        }
+
+        // After value update.
+        {
+          const Environment &Env3 = getEnvironmentAtAnnotation(Results, "p3");
+
+          const auto *FooLoc3 =
+              cast<AggregateStorageLocation>(Env3.getStorageLocation(*FooDecl));
+          const auto *BarLoc3 =
+              cast<AggregateStorageLocation>(Env3.getStorageLocation(*BarDecl));
+          EXPECT_FALSE(recordsEqual(*FooLoc3, *BarLoc3, Env3));
+
+          const auto *FooBazVal3 =
+              cast<IntegerValue>(Env3.getValue(FooLoc3->getChild(*BazDecl)));
+          const auto *BarBazVal3 =
+              cast<IntegerValue>(Env3.getValue(BarLoc3->getChild(*BazDecl)));
+          EXPECT_NE(FooBazVal3, BarBazVal3);
+        }
+      });
+}
+
+TEST(TransferTest, AssignmentOperatorFromCallResult) {
+  std::string Code = R"(
+    struct A {};
+    A ReturnA();
+
+    void target() {
+      A MyA;
+      MyA = ReturnA();
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        // As of this writing, we don't produce a `Value` for the call
+        // `ReturnA()`. The only condition we're testing for is that the
+        // analysis should not crash in this case.
       });
 }
 
 TEST(TransferTest, CopyConstructor) {
   std::string Code = R"(
     struct A {
       int Baz;
     };
 
     void target() {
-      A Foo;
-      (void)Foo.Baz;
+      A Foo = { 1 };
       A Bar = Foo;
-      // [[p]]
+      // [[after_copy]]
+      Foo.Baz = 2;
+      // [[after_update]]
     }
   )";
   runDataflow(
       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());
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
 
+        // after_copy
+        {
+          const Environment &Env =
+              getEnvironmentAtAnnotation(Results, "after_copy");
+
           const auto *FooLoc =
               cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
           const auto *BarLoc =
               cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
 
+          // `Foo` and `Bar` have different `StructValue`s associated with them.
           const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
           const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
-        EXPECT_EQ(FooVal, BarVal);
+          EXPECT_NE(FooVal, BarVal);
 
+          // But the records compare equal.
+          EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
+
+          // In particular, the value of `Baz` in both records is the same.
           const auto *FooBazVal =
               cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
           const auto *BarBazVal =
               cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
           EXPECT_EQ(FooBazVal, BarBazVal);
+        }
+
+        // after_update
+        {
+          const Environment &Env =
+              getEnvironmentAtAnnotation(Results, "after_update");
+
+          const auto *FooLoc =
+              cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
+          const auto *BarLoc =
+              cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
+
+          EXPECT_FALSE(recordsEqual(*FooLoc, *BarLoc, Env));
+
+          const auto *FooBazVal =
+              cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
+          const auto *BarBazVal =
+              cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
+          EXPECT_NE(FooBazVal, BarBazVal);
+        }
       });
 }
 
 TEST(TransferTest, CopyConstructorWithDefaultArgument) {
   std::string Code = R"(
     struct A {
       int Baz;
       A() = default;
runDataflow(
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
 
         const auto *FooLoc =
             cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
         const auto *BarLoc =
             cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
-
-        const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
-        const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
-        EXPECT_EQ(FooVal, BarVal);
+        EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
 
         const auto *FooBazVal =
             cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
         const auto *BarBazVal =
             cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
         EXPECT_EQ(FooBazVal, BarBazVal);
       });
 }
runDataflow(
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
 
         const auto *FooLoc =
             cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
         const auto *BarLoc =
             cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
-
-        const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
-        const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
-        EXPECT_EQ(FooVal, BarVal);
+        EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
 
         const auto *FooBazVal =
             cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
         const auto *BarBazVal =
             cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
         EXPECT_EQ(FooBazVal, BarBazVal);
       });
 }
runDataflow(
             cast<AggregateStorageLocation>(Env1.getStorageLocation(*FooDecl));
         const auto *BarLoc1 =
             cast<AggregateStorageLocation>(Env1.getStorageLocation(*BarDecl));
 
         const auto *FooVal1 = cast<StructValue>(Env1.getValue(*FooLoc1));
         const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1));
         EXPECT_NE(FooVal1, BarVal1);
 
+        EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
+
         const auto *FooBazVal1 =
             cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl)));
         const auto *BarBazVal1 =
             cast<IntegerValue>(Env1.getValue(BarLoc1->getChild(*BazDecl)));
         EXPECT_NE(FooBazVal1, BarBazVal1);
 
         const auto *FooLoc2 =
             cast<AggregateStorageLocation>(Env2.getStorageLocation(*FooDecl));
         const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2));
-        EXPECT_EQ(FooVal2, BarVal1);
+        EXPECT_NE(FooVal2, BarVal1);
+        EXPECT_TRUE(recordsEqual(*FooLoc2, Env2, *BarLoc1, Env1));
 
         const auto *FooBazVal2 =
             cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl)));
         EXPECT_EQ(FooBazVal2, BarBazVal1);
       });
 }
 
 TEST(TransferTest, BindTemporary) {