Specifically, there is currently a bug in TypeCheckConcurrency.cpp where we do
not visit autoclosures. This causes us to never set the autoclosure's
ActorIsolation field like all other closures. For a long time we were able to
get away with this just by relying on the isolation of the decl context of the
autoclosure... but with the introduction of nonisolated(nonsending), we found
cases where the generated single curry autoclosure would necessarily be
different than its decl context (e.x.: a synchronous outer curry thunk that is
nonisolated that returns an inner curry thunk that is
nonisolated(nonsending)). This problem caused us to hit asserts later in the
compiler since the inner closure was actually nonisolated(nonsending), but we
were thinking that it should have been concurrent.
To work around this problem, I changed the type checker in
ced96aa5cd to explicitly set the isolation of
single curry thunk autoclosures when it generates them. The reason why we did
this is that it made it so that we did not have to have a potential large source
break in 6.2 by changing TypeCheckConcurrency.cpp to visit all autoclosures it
has not been visiting.
This caused a follow on issue where since we were now inferring the inner
autoclosure to have the correct isolation, in cases where we were creating a
double curry thunk for an access to a global actor isolated field of a
non-Sendable non-global actor isolated nominal type, we would have the outer
curry thunk have unspecified isolation instead of main actor isolation. An
example of this is the following:
```swift
class A {
var block: @MainActor () -> Void = {}
}
class B {
let a = A()
func d() {
a.block = c // Error! Passing task isolated 'self' to @MainActor closure.
}
@MainActor
func c() {}
}
```
This was unintentional. To work around this, this commit changes the type
checker to explicitly set the double curry thunk isolation to the correct value
when the type checker generates the double curry thunk in the same manner as it
does for single curry thunks and validates that if we do set the value to
something explicitly that it has the same value as the single curry thunk.
rdar://152522631
When the attribute is specified explicitly passing a `@concurrent`
closure to a global actor-isolated parameter or contextual type
should result in a conversion and closure itself should be nonisolated.
Resolves: rdar://151797372
Convert a bunch of places where we're dumping to stderr and calling
`abort` over to using `ABORT` such that the message gets printed to
the pretty stack trace. This ensures it gets picked up by
CrashReporter.
While building an initializer call the declaration reference
should have the same implicitness as the call when it doesn't
require thunking, otherwise don't attempt to mark autoclosures
as non-implicit because it could break assumptions elsewhere.
Some notes:
1. In most cases, I think we were getting lucky with this by just inferring the
closure's isolation from its decl context. In the specific case that we were
looking at here, this was not true since we are returning from an @concurrent
async function a nonisolated(nonsending) method that closes over self. This
occurs since even when NonisolatedNonsendingByDefault we want to start importing
objc async functions as nonisolated(nonsending).
2. I also discovered that in the ActorIsolationChecker we were not visiting the
inner autoclosure meaning that we never set the ActorIsolation field on the
closure. After some discussion with @xedin about potentially visiting the
function in the ActorIsolationChecker, we came to the conclusion that this was
likely to result in source stability changes. So we put in a targeted fix just
for autoclosures in this specific case by setting their actor isolation in the
type checker.
3. Beyond adding tests to objc_async_from_swift to make sure that when
NonisolatedNonsendingByDefault is disabled we do the right thing, I noticed that
we did not have any tests that actually tested the behavior around
objc_async_from_swift when NonisolatedNonsendingByDefault is enabled. So I added
the relevant test lines so we can be sure that we get correct behavior in such a
case.
rdar://150209093
Instead of passing in the substituted type, we pass in the
InFlightSubstitution. This allows the substituted type to be
recovered if needed, but we can now skip computing it for
the common case of LookUpConformanceInSubstitutionMap.
Check for unsafe constructs in all modes, so that we can emit the
"unsafe does not cover any unsafe constructs" warning consistently.
One does not need to write "unsafe" outside of strict memory safety
mode, but if you do... it needs to cover unsafe behavior.
Always infer `nonisolated(nonsending)` from context directly on
a closure unless the closure is marked as `@concurrent`, otherwise
the closure is not going to get correct isolation and going to hop
to the wrong executor in its preamble.
Resolves: rdar://149107104
Downgrade to a warning until the next language mode. This is
necessary since we previously missed coercing macro arguments to
parameter types, resulting in cases where closure arguments weren't
being treated as `async` when they should have been.
rdar://149328745
Previously we would avoid rewriting the arguments in CSApply, but
that can result in incorrect behavior in MiscDiagnostics passes, e.g
incorrectly treating all closure arguments as escaping. Make sure
we rewrite the arguments as we would in regular type-checking.
rdar://148665502
The code that determines whether a reference to a static method (that
is not a call) assumed that metatypes were always Sendable. This is no
longer the case, so update this code to go through the normal Sendable
checking on the metatype.
Previously we would avoid rewriting the arguments in CSApply, but
that can result in incorrect behavior in MiscDiagnostics passes, e.g
incorrectly treating all closure arguments as escaping. Make sure
we rewrite the arguments as we would in regular type-checking.
rdar://148665502
- Track environments for `PackExpansionExpr` directly
instead of using a locator.
- Split up the querying and creation of the environment
such that the mismatch logic can be done directly in
CSSimplify instead of duplicating it.
- Just store the environment directly instead of
the shape and UUID.
It should be possible to pass values with `any Sendable` as arguments
to `inout` parameters that expect `Any`. This is pretty much the same
as an l-value conversion.
Resolves: https://github.com/swiftlang/swift/issues/79361
Resolves: rdar://144794132
When calling a distributed function for an actor that might not be local,
the call can throw due to the distributed actor system producing an
error. The function might, independently, also throw. When the
function uses typed throws, we incorrectly treated the call is if it
would always throw the error type specified by the function. This
leads to incorrectly accepting invalid code, and compiler crashes in
SILGen.
The change here is to always mark calls to distributed functions
outside the actor as "implicitly throwing", which makes sure that we
treat the call sites as throwing 'any Error'. The actual handling of
the typed throw (from the local function) and the untyped throw (from
the distributed actor system) occurs in thunk generation in SILGen,
and was already handled correctly.
Fixes rdar://144093249, and undoes the ban introduced by rdar://136467528
