Skip to content

[Distributed] Strip marker protocols from distributed thunks and accessible functions #41871

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 18, 2022
Merged

[Distributed] Strip marker protocols from distributed thunks and accessible functions #41871

merged 5 commits into from
Mar 18, 2022

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Mar 17, 2022

Since marker protocols do not exist at runtime, let's strip them from mangling of distributed thunks
and generic environments associated with accessible functions.

Resolves: rdar://90293494

xedin added 5 commits March 17, 2022 14:00
@xedin
[ASTMangler] Add a special entry point for mangling of distributed th…
7e182cf 
…unks

Distributed thunks loose all of the marker protocols associated with their
generic parameters.
@xedin
[Distributed] Synthesis: Start using special mangling for distributed…
c9a0295 
… thunks
@xedin
[AST] GenericSignature: Add a way to drop marker protocol requirements
2b6379d
@xedin
[Distributed] IRGen: Drop marker protocol requirements from accessibl…
1330e34 
…e function generic environments

Since marker protocol do not exist at runtime, it's harmful to keep them
in a generic environment because they cannot be checked.

Resolves: rdar://90293494
@xedin
[Distributed] NFC: Add accessor section test-case to make sure that m…
ab03005 
…arker protocols are not mangled
@xedin xedin requested review from ktoso and DougGregor March 17, 2022 22:41
@xedin xedin mentioned this pull request Mar 17, 2022
Closed
ktoso
ktoso approved these changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Nice,:l thank you!

lib/AST/GenericSignature.cpp
return !protocol->isMarkerProtocol();
}
return true;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I see, nice

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Mar 18, 2022
@ktoso
Copy link
Contributor

ktoso commented Mar 18, 2022

@swift-ci please smoke test

@xedin
Copy link
Contributor Author

xedin commented Mar 18, 2022

@swift-ci please test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Mar 18, 2022

Let's run a full suite just in case.

@ktoso
Copy link
Contributor

ktoso commented Mar 18, 2022

Yeah, sounds good 👍

@ktoso
Copy link
Contributor

ktoso commented Mar 18, 2022

CI node died, starting again

uest: Termination requested at Fri Mar 18 05:34:12 UTC 2022 by Thread[Ping thread for channel hudson.remoting.Channel@1909955:macos-node-i-0c48462df7e9f71e1,5,main] [id=2687760]
		at hudson.model.Computer.recordTermination(Computer.java:232)
		at hudson.model.Computer.disconnect(Computer.java:500)

@swift-ci please test macOS platform

@ktoso ktoso merged commit 14fb9a0 into swiftlang:main Mar 18, 2022
@ktoso
Copy link
Contributor

ktoso commented Mar 18, 2022

and full test runs all good 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktoso ktoso ktoso approved these changes

@DougGregor DougGregor Awaiting requested review from DougGregor

Assignees
No one assigned
Labels
distributed Feature → concurrency: distributed actor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xedin @ktoso