Distributed actor isolation #39558
Merged
Distributed actor isolation #39558
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove a few places where we are introducing extra distributed-related actor isolation checking that either isn't necesssary or is incorrect. Specifically: * Hopping to a global actor *from* a distributed-actor isolated context does not go through a distributed thunk (global actors can't have one). * "Unrestricted" declarations are always unrestricted, so we don't need an extra distributed check here (it won't ever occur). * Actor isolation computation doesn't need a special case for distributed; it marks too much, overriding (e.g.) global actor isolation. The primary semantic change visible here is that distributed actors can now have truly 'nonisolated' members. They aren't required to be 'distributed' because they can't touch state anyway.
ktoso
reviewed
Oct 2, 2021
| @@ -115,6 +114,9 @@ distributed actor DistributedActor_1 { | |||
| await self.distHelloAsync() | |||
| try self.distHelloThrows() | |||
| try await self.distHelloAsyncThrows() | |||
|
|
|||
| // Hops over to the global actor. | |||
| _ = await DistributedActor_1.staticMainActorFunc() | |||
There was a problem hiding this comment.
Yay! Glad that just works, we'll need to double check if we reject distributed func getting a global actor -- I'm guessing that's nonsense hmmm
| @@ -33,7 +31,7 @@ distributed actor DA { | |||
| // self is a distributed actor self is NOT isolated | |||
| _ = self.local // expected-error{{distributed actor-isolated property 'local' can only be referenced inside the distributed actor}} | |||
| _ = try await self.dist() // ok, was made implicitly throwing and async | |||
| _ = self.computedNonisolated // expected-error{{only 'distributed' functions can be called from outside the distributed actor}} | |||
| _ = self.computedNonisolated // it's okay, only the body of computedNonisolated is wrong | |||
There was a problem hiding this comment.
Uff, yes absolutely you're right on that
The `@__distributedActorIndependent` attribute is effectively the same as nonisolated, so start treating it that way by making actor-isolation checking look for it specifically and conclude "nonisolated". Remove various special cases for this attribute that don't need to exist.
This is an non-user-visible attribute that is semantically identical to `nonisolated` except that it allows use in distributed actors. It is only, and can only, be used for the two fields that distributed actors have in both the local actor and in the remote proxy, "id" and "actorTransport". So, special-case the "nonisolated" check for those fields and stop using `@_distributedActorIndependent`.
All its uses have been subsumed into `nonisolated`.
ktoso
reviewed
Oct 2, 2021
include/swift/AST/Attr.def
Outdated
| ABIStableToAdd | ABIStableToRemove | | ||
| APIBreakingToAdd | APIBreakingToRemove, | ||
| 119) | ||
| // 110 is unused |
| @@ -385,20 +385,6 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration( | |||
| if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) { | |||
| if (func->isAsyncContext()) | |||
| isAccessibleAcrossActors = true; | |||
|
|
|||
| // FIXME: move diagnosis out of this function entirely (!) | |||
|
|
||
| /// References to declarations that are part of a distributed actor are | ||
| /// only permitted if they are async. | ||
| CrossDistributedActorSelf, // FIXME(distributed): remove this case entirely rdar://83713366 |
There was a problem hiding this comment.
Great to get rid of it too, yay
Unify the logic for checking and marking accesses as async/throws/uses-distributed-thunk to reduce the overall amount of code, clarify the isolated/local/potentially-remote cases, and consistently set these options. There are tiny semantic improvements here where, e.g., one can asynchronously use non-distributed functions, properties, and subscripts if we're in a context where we know the actor is not remote.
|
@swift-ci please smoke test |
|
@DougGregor @ktoso : can you merge my PR if possible #39536 Sorry for posting this here I have tried reaching all but dint found and solution |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clean up some aspects of checking distributed actor isolation rules:
@_distributedActorIndependentlikenonisolated, simplifying away many special cases