Skip to content

[Distributed][Macro] Handle more cases in distributed protocol macro #72177

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
merged 7 commits into from
Mar 12, 2024

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 8, 2024

The distributed protocol macro was blocked from generating the full right thing until we fixed generic actors and protocols and the Codable actor behaviors.

A few more cases to be handled here.

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Mar 8, 2024
@ktoso ktoso marked this pull request as ready for review March 8, 2024 07:09
@ktoso ktoso force-pushed the wip-distributed-macro-fixes branch 2 times, most recently from ab06f36 to 99450d7 Compare March 8, 2024 07:25
@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2024

@swift-ci please smoke test

@ktoso ktoso force-pushed the wip-distributed-macro-fixes branch from 99450d7 to 09298ae Compare March 11, 2024 14:29
@ktoso
Copy link
Contributor Author

ktoso commented Mar 11, 2024

@swift-ci please smoke test

@ktoso ktoso enabled auto-merge (squash) March 11, 2024 14:29
@ktoso
Copy link
Contributor Author

ktoso commented Mar 11, 2024

Lots of cleanups to the macros, thanks to landing all our previous changes 👍


@_DistributedProtocol
protocol Greeter: DistributedActor where ActorSystem == FakeActorSystem {
distributed func greet(name: String) -> String
Copy link
Contributor

@xedin xedin Mar 11, 2024

Choose a reason for hiding this comment

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

I'm just going to leave an enhancement request here - I think we need to get tested at least:

  • methods with generic parameters
  • distributed variables i.e. protocol requirements like distributed var compute: Int { get set }
  • Multiple distributed members

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call on properties actually, I don't think we roundtrip tested them yet

Copy link
Contributor Author

@ktoso ktoso Mar 12, 2024

Choose a reason for hiding this comment

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

Covered all these and will add runtime tests as well in a follow up PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

@ktoso ktoso force-pushed the wip-distributed-macro-fixes branch from 09298ae to f7b27ec Compare March 12, 2024 02:14
@ktoso
Copy link
Contributor Author

ktoso commented Mar 12, 2024

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 12, 2024

@swift-ci please smoke test Windows

@ktoso ktoso merged commit 27702fa into swiftlang:main Mar 12, 2024
@ktoso ktoso deleted the wip-distributed-macro-fixes branch March 13, 2024 12:15
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