-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[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
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!
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.
@@ -0,0 +1,52 @@ | |||
# Sending value risks causing data races | |||
|
|||
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. |
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.
|
||
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. | ||
|
||
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: |
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: |
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!🎉
@@ -0,0 +1,52 @@ | |||
# Sending value risks causing data races | |||
|
|||
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. |
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.md
because it's just out of date. Multi-statement closure inference was added in SE-0362.Resolves #79640