Skip to content

Teach #isolation to respect the flow-sensitive nature of actor initializers #74225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

DougGregor
Copy link
Member

Actor initializers have a flow-sensitive property where they are isolated to the actor being initialized only after the actor instance itself is fully-initialized. However, this behavior was not being reflected in the expansion of #isolation, which was always expanding to self, even before self is fully formed.

This led to a source compatibility issue with code that used the async for..in loop within an actor initializer prior to the point where the actor was fully initialized, because the type checker is introducing the #isolation (SE-0421) but Definite Initialization properly rejects the use of self before it is initialized.

Address this issue by delaying the expansion of #isolation until after the actor is fully initialized. In SILGen, we introduce a new builtin for this case (and just this case) called flowSensitiveSelfIsolation, which takes in self as its argument and produces an (any Actor)?. Definite initialization does not treat this as a use of self. Rather, it tracks these builtins and replaces them either with self (if it is fully-initialized at this point) or nil (if it is not fully-initialized at this point), mirroring the flow-sensitive isolation semantics described in SE-0327.

Fixes rdar://127080037.

…ializers

Actor initializers have a flow-sensitive property where they are isolated
to the actor being initialized only after the actor instance itself is
fully-initialized. However, this behavior was not being reflected in
the expansion of `#isolation`, which was always expanding to `self`,
even before `self` is fully formed.

This led to a source compatibility issue with code that used the async
for..in loop within an actor initializer *prior* to the point where the
actor was fully initialized, because the type checker is introducing
the `#isolation` (SE-0421) but Definite Initialization properly rejects
the use of `self` before it is initialized.

Address this issue by delaying the expansion of `#isolation` until
after the actor is fully initialized. In SILGen, we introduce a new
builtin for this case (and *just* this case) called
`flowSensitiveSelfIsolation`, which takes in `self` as its argument
and produces an `(any Actor)?`. Definite initialization does not treat
this as a use of `self`. Rather, it tracks these builtins and
replaces them either with `self` (if it is fully-initialized at this
point) or `nil` (if it is not fully-initialized at this point),
mirroring the flow-sensitive isolation semantics described in SE-0327.

Fixes rdar://127080037.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

Distributed actors can be treated as actors by accessing the `asLocalActor`
property. When lowering `#isolation` in a distributed actor initializer,
use a separate builtin `flowSensitiveDistributedSelfIsolation` to
capture the conformance to `DistributedActor`, and have Definite
Initialization introduce the call to the `asLocalActor` getter when
needed.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great lgtm, Thank you!

@DougGregor DougGregor merged commit 25830d6 into swiftlang:main Jun 10, 2024
3 checks passed
@DougGregor DougGregor deleted the flow-sensitive-actor-init-isolation branch June 10, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants