Skip to content

[Distributed] Reimplement distributed call thunks completely in AST #41616

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

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 2, 2022

Simplifies a ton of things but not complete yet.

@ktoso ktoso force-pushed the wip-allow-distributed-in-protocols-hop-to-ast branch from a52448d to a98d34f Compare March 3, 2022 22:38
@ktoso ktoso force-pushed the wip-allow-distributed-in-protocols-hop-to-ast branch from 9ab487f to d8afe3f Compare March 7, 2022 11:55
@ktoso
Copy link
Contributor Author

ktoso commented Mar 7, 2022

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 7, 2022

@swift-ci please build toolchain

@ktoso
Copy link
Contributor Author

ktoso commented Mar 7, 2022

@swift-ci please smoke test

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Mar 7, 2022
@ktoso ktoso force-pushed the wip-allow-distributed-in-protocols-hop-to-ast branch from 883b0f5 to 5c09384 Compare March 8, 2022 01:24
@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2022

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2022

@swift-ci please build toolchain

@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2022

resolves rdar://84054772

@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2022

The failures are some linux only thing, we'll need to investigate...

@ktoso ktoso force-pushed the wip-allow-distributed-in-protocols-hop-to-ast branch from ceddca8 to 3c41b0a Compare March 10, 2022 00:24
@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2022

@swift-ci please smoke test and merge

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Looks good to me! Majority of comments could be addressed separately only ones worth addressing now is missing mapTypeIntoContext and unnecessary copy of the thunk name.

AbstractFunctionDecl *ASTContext::getRemoteCallOnDistributedActorSystem(
NominalTypeDecl *actorOrSystem, bool isVoidReturn) const {
assert(actorOrSystem && "distributed actor (or system) decl must be provided");
const NominalTypeDecl *system = actorOrSystem;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should drop const here.

@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2022

@swift-ci please smoke test linux

@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2022

Addressed comments and XFAILed a few linux tests because I really want to land this PR, it's holding up a lot of the other ones.

Will fix the linux tests ASAP though, it is just mangling fixes i think. rdar://90078069

@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2022

@swift-ci please smoke test and merge

@ktoso ktoso force-pushed the wip-allow-distributed-in-protocols-hop-to-ast branch from 3e1819b to b91905f Compare March 10, 2022 07:04
@ktoso ktoso force-pushed the wip-allow-distributed-in-protocols-hop-to-ast branch from b91905f to 6440861 Compare March 10, 2022 11:16
@ktoso
Copy link
Contributor Author

ktoso commented Mar 10, 2022

@swift-ci please smoke test

@ktoso ktoso merged commit 5ab8e08 into swiftlang:main Mar 10, 2022
@ktoso ktoso deleted the wip-allow-distributed-in-protocols-hop-to-ast branch March 10, 2022 14:58
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