[Distributed] Prepare stubs for static func resolve #38530
Conversation
ktoso
force-pushed
the
wip-instroduce-static-resolve
branch
from
093a8c3 to
08b99fa
Compare
| @@ -4436,8 +4436,8 @@ NOTE(actor_isolated_sync_func,none, | |||
| "implicitly asynchronous", | |||
| (DescriptiveDeclKind, DeclName)) | |||
| NOTE(distributed_actor_isolated_method_note,none, | |||
| "only 'distributed' functions can be called from outside the distributed actor", | |||
| ()) | |||
| "only 'distributed' functions can be called from outside the distributed actor", // TODO: improve error message | |||
There was a problem hiding this comment.
the note eventually I guess we'll just do a "declared here"
| @@ -579,9 +579,19 @@ SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) | |||
| void swift_defaultActor_deallocateResilient(HeapObject *actor); | |||
|
|
|||
| /// Initialize the runtime storage for a distributed remote actor. | |||
| // TODO: this may end up being removed as we move to the "proxy creation" below | |||
| SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) | |||
| void swift_distributedActor_remote_initialize(DefaultActor *actor); | |||
There was a problem hiding this comment.
this was still from initial attempts to make use of the default actor infra somewhat; but it seams instead we want to create the remote ref instead. So this function would go away completely.
| /* +1 */OpaqueValue *transport | ||
| // metadata for identity | ||
| // metadata for transport | ||
| ); |
There was a problem hiding this comment.
need to pass the generic types here; not sure what we'll want to return so for now i just stubbed as an opaque value
| @@ -385,6 +385,7 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration( | |||
| if (func->isAsyncContext()) | |||
| isAccessibleAcrossActors = true; | |||
|
|
|||
| // FIXME: move diagnosis out of this function entirely (!) | |||
There was a problem hiding this comment.
I'll do this cleanup shortly. but separately would be better so we have the new function in so @kavon can start poking at implementing it.
| @@ -3164,7 +3182,8 @@ ActorIsolation ActorIsolationRequest::evaluate( | |||
| } | |||
|
|
|||
| if (auto nominal = value->getDeclContext()->getSelfNominalTypeDecl()) { | |||
| if (nominal->isDistributedActor()) { | |||
| /// Unless the function is static, it is isolated to the dist actor | |||
| if (nominal->isDistributedActor() && !func->isStatic()) { | |||
There was a problem hiding this comment.
This is a bit nasty that we had to check ad-hoc here, i'll rethink our isolation in general...
| @@ -48,7 +48,7 @@ public protocol ActorTransport: Sendable { | |||
| /// | |||
| /// Detecting liveness of such remote actors shall be offered / by transport libraries | |||
| /// by other means, such as "watching an actor for termination" or similar. | |||
| func resolve<Act>(_ identity: Act.ID, as actorType: Act.Type) throws -> ActorResolved<Act> | |||
| func resolve<Act>(_ identity: AnyActorIdentity, as actorType: Act.Type) throws -> ActorResolved<Act> | |||
| where Act: DistributedActor | |||
There was a problem hiding this comment.
Act.ID always is AnyActorIdentity anyway
| /// Resolves the passed in `address` against the `transport`, | ||
| /// returning either a local or remote actor reference. | ||
| @available(*, deprecated, renamed: "SomeDistributedActor.resolve(_:using:)") | ||
| init(resolve id: AnyActorIdentity, using transport: ActorTransport) throws |
There was a problem hiding this comment.
let's completely ignore the existence of this; I'm only keeping it around so we don't have to remove useful tests that we have already.
Once the resolve function exists I'll remove all backing infra for this init.
| /// - Parameter identity: identity uniquely identifying a, potentially remote, actor in the system | ||
| /// - Parameter transport: `transport` which should be used to resolve the `identity`, and be associated with the returned actor | ||
| static func resolve<Identity>(_ identity: Identity, using transport: ActorTransport) | ||
| throws -> Self where Identity: ActorIdentity |
| return remote as! Self | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we'll actually get away with just implementing it like that...
It depends how we'd want to make it available for protocols later on, but that's "just" a typechecking change, the implementation is always the same :-)
| @@ -863,6 +886,8 @@ func testCrossActorProtocol<T: P>(t: T) async { | |||
| t.g() | |||
| // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }} | |||
| // expected-note@-2{{calls to instance method 'g()' from outside of its actor context are implicitly asynchronous}} | |||
| ASP.s() | |||
| ASPD.sd() | |||
| } | |||
There was a problem hiding this comment.
Silly, but wanted to add coverage of those since I was touching static funcs in distributed actors.
| // TODO: implement resolve being able to be called on a distributed actor protocol | ||
| // (yes, normally it is not allowed to call such function... so we need to | ||
| // discuss and figure out how we want to expose the resolve of a protocol) | ||
| // let c: Wheeker = try Wheeker.resolve(.init(identity), using: transport) |
There was a problem hiding this comment.
This is tricky future work, but leaving the TODO here as a reminder already; it's crucial we get this right, in however shape we'll surface it in the future,
ktoso
force-pushed
the
wip-instroduce-static-resolve
branch
from
08b99fa to
08dbfa6
Compare
ktoso
force-pushed
the
wip-instroduce-static-resolve
branch
from
08dbfa6 to
96fdbed
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test windows But it seems like some spurious failure |
|
@swift-ci please smoke test windows |
This just sets the stage for the
static func resolveimplementation rather than the resolve initializer.I did not remove existing code yet, but once we nail the remote creation I'll follow up with cleanups of anything not used.