Data Racy Safety Editing #109
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattmassicotte
requested review from
hborla,
shahmishal and
ktoso
as code owners
mattmassicotte
commented
Jun 25, 2024
| @@ -45,7 +45,7 @@ you are doing so statically. Isolation can be a part of these static | |||
| declarations. | |||
|
|
|||
| There are cases, however, where the type system alone cannot sufficiently | |||
| describe a system's behavior. An example could be an Objective-C type | |||
| describe runtime behavior. An example could be an Objective-C type | |||
There was a problem hiding this comment.
awkward duplication of the word "system" here with different meaings.
mattmassicotte
commented
Jun 25, 2024
| @@ -130,8 +130,7 @@ access from any other domain. | |||
|
|
|||
| Actors give the programmer a way to define an isolation domain, | |||
| along with methods that operate within that domain. | |||
| All stored instance properties of an actor are isolated to the enclosing | |||
| actor instance. | |||
| All stored properties of an actor are isolated to the enclosing actor instance. | |||
There was a problem hiding this comment.
Change wording to "stored properties" to match TSPL and also avoid duplicating the word "instance" within the sentence.
mattmassicotte
commented
Jun 25, 2024
| @@ -226,7 +225,7 @@ but that doesn't mean you must always manually start one. | |||
| Typically, asynchronous functions do not need to be aware of the | |||
| task running them. | |||
| In fact, tasks can often begin at a much higher level, | |||
| within an application framework, or even at the root of a program. | |||
| within an application framework, or even at the entry point of the program. | |||
There was a problem hiding this comment.
what is "the root of a program"?
There was a problem hiding this comment.
hehe yeah... well, not where the leafs are! 🥬
Good rewording
mattmassicotte
commented
Jun 25, 2024
| @@ -243,7 +242,7 @@ actor instance, a global actor, or could be non-isolated. | |||
| This isolation can be established manually, but can also be inherited | |||
| automatically based on context. | |||
| Task isolation, just like all other Swift code, determines what mutable state | |||
| they can access. | |||
| is accessible. | |||
There was a problem hiding this comment.
"they" is quite unclear here.
mattmassicotte
commented
Jun 25, 2024
| @@ -388,7 +387,7 @@ The Swift Programming Language. | |||
| Isolation domains protect their mutable state, but useful programs need more | |||
| than just protection. They have to communicate and coordinate, | |||
| often by passing data back and forth. | |||
| Moving values into or out of an isolation domain is known as crossing an | |||
| Moving values into or out of an isolation domain is known as _crossing_ an | |||
There was a problem hiding this comment.
New concepts are emphasized throughout this document.
mattmassicotte
commented
Jun 25, 2024
| When you see a conformance to `Sendable` in documentation, | ||
| it means the given type is thread safe, | ||
| This is represented by the `Sendable` protocol. | ||
| A conformance to `Sendable` means the given type is thread safe, |
There was a problem hiding this comment.
Lots of redundancy within this sentences. Plus, referencing "documentation" here is just strange.
mattmassicotte
commented
Jun 25, 2024
| @@ -423,7 +420,7 @@ types in Swift are implicitly `Sendable` when all their stored properties | |||
| are also Sendable. | |||
| However, this implicit conformance is not visible outside of their | |||
| defining module. | |||
| Making a class `Sendable` is part of its public API contract, | |||
| Making a type `Sendable` is part of its public API contract | |||
There was a problem hiding this comment.
This whole section is about value types!
mattmassicotte
commented
Jun 25, 2024
| @@ -532,16 +529,13 @@ class ChickenValley { | |||
| } | |||
| ``` | |||
|
|
|||
| Being `Sendable`, actor and global-actor-isolated types are always safe | |||
| to pass across isolation boundaries. | |||
There was a problem hiding this comment.
This is just re-stating information that has appeared immediately before.
mattmassicotte
commented
Jun 25, 2024
| To make a class `Sendable`, it must contain no mutable state. | ||
| And any immutable properties must also be `Sendable`. | ||
| To make a class `Sendable` it must contain no mutable state and all | ||
| immutable properties must also be `Sendable`. |
There was a problem hiding this comment.
Awkard wording and structure here.
mattmassicotte
commented
Jun 25, 2024
| @@ -31,7 +31,7 @@ Or, you may have been waiting for the Swift 6 release to begin using them. | |||
| Regardless of where your project is in this process, this guide provides | |||
| concepts and practical help to ease the migration. | |||
|
|
|||
| Here you will find articles and code examples that will: | |||
| You will find articles and code examples here that: | |||
There was a problem hiding this comment.
Really don't like the sound of the duplicated "here"
mattmassicotte
commented
Jun 25, 2024
| @@ -43,8 +43,7 @@ Here you will find articles and code examples that will: | |||
| > Important: The Swift 6 language mode is _opt-in_. | |||
| Existing projects will not switch to this mode without configuration changes. | |||
| > | |||
| > There is an important distinction between the _compiler version_ | |||
| and _language mode_. | |||
| > There is a distinction between the _compiler version_ and _language mode_. | |||
There was a problem hiding this comment.
This is within a note marked "Important". I don't think restating that this is also important is adding anything.
Merged
ktoso
approved these changes
Jul 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I did a pass over the Data Racy Safety section. It's been a while now and I found many small issues.
I know there's the potential for more changes coming via #50, but this still seems worthwhile in the meantime.