Skip to content
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

Expand the concurrency chapter #159

Merged
merged 31 commits into from
Dec 4, 2023

Conversation

amartini51
Copy link
Collaborator

  • Integrate the discussion of task cancellation into the running example.
  • Add more detailed explanation to the suspension points example
  • Compare & contrast throwing and async code.

Fixes: rdar://75895184

Copy link
Member

@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.

Some minor comments and ideas, hope this helps!

TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
amartini51 and others added 5 commits September 5, 2023 20:38
Co-authored-by: Evan Wilde <ewilde@apple.com>
Co-authored-by: Konrad Malawski <konrad_malawski@apple.com>
Co-authored-by: Konrad Malawski <konrad_malawski@apple.com>
Incorporates feedback from Evan Wilde <ewilde@apple.com>
and Konrad Malawski <konrad_malawski@apple.com>.
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
amartini51 and others added 2 commits October 16, 2023 14:48
Co-authored-by: Evan Wilde <ewilde@apple.com>
Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

Looks good to me

The framing of "no suspension points means no other code runs" is at
best misleading outside of an actor, because there can be other code
running concurrently.  Inside of an actor, this kind of no-await code is
a useful best practice that we want to show.
The previous paragraph talks about a single function that is both async
and throwing -- but this paragraph describes some similarities between
the two different kinds of functions, not about functions that are both.
Parallel and async code, by itself, doesn't give you memory safety.
That's one of the reasons to use Swift concurrency -- to get memory
safety in your code.

Expand the "you can't do this" to give some follow-on advice about what
you can do, in terms of adoption.

Avoid the wording "even stronger" when there isn't an obvious guarantee
being compared to.
Copy link
Contributor

@chuckdude chuckdude left a comment

Choose a reason for hiding this comment

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

Please see edits and comments throughout. Please let me know if you have any questions or would like to discuss anything.

TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
TSPL.docc/LanguageGuide/Concurrency.md Outdated Show resolved Hide resolved
amartini51 and others added 2 commits November 14, 2023 15:30
Edits from <rdar://117556772>

Co-authored-by: Chuck Toporek <chuck_toporek@apple.com>
@ktoso
Copy link
Member

ktoso commented Nov 29, 2023

This has been looking quite good for a while, do we intend to merge soon @amartini51 or keep adding @amartini51 ?

@amartini51
Copy link
Collaborator Author

@ktoso I've got a few minor changes to make from Chuck's editorial review (likely this afternoon or tomorrow morning) then this will be ready to merge.

Copy link
Contributor

@chuckdude chuckdude left a comment

Choose a reason for hiding this comment

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

Approved.

@ktoso
Copy link
Member

ktoso commented Dec 2, 2023

Looks good I think! Thanks Alex et al

@amartini51 amartini51 merged commit 3fc858b into apple:main Dec 4, 2023
@amartini51 amartini51 deleted the concurrency_75895184 branch December 4, 2023 19:20
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.

None yet

5 participants