[region-isolation] Fix join to be a true union operation including the symmetric difference. #69423
Conversation
…st CMakeLists.txt I forgot to do this when moving the unittests so we were not building/running them.
…e symmetric difference.
We were performing a union on the intersection of the lhs/rhs but were dropping
the parts of lhs/rhs that were in the symmetric difference of the two sets.
Without this, we would not diagnose cases like this where we had elements on the
lhs/rhs that were not in the intersection.
```
var closure: () -> () = {}
await transferToMain(closure)
if await booleanFlag {
closure = {
print(self.klass)
}
} else {
closure = {}
}
// At this point we would lose closure since they were different elements
await transferToMain(closure) // We wouldn't error on this!
```
rdar://117437059
|
@swift-ci smoke test |
|
I messed up a little bit of the output from one test for without region isolation when I updated it for region isolation... oops. Posting a commit. |
gottesmm
force-pushed
the
pr-4c7e13d3cbc1677f17c72b86d9a02042e97eded1
branch
from
96ec732 to
93e98aa
Compare
|
@swift-ci smoke test |
|
@swift-ci smoke test |
|
@swift-ci smoke test |
1 similar comment
|
@swift-ci smoke test |
|
@swift-ci smoke test |
| public: | ||
| /// A class defined in PartitionUtils unittest used to grab state from | ||
| /// Partition without exposing it to other users. | ||
| struct PartitionTester; |
There was a problem hiding this comment.
Could this be declared in the test fixture and become a friend struct here instead that way getRegion could also be declared in the tester struct itself?
| return p; | ||
|
|
||
| Region max_index = | ||
| Region(*std::max_element(indices.begin(), indices.end())); |
There was a problem hiding this comment.
Maybe fresh_label could be assigned after we went over indices, seems like max_index could be computed by that loop?
| ASSERT_EQ(tester.getRegion(41), 13); | ||
| ASSERT_EQ(tester.getRegion(42), 33); | ||
| ASSERT_EQ(tester.getRegion(43), 13); | ||
| ASSERT_EQ(tester.getRegion(44), 3); |
There was a problem hiding this comment.
A suggestion for future testing - random number of elements assigned randomly, we could keep track of all of these assignments in a map and double-check that join is correct via double-checking that map in a loop.
There was a problem hiding this comment.
@xedin I did that here. I just generated it offline.
|
I will fix @xedin 's suggestions in a follow on. |
We were performing a union on the intersection of the lhs/rhs but were dropping
the parts of lhs/rhs that were in the symmetric difference of the two sets.
Without this, we would not diagnose cases like this where we had elements on the
lhs/rhs that were not in the intersection.
rdar://117437059