swift
82fa88a8 - [rbi] Fix logic around tracking of Sendable/non-Sendable field projections from Sendable/non-Sendable values

Commit
90 days ago
[rbi] Fix logic around tracking of Sendable/non-Sendable field projections from Sendable/non-Sendable values Previously, region-based isolation was not diagnosing cases where a non-Sendable value projected from a Sendable base that was MainActor isolated. For example: ```swift @MainActor struct MainActorBox { var value: NonSendableKlass? mutating func take() -> sending NonSendableKlass { if let value { self.value = nil return value // Should warn: main actor-isolated value cannot be 'sending' } else { preconditionFailure("Consumed twice") } } } ``` This was caused by two issues: 1. A logic bug in `AddressBaseComputingVisitor::visitAll` where we overwrote an already-found Sendable value when recursing. The visitor should only record the first Sendable value found, not subsequent ones. This caused us to incorrectly return the `@MainActor` `MainActorBox` as the base instead of emitting a require for the projected value. 2. struct_element_addr being transparent unconditionally causing us to not even emit a require at all. This commit has two parts: 1. I fixed the first two issues by fixing the logic bug and by making struct_element_addr only transparent if its operand and result are non-Sendable. I added logic that we have used in other such cases to handle non-Sendable operand/Sendable result as well as Sendable operand and non-Sendable result. I then added the same support for tuple_element_addr and unchecked_take_enum_data_addr since they suffered from a similar problem. 2. Adding the logic to handle variants where the operand was non-Sendable and the result was Sendable, caused a bunch of tests to break so I had to fix that. To fix the broken test cases, I had to make the compiler smarter about how it was inserting this require. To do this: 1. I checked if the access base of the projection was actually immutable or if we are an alloc_stack that there was a prefix path in the projection path of immutable projections that end in the alloc_stack. In such a case, we know that we do not need to require that the operand is available in the current isolation domain since we will never write to that piece of memory and once we have loaded the result from memory, we know that the value is Sendable so nothing bad can happen. 2. If the access base of the projection was mutable, I used the same logic that we used for alloc_box that were non-Sendable that stored a Sendable thing by changing operand to be a `require [mutable_base_of_sending]` on the result of the projection instead of requiring the projection's operand. This ensures that we handled important flow sensitive cases. rdar://169626088
Author
Committer
Parents
Loading