Skip to content

[Distributed] Emit accessors for distributed protocol requirements #72073

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

xedin
Copy link
Contributor

@xedin xedin commented Mar 5, 2024

Given the following protocol:

protocol Greeter : DistributedActor {
  distributed func greet()
}

The changes make it possible to synthesize a distributed accessor thunk for
the requirement greet which would be dispatched to the underlying
concrete actor implementation at runtime.

xedin added 9 commits March 1, 2024 14:32
…e underlying thunks

Preparing to start emitting distributed requirement accessors
that don't have `SILFunction *` because they are associated with
protocol requirements.
…tocol requirements

Given the following protocol:

```
protocol Greeter : DistributedActor {
  distributed func greet()
}
```

The changes make it possible to synthesize a distributed accessor
thunk for the requirement `greet` which would be dispatched to the
underlying concrete actor implementation at runtime.
…ould be backed by SILFunction

Distributed protocol requirements don't have associated SILFunction,
let's introduce a more flexible way to define it that collects only
the information necessary for the function to become accessible.
… `$` mangling

This way distributed thunks could be formed in contexts that don't
know what concrete actor they'd be dispatched to on the other side
i.e. `distributed` members in protocol extensions should refer to
protocol requirement accessors.
@xedin xedin marked this pull request as ready for review March 5, 2024 00:00
@xedin
Copy link
Contributor Author

xedin commented Mar 5, 2024

@swift-ci please test

auto emission =
getCallEmission(IGF, callee.getSwiftContext(), std::move(callee));

emission->begin();
emission->setArgs(arguments, /*isOutlined=*/false,
/*witnessMetadata=*/nullptr);
Target.getWitnessMetadata(actorSelf));
Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha 👍

@xedin xedin force-pushed the distributed-protocol-requirement-accessors branch from 3525758 to c00c692 Compare March 5, 2024 01:47
@xedin
Copy link
Contributor Author

xedin commented Mar 5, 2024

@swift-ci please test Linux platform

@xedin
Copy link
Contributor Author

xedin commented Mar 5, 2024

@swift-ci please test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Mar 5, 2024

@swift-ci please test Windows platform

@ktoso
Copy link
Contributor

ktoso commented Mar 5, 2024

We may need a clean build?

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift/stdlib/private/StdlibUnittest/StdlibUnittest.swift:14:8: error: compiled module was created by an older version of the compiler; rebuild 'SwiftPrivate' and try again: /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/minimalstdlib-macosx-x86_64/lib/swift/freestanding/SwiftPrivate.swiftmodule/x86_64-apple-macos.swiftmodule
  12 │ 
  13 │ 
  14 │ import SwiftPrivate
     │        ╰─ error: compiled module was created by an older version of the compiler; rebuild 'SwiftPrivate' and try again: /Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/build/buildbot_incremental/minimalstdlib-macosx-x86_64/lib/swift/freestanding/SwiftPrivate.swiftmodule/x86_64-apple-macos.swiftmodule

@ktoso
Copy link
Contributor

ktoso commented Mar 5, 2024

@swift-ci please clean test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Mar 5, 2024

@swift-ci please smoke test

@xedin
Copy link
Contributor Author

xedin commented Mar 6, 2024

@swift-ci please smoke test Linux platform

@ktoso
Copy link
Contributor

ktoso commented Mar 6, 2024

all tests passed but there was a merge conflict, I resolved it

@swift-ci please smoke test

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Mar 6, 2024
@xedin
Copy link
Contributor Author

xedin commented Mar 6, 2024

@swift-ci please smoke test

@xedin
Copy link
Contributor Author

xedin commented Mar 6, 2024

Previous comment didn’t trigger CI for some reason.

@ktoso
Copy link
Contributor

ktoso commented Mar 6, 2024

Heh, I'll fix the compile issue, that's what I get for fixing a "trivial" merge conflict on github UI.

I see, @kavon in the meantime changed syntax of appendContext it seems so this needed to be updated to the new syntax now. Fixed here: d76e560

@ktoso
Copy link
Contributor

ktoso commented Mar 6, 2024

@swift-ci please smoke test

@ktoso ktoso enabled auto-merge (squash) March 6, 2024 10:59
@ktoso ktoso disabled auto-merge March 6, 2024 10:59
@xedin xedin merged commit afeef85 into swiftlang:main Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants