These conversions are artificial conversions inserted by Sema since isolation is
not represented on interface types. We previously looked though:
1. A single conversion that added nonisolated(nonsending).
2. A two level conversion where the first conversion added Sendable and the
second added nonisolated(nonsending).
In this case, Sema is generating a single conversion that combines the Sendable
and isolation conversion.
To be consistent, I also updated the code that involved conversion from
execution caller to global actor as well in case we hit the same case there.
https://github.com/swiftlang/swift/issues/83812
rdar://158685169
Don't look through sugar when checking whether to print
`nonisolated(nonsending)` in parameter positions, because
underlying types would already be marked appropriately
and parameters themselves doesn't require the modifier
transfer even when the sugar is stripped.
Resolves: rdar://167050741
Inferred `nonisolated` from the context i.e. an extension should
mark the member as `@concurrent` instead it should behave as
`nonisolated(nonsending)` when `NonisolatedNonsendingByDefault`
is enabled.
Resolves: rdar://157789572
Looks like the issue has been fixed already, so adding a
test-case to make sure that it doesn't regress in the future.
Resolves: https://github.com/swiftlang/swift/issues/84509
Resolves: rdar://161307898
This is an "off-by-one" error because the code to emit bridged
arguments didn't account for the fact that "native" function
type can have a leading implicit isolation parameter if declaration
is `nonisolated(nonsending)`.
Resolves: rdar://164561176
Isolation boundary closures don't assume parent isolation and should
be able to get `nonisolated(nonsending)` transferred to them jus
like regular nonisolated closures do.
Resolves: rdar://163792371
Printing shouldn't rely on parameter declaration bit because it only
works in cases when there is an explicit `nonisolated(nonsending)`
modifier on the type.
Always print `nonisolated(nonsending)` before `sending`, `@escaping`
and other declaration attributes/modifiers to avoid parsing issues.
Resolves: rdar://164267736
With the feature enabled any function type without an explicit `@concurrent`
is going to be inferred to be `nonisolated(nonsending)`. `@concurrent` should
always be printed for diagnostic and other purposes because the isolation
checking would consider them to be `nonisolated(nonsending)` otherwise, especially
important to code produced by fix-its.
Resolves: rdar://161739470
We want SILGen to have a simplified view of its executor and know that whenever
one sees an Actor, it is an actual actor instead of a Builtin.Executor. This
just simplifies code. Also, we should eventually have an invariant that
Builtin.Executor should only be allowed in LoweredSIL after LowerHopToExecutor
has run. But that is a change for another day.
`nonisolated(nonsending)` is not presented in interface types
which means that references to declarations that have this attribute
require a function conversion to apply it. Function conversion
checking should detect that and avoid sendability checking for
situations like that but there is really no conversion there.
The isolation doesn't change in this cause and making function type
non-Sendable means that it woun't be able to leave the current concurrency
domain and the compiler won't generate a thunk.
If there are no explicit concurrency attributes, isolated parameters,
or captures associated with the closure it should infer `nonisolated(nonsending)`
for the parent conversion injected by the solver (this conversion is injected
because the solver cannot check captures to elide it).
The change pushes `isIsolationInferenceBoundaryClosure` check down
with added benefit of getting preconcurrency context from the parent.
Specifically, we were not inserting the implicit isolated parameter and were not
setting up the actor prologue. To keep this specific to nonisolated(nonsending)
code, I only setup the actor prologue if we know that we have something that is
nonisolated(nonsending).
I also ported some async initializer tests to run with/without
nonisolated(nonsending) just to increase code coverage.
rdar://156919493
The constraint solver does not reliably give closures a function type
that includes `nonisolated(noncaller)`, even when the immediate context
requires a conversion to such a type. We were trying to work around this
in SILGen, but the peephole only kicked in if the types matched exactly,
so a contextual conversion that e.g. added `throws` was still emitting
the closure as `@concurrent`, which is of course the wrong semantics.
It's relatively easy to avoid all this by just rewriting the closure's
type to include `nonisolated(nonsending)` at a point where we can reliably
decide that, and then SILGen doesn't have to peephole anything for
correctness.
Fixes rdar://155313349
Specifically:
1. When we convert a function to nonisolated(nonsending), we need to
make sure that in the thunk we hop upon return since nonisolated(nonsending)
functions are assumed to preserve the caller's isolation.
2. When we convert a function from nonisolated(nonsending), we need to
make sure that in the thunk we hop onto the actor that we are passing in as the
isolated parameter of the nonisolated(nonsending) function. This ensures that
the nonisolated(nonsending) function can assume that it is already on its
isolated parameter's actor at function entry.
rdar://155905383
`_openExistential` is type-checked in a special way which
means that we need to explicitly inject `nonisolated(nonsending)`
isolation when forming a reference to this builtin.
`withoutActuallyEscaping` is type-checked in a special way which
means that we need to explicitly inject `nonisolated(nonsending)`
isolation when forming a reference to this builtin.
The present approach is not prudent because `@concurrent` synchronous
functions, a natural extension, are a likely-to-happen future direction,
whereas the current inference rule is entirely grounded on `@concurrent`
being exclusive to async functions.
If we were to ship this rule, we would have to keep the promise for
backwards compatibility when implementing the aforementioned future
direction, replacing one inconsistency with another, and possibly
introducing new bug-prone expression checking code.
```swift
func foo(_: () -> Void) {}
func foo(_: () async -> Void) {}
// In a future without this change and `@concurrent` synchronous
// functions accepted, the first call resolves to the first overload,
// and the second call resolves to the second, despite `@concurrent` no
// longer implying `async`.
foo { }
foo { @concurrent in }
```
This change also drops the fix-it for removing `@concurrent` when used
on a synchronous closure. With the inference rule gone, and the
diagnosis delayed until after solution application, this error raises a
fairly balanced choice between removing the attribute and being
explicit about the effect, where a unilateral suggestion is quite
possibly more harmful than useful.
With `NonisolatedNonsendingByDefault` an explicit `nonisolated`
attribute in declaration context is inferred to mean `nonisolated(nonsending)`
and it should be printed as such in interface files and other places.
The inference logic that did didn't remove the original attribute
which meant that it would be printed twice i.e.
`nonisolated nonisolated(nonsending) func test() async` which is
incorrect and would fail swift interface validation.
Resolves: rdar://155847011
Global actor kind also appends type offset that indicates what
global actor to use with the type. All of the isolation kinds
should be placed above it to make sure that there is never a
clash when i.e. `MainActor` is serialized as id `1`.
Resolves: rdar://153487603
The reason why this failed is that concurrently to @xedin landing
79af04ccc4, I enabled
NonisolatedNonsendingByDefault on a bunch of other tests. That change broke the
test and so we needed to fix it.
This commit fixes a few issues that were exposed:
1. We do not propagate nonisolated(nonsending) into a closure if its inferred
context isolation is global actor isolated or if the closure captures an
isolated parameter. We previously just always inferred
nonisolated(nonsending). Unfortunately since we do not yet have capture
information in CSApply, this required us to put the isolation change into
TypeCheckConcurrency.cpp and basically have function conversions of the form:
```
(function_conversion_expr type="nonisolated(nonsending) () async -> Void"
(closure_expr type="() async -> ()" isolated_to_caller_isolation))
```
Notice how we have a function conversion to nonisolated(nonsending) from a
closure expr that has an isolation that is isolated_to_caller.
2. With this in hand, we found that this pattern caused us to first thunk a
nonisolated(nonsending) function to an @concurrent function and then thunk that
back to nonisolated(nonsending), causing the final function to always be
concurrent. I put into SILGen a peephole that recognizes this pattern and emits
the correct code.
3. With that in hand, we found that we were emitting nonisolated(nonsending)
parameters for inheritActorContext functions. This was then fixed by @xedin in
With all this in hand, closure literal isolation and all of the other RBI tests
with nonisolated(nonsending) enabled pass.
rdar://154969621
`@_inheritActorContext` is a form of isolation which precludes
direct use of inference of `nonisolated(nonsending)` and `@concurrent`
just like other isolation attributes/modifiers would i.e. `isolated`
or `@isolated(any)`.
Currently only declarations would get `nonisolated(nonsending)`
inferred if the upcoming flag is enabled, this changes extend
this to apply to asynchronous nonisolated function types as well.
Resolves: rdar://154808850
We sometimes mangle SILFunctionTypes when generating debug info
for reabstraction thunks, and these can have various exotic
parameter and result attributes. Two recent additions were
never plumbed through the mangler, causing assertion failures
when emitting debug info.
Fixes rdar://153730847.
Make sure that referencing `nonisolated(nonsending)` properties,
especially through a witness is as not treated as leaving the
isolation domain of the caller.
Resolves: rdar://153922620
If the requirement is `nonisolated(nonsending)` but witness is
synchronous, prevent actor isolation inference from requirements
because this isolation only applies to asynchronous declarations
at the moment.
Resolves: rdar://153680826
Prevent migration from handling declarations that come from a
module different from the current one, this is primarily a
problem for swiftinterfaces that can get rebuilt when the module
is imported by a model that has migration mode enabled.
Resolves: rdar://152687353
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
See the inline comments for more details. Depending on the closure's
type signature, sometimes adding the attribute will break code. Fix this
by also adding inferred effects to the signature in these cases.
This ensures that when we generate the vtable thunk for a
nonisolated(nonsending) override (or vis-a-versa), we get the ABI correct. I
also added tests for all of the relevant cases for vtables that we check for
protocols.
rdar://151394209
We were using the isolation from the witness not from the requirement which we
are supposed to do. The witness thunk thunks the isolation from the requirement
to the witness so from an ABI perspective it should have the interface implied
by the requirement's isolation since that is what callers of the witness method
will expect.
rdar://151394209
This broke when we split `@execution(...)` into `@concurrent` and
`nonisolated(nonsending)` because the latter became its own `TypeRepr`,
whereas the condition for whether to attempt migration diagnostics
inside `resolveASTFunctionType` is still based on the function type's
attributes alone.
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.
Perform `Sendable` checking on parameter/result of the function
type when conversion between asynchroneous functions results in
a loss of global actor isolation attribute because access would
result in data crossing an isolation boundary.
This is a warning until Swift language mode 6.
Resolves: rdar://130168104
