Skip to content

Don't diagnose non-Sendable captures in actor-isolated @Sendable closures #42126

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

Conversation

DougGregor
Copy link
Member

Another important step towards implementing SE-0338 semantics.
Fixes rdar://90950355.

…ures

Another important step towards implementing SE-0338 semantics.
Fixes rdar://90950355.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor DougGregor merged commit 1629d83 into swiftlang:main Apr 1, 2022
@DougGregor DougGregor deleted the se-0338-actor-isolated-sendable-closure-capture branch April 1, 2022 00:30
Task {
_ = sc // expected-warning{{capture of 'sc' with non-sendable type 'SomeClass' in a `@Sendable` closure}}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird actually... are you saying this 3rd Task does NOT execute on the actor because does not capture self? That's insanely brittle isn't it? Isn't the model we explain that if "syntactically created inside an actor" then runs on actor...? 😕 We're not doing a good job explaining the actual semantics of the implicit executor inheritance tbh :~

Copy link
Member Author

Choose a reason for hiding this comment

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

If you don't capture self, we shouldn't capture it on your behalf. At most, this could be a warning.

func testNonSendableCaptures(sc: SomeClass) {
Task {
_ = self
_ = sc
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


Task { [sc,self] in
_ = self
_ = sc
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants