================
@@ -121,18 +121,18 @@ static Value *mergeDistinctValues(QualType Type, Value 
&Val1,
 
   Value *MergedVal = nullptr;
   if (auto *RecordVal1 = dyn_cast<RecordValue>(&Val1)) {
-    [[maybe_unused]] auto *RecordVal2 = cast<RecordValue>(&Val2);
-
-    // Values to be merged are always associated with the same location in
-    // `LocToVal`. The location stored in `RecordVal` should therefore also
-    // be the same.
-    assert(&RecordVal1->getLoc() == &RecordVal2->getLoc());
-
-    // `RecordVal1` and `RecordVal2` may have different properties associated
-    // with them. Create a new `RecordValue` without any properties so that we
-    // soundly approximate both values. If a particular analysis needs to merge
-    // properties, it should do so in `DataflowAnalysis::merge()`.
-    MergedVal = &MergedEnv.create<RecordValue>(RecordVal1->getLoc());
+    auto *RecordVal2 = cast<RecordValue>(&Val2);
+
+    if (&RecordVal1->getLoc() == &RecordVal2->getLoc())
+      // `RecordVal1` and `RecordVal2` may have different properties associated
+      // with them. Create a new `RecordValue` without any properties so that 
we
+      // soundly approximate both values. If a particular analysis needs to
+      // merge properties, it should do so in `DataflowAnalysis::merge()`.
+      MergedVal = &MergedEnv.create<RecordValue>(RecordVal1->getLoc());
+    else
+      // If the locations for the two records are different, need to create a
+      // completely new value.
----------------
martinboehme wrote:

Not sure exactly what you mean...

Whether the locations are the same or not doesn't really depend on the order in 
which we process CFG blocks. Instead, locations are guaranteed to be the same 
if the `RecordValue` is stored in `ExprToLoc`, and they are usually (but not 
always) different if the `RecordValue` is stored in `ExprToVal`.

The order in which we process CFG blocks affects whether we actually see 
different `RecordValue`s or not when the merge happens -- which makes merges 
difficult to trigger reliably from an integration test 
([example](https://github.com/google/crubit/blob/3ab9d49fde5a59262903c589bb6963b8db4c0541/nullability/test/merge.cc#L342)).
 But I'm not sure how relevant that is to the situation here?

Anyway, my hope is that all of this will go away relatively soon because we're 
now working on eliminating the `RecordStorageLocation` from `RecordValue`.

https://github.com/llvm/llvm-project/pull/65319
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to