Sema's sendable checking is subsumed by the region isolation SIL pass. Now
that region isolation is always enabled under complete concurrency checking,
the code can be deleted from the actor isolation checker. Note that this
removes these diagnostics from targeted concurrency checking. I think it's
better to remove these diagnostics from targeted checking because in many
cases, they're false positive data-race reports that the programmer ultimately
won't have to address. If we want these diagnostics in targeted checking, we
should do it via region isolation.
Otherwise, we will assume that an async let autoclosure infers isolation from
its DeclContext... which we do not want. An async let autoclosure should always
be nonisolated + sending.
The diagnostic change that I mentioned in the header is that we were emitting
unfortunate "sending task or actor isolated could result in races" error. I
eliminated this by adding a new diagnostic for transfer non transferrable errors
happening in autoclosures. So now we emit this:
```swift
func asyncLetInferAsNonIsolated<T : Actor>(
isolation actor: isolated T
) async throws {
async let subTask: Void = {
await useValueAsyncNoReturnWithInstance(self, actor)
// expected-warning @-1:47 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
}()
await subTask
```
I also noticed that we did not have enough test cases for autoclosures in
general so I also added a bunch of tests just so we can see what the current
behavior is. I think there are a few issues therein (I believe some may have
been reported due to '??').
rdar://130151318
TLDR: This makes it so that we always can parse sending/transferring but changes
the semantic language effects to be keyed on RegionBasedIsolation instead.
----
The key thing that makes this all work is that I changed all of the "special"
semantic changes originally triggered on *ArgsAndResults to now be triggered
based on RegionBasedIsolation being enabled. This makes a lot of sense since we
want these semantic changes specifically to be combined with the checkers that
RegionBasedIsolation turns on. As a result, even though this causes these two
features to always be enabled, we just parse it but we do not use it for
anything semantically.
rdar://128961672
A few things:
1. Internally except for in the parser and the clang importer, we only represent
'sending'. This means that it will be easy to remove 'transferring' once enough
time has passed.
2. I included a warning that suggested to the user to change 'transferring' ->
'sending'.
3. I duplicated the parsing diagnostics for 'sending' so both will still get
different sets of diagnostics for parsing issues... but anywhere below parsing,
I have just changed 'transferring' to 'sending' since transferring isn't
represented at those lower levels.
4. Since SendingArgsAndResults is always enabled when TransferringArgsAndResults
is enabled (NOTE not vis-a-versa), we know that we can always parse sending. So
we import "transferring" as "sending". This means that even if one marks a
function with "transferring", the compiler will guard it behind a
SendingArgsAndResults -D flag and in the imported header print out sending.
rdar://128216574
Some notes:
1. If the result is non-Sendable and we didn't infer something that is
transferring, we still emit the current sema error that says that one cannot
assign a non-Sendable value to an async let.
2. When region isolation is enabled, but transferring args and results are
disabled, we leave the async let semantics alone. This means that the async let
closure is still @Sendable and one cannot pass in non-Sendable values to it.
We do not emit these very much anymore. I am just keeping it in place a bit
longer... eventually, this should just be an unknown pattern error since if a
user sees this it won't compile and we would rather then send it to us so we can
fix it.
Specifically:
1. If the value is transferred such that it becomes part of an actor region, the
value is permanently part of the actor region as one would normally have.
2. If the value is just used in an async let or is used by a nonisolated async
function within the async let then while the async let is alive it cannot be
used. But once the async let has been awaited upon, we allow for it to be used
again.
rdar://117506395
This means that:
1. In test cases where minimal is the default (swift 5 without
-warn-concurrency), I added RUN lines for targeted, complete, and complete +
sns.
2. In test cases where complete is the default (swift 6, -warn-concurrency,
specified complete with -strict-concurrency), I added a send non-sendable run
line.
In each of these cases, I added additional expected-* lines as appropriate so
the tests can compile in each mode successfully.