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

Concurrency fixes #2118

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Concurrency fixes #2118

wants to merge 16 commits into from

Conversation

mgeisler
Copy link
Collaborator

@mgeisler mgeisler commented Jun 5, 2024

Hi folks! I'm teaching the Concurrency class, so I went through it and made a bunch of changes.

I debated trying to split this up into smaller PRs — some of the commits here depend on the previous commits, so it's a bit annoying to split up.

If you're okay with looking at this via the individual commits, then it's probably easier to merge it like that. Otherwise I can try splitting things up after teaching my class tomorrow.

Copy link
Collaborator

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

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

A few comments inline, but looks good otherwise.

Comment on lines -23 to -26
- The type system gives us safety for concurrency without any special features.
- The same tools that help with "concurrent" access in a single thread (e.g., a
called function that might mutate an argument or save references to it to read
later) save us from multi-threading issues.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this text is relatively important and shouldn't be dropped. It's not just that the type system is what Rust uses to enable fearless concurrency, but it's that the same type system that students already understand does so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, thanks! I don't think I wanted to remove this part 😄 I wanted move it to somewhere else... I'll have another look!

Comment on lines 34 to 36
- What is the return type of an async call?
- Use `let future: () = async_main(10);` in `main` to see the type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's still worthwhile to take the step of demonstrating that the return type of an async fn is not the written return type, but actually a future type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that the answer is now less detailed (and less confusing), but I agree with @fw-immunant) that this should stay:

  • it illustrates that this is not a magical exception from the type rules
  • in general, it's a good technique for finding the type of a value, and bears a bit of repeating

Async methods in traits are were stabilized only recently, in the 1.75 release.
This required support for using return-position `impl Trait` (RPIT) in traits,
as the desugaring for `async fn` includes `-> impl Future<Output = ...>`.
Async methods in traits are were stabilized in the 1.75 release. This required
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should fix "are were" while we're at it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume you mean

Suggested change
Async methods in traits are were stabilized in the 1.75 release. This required
Async methods in traits are, were, and always will be stabilized in the 1.75 release. This required

;)

Copy link
Collaborator

@djmitche djmitche 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!

Comment on lines 34 to 36
- What is the return type of an async call?
- Use `let future: () = async_main(10);` in `main` to see the type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that the answer is now less detailed (and less confusing), but I agree with @fw-immunant) that this should stay:

  • it illustrates that this is not a magical exception from the type rules
  • in general, it's a good technique for finding the type of a value, and bears a bit of repeating

Async methods in traits are were stabilized only recently, in the 1.75 release.
This required support for using return-position `impl Trait` (RPIT) in traits,
as the desugaring for `async fn` includes `-> impl Future<Output = ...>`.
Async methods in traits are were stabilized in the 1.75 release. This required
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume you mean

Suggested change
Async methods in traits are were stabilized in the 1.75 release. This required
Async methods in traits are, were, and always will be stabilized in the 1.75 release. This required

;)

@@ -23,6 +23,9 @@ members = [
"src/std-traits",
"src/std-types",
"src/testing",
"src/concurrency/async",
"src/concurrency/async-control-flow",
"src/concurrency/async-pitfalls",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this fixes #567. Thanks!!

We now avoid the `<T>` in the page title. We refer to `Rc` and `Arc`
as reference-counted shared pointers that allow access to data from
multiple places or threads.
- Avoid time-specific statements (“recently”, “today”, …).

- Move details such as “RPIT” to the speaker notes.

- More links to our slides as well as community documentation.

- Formatting fixes
This moves all non-trivial examples in the async part of the course to
self-contained Rust files. This ensures that we can test them in CI.
The 1-day classes are sometimes taught to people who haven’t taken
Rust Fundamentals, or who have taken it a while ago. So it seems nice
to remind everybody that questions are very welcome.
Bullet points makes it looks more like, well, a slide!
This keeps the original structure, but removes 7 lines of code.
This removes a nested import (which I find hard to read) and removes
an unnecessary type annotation.
This reduces the vertical space needed by the example, thus making it
easier to keep on the screen while refactoring it.
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.

3 participants