- Notifications
You must be signed in to change notification settings - Fork 10.6k
[Educational Notes] Start adding educational notes for data-race safety. #79509
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
This educational note was obsoleted by SE-0362.
186e890 to a0e05f7 Compare
leberwurstsaft left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay education 🎉
Thanks for adding this!
heckj left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the set of changes I tried to make the wording a bit more direct, assuming the reading is a developer who's seeing these messages "in flight" and may or may not know about concurrency at any level of detail. So in particular, I tried to match the phrasing in the Swift 6 migration guide for terms, tried to make the discussion a bit more direct in guidance, and knocked out some of the passive verbs to make it more clear what was doing the guaranteeing or allowing when it comes to the code.
I'm hoping it's helpful, but please disregard if it isn't - or if I'm inaccurate in anything. I tried hard not to introduce any inaccuracies, but I don't know this topic to the depth y'all do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| If a type does not conform to `Sendable` the compiler will enforce that each instance of that type is only accessed by one concurrency domain at a time. The `sending 'x' risks causing data races` error indicates that your code can access a non-`Sendable` value from multiple concurrency domains at once. | |
| If a type does not conform to `Sendable` the compiler will enforce that each instance of that type is only accessed by one isolation domain at a time. The `sending 'x' risks causing data races` error indicates that your code can access a non-`Sendable` value from multiple isolation domains at once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think by my reading that "concurreny domain" and "isolation domain" are equivalent, and existing docs seem to lean into the term "isolation domain" over the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these terms are equivalent. But I also tend to agree that it may be confusing for programmers who've seen "isolation domain" before in Swift's documentation. It sounds like "concurrency domain" is a more approachable/readable term for this, so this is why it appears in these educational notes. That being said, I agree that we should understand when each of these terms are most appropriate to use, or whether they should be used interchangeably to avoid possible confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I go back and forth on which term is better. The current documentation is already fragmented - TSPL uses "concurrency domain" and the migration guide uses "isolation domain". I think we should settle which term to standardize on in swiftlang/swift-migration-guide#130 (or perhaps in a forum discussion) and update everything at once. I'm going to continue using "concurrency domain" for now, but I will update these notes and TSPL, or the migration guide depending on what we decide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| For example, if a value can be accessed from the main actor, it's invalid to send the same instance to another concurrency domain while the main actor can still access it. This mistake is common when calling an `async` function on a class from the main actor: | |
| For example, if a value can be accessed from the main actor, it's invalid to send the same instance to another isolation domain while the main actor can still access it. This mistake is common when calling an `async` function on a class from the main actor: |
simanerush left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for writing these, so cool to see progress in concurrency documentation!🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these terms are equivalent. But I also tend to agree that it may be confusing for programmers who've seen "isolation domain" before in Swift's documentation. It sounds like "concurrency domain" is a more approachable/readable term for this, so this is why it appears in these educational notes. That being said, I agree that we should understand when each of these terms are most appropriate to use, or whether they should be used interchangeably to avoid possible confusion.
1a224bf to 8e68e01 Compare 690f1c0 to 61295bd Compare 8fda891 to 28666ae Compare 28666ae to 36d1d34 Compare 36d1d34 to fdd7402 Compare | @swift-ci please smoke test |
| Thanks everyone for the feedback so far! There's more I want to do to improve these notes, but I think those changes will be easier to review as follow-up PRs. I'd also like to experiment with incorporating some more structure into the notes, e.g. with headings so they're easier to scan. |
| swiftlang/swift-installer-scripts#397 @swift-ci please test Windows |
Putting this PR up now because I'd love some feedback on the depth of explanation and the writing style of the educational note. I'm working on more, but am still trying to get a feel for how best to write these notes.
Note that the suggested fix for the region isolation error assumes the existence of
@execution(caller)attribute because it does exist right now onmain. However, the proposal that adds this feature is under review now, and this note will be updated along with any changes to the design as a result of the review discussion.I also removed
complex-closure-inference.mdbecause it's just out of date. Multi-statement closure inference was added in SE-0362.Resolves #79640