-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[WIP] async
and .await
chapter
#3909
base: main
Are you sure you want to change the base?
Conversation
We will primarily just be re-exporting this, though we may also have a couple cases where we choose to implement something small around it.
1. Introduce an integration tests crate. Structure it the way people *should* for large crates (although this is not that) and document why, including linking to a relevant post. 2. Add a basic integration test that verifies the re-export works as it should. (This is not exactly rocket science, but we want to make sure these things don’t just stop working on accident.) An open question that remains here: do we want some structure to the crate beyond the top level re-exports? My inclination at this moment is: no, because we don’t have any *motivation* for that, and naming things is difficult. (We cannot do `trpl::async`, for example, because `async` is a keyword!)
- 17.00: Introduction to the conceptual machinery. This is very nascent but has some decent bones. - 17.01: Trying to get at the foundations for tasks, laziness, etc.; this is *especially* incomplete. - 17.02: Just a paragraph I do not want to lose, which I think *will* be useful… eventually.
Add a fair bit more material about the `futures` executor and why we might prefer to use something else. With that motivation in place, have the readers add our `trpl` crate. (We can of course rename that crate, but it does the job for now.) Use it to get the equivalent of `#[tokio::main]` equivalent and incorporate it into an example.
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.
Some context on these comments: I know this is an early draft, but I have pointed out some things that appear small but I think are important for us to figure out while working, like figuring out what terminology we should standardize on and where we should define which terms. Please feel free to tell me "yeah yeah I agree but don't want to think about that right now" :)
1,000% on board with this! Also very on board with working on this separate branch. I will likely keep the other around for reference (and we may even be able to merge it when all is said and done, who knows), but it will be a lot easier to deal with the mechanical bits separately from the prose bits, and we really don’t want to land any of that till we are actually ready to land the chapter. 😅 I have replied to most of the comments here, and I have read all of them; if I haven’t yet replied to one it is just because I am still mulling on it! |
Move up the installation of the futures crate: get the initial example compiling as soon as possible (even though it is just "Hello, world!") and *then* explain what was going on.
- Update the instructions to install `trpl` instead of `futures`, and move the introductory text there. - Update the note about Tokio in 17.02 to describe both `futures` and `tokio`.
f3fc42f
to
87a015f
Compare
mdBook does not currently have particularly good support for “external” crates. To make the test suite work correctly with `trpl`, we must first build `trpl` itself (`mdbook` will not do it), and then explicitly pass its `deps` path as a library search path for `mdbook test`. That will make sure all the crates can be resolved when running the tests. .github/workflows/main.yml
- Update the contents of the code in the chapter so it is correct, and update the text to match. Given this is about `Future`, this may also warrant moving/simplifying that whole chunk of code, too. - Update the listing to use `extern crate` since it does not otherwise work correctly with `mdbook test`. Alas.
- Update all the listings in the chapter to use `extern crate` since `mdbook test` does not understand Rust 2018. Alas. - Ignore two of the listings because they never stop.
- Update all the listings in the chapter to use `extern crate` since `mdbook test` does not understand Rust 2018. Alas. - Fix one listing which had gotten out of sync somewhere along the way. (This also fixes a comment flagged up by a reviewer!)
- Update all the listings in the chapter to use `extern crate` since `mdbook test` does not understand Rust 2018. Alas. - Ignore a listing which has a missing body apurpose.
- Rewrite the `StreamExt` definition to be more correct, and extract it to a standalone “no-listing listing” so we can make sure its definition at least type checks. - Update all the listings in the chapter to use `extern crate` since `mdbook test` does not understand Rust 2018. Alas. - Ignore the listings (inline or otherwise!) which are *intended* not to compile, so mdbook does not try to test them. - Clarify some of the text around the *actual* Tokio definition of the `StreamExt::next` method.
Note: this does *not* include all fixes for the text, only for the links themselves. For the text, we will also need to search for references to chapters 17-20. This catches a few of those along the way, but there are no doubt others.
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.
Submitting notes through the end of 17-01
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.
Almost to the end of 17-03
|
||
</Listing> | ||
|
||
This is definitely a nice improvement over needing to swap between `join` and |
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'm not sure we need to show join
and join3
-- the reader has been using println!
and (should) know that macros are for taking a variable number of arguments. Any particular reason not to start out using the macro? Then at this point, talk about the common pattern?
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.
Huh! I don’t think I had ever actually framed it as “macros are for taking a variable number of arguments” before reading this comment from you! Interesting. I am definitely open to revisiting the order. I think I defaulted away from using join!
because of how bad a taste tokio::select!
left in my mouth, but futures::join!
is much, much less magical, so I don’t have any objection to doing that. Will leave this one for a further pass, though, after getting through all the other stuff, since it will require some rejiggering of listings etc.
`Pin` builds on that to give us the exact guarantee we need. When we *pin* a | ||
value by wrapping a pointer to it in `Pin`, it can no longer move. Thus, if you | ||
have `Pin<Box<SomeType>>`, you actually pin the `SomeType` value, *not* the | ||
`Box` pointer. In fact, the pinned box pointer can move around freely. Remember: | ||
we care about making sure the data ultimately being referenced stays in its | ||
place. If a pointer moves around, but the data it points to is in the same | ||
place, there is no problem. |
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.
This could use a diagram, along the lines of some of the ones in chapter 4
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.
Ah, very good call; I will make a diagram for this! (Note also that this whole section is now part of a new 17.05, which pulls together all the walk-throughs of the relevant traits.)
Co-authored-by: Tim McNamara <paperless@timmcnamara.co.nz> Co-authored-by: James Munns <james@onevariable.com>
Co-authored-by: Carol (Nichols || Goulding) <carol.nichols@gmail.com> Co-authored-by: Will Crichton <crichton.will@gmail.com> Co-authored-by: Tim McNamara <paperless@timmcnamara.co.nz>
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.
Partway through 17-05
So far so good: if we get anything wrong about the ownership or references in a | ||
given async block, the borrow checker will tell us. When we want to move around | ||
the future that corresponds to that block—like moving it into a `Vec` to pass to | ||
`join_all`—things get trickier. |
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 around this spot it would be awesome if we had an example of, say, trying to use a reference in an async block, show the error message, and show how to fix it-- I get that sort of error message alllll the time when i'm switching between sync and async code because it looks like i'm just within a block or something and it should be totally fine to use a reference and then error message and then i have to remember to clone the arc or whatever.
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.
@carols10cents I am going to leave this specific comment open but without making a specific change related to it, because I think the overall suggestion here is a good one but I need to mull on how it works with, and where it belongs in, the chapter after the restructuring I did. It does not flow well with this paragraph where it landed in the new section dedicated to discussing the traits, which is (in its current incarnation, anyway) much more abstract. But I think that it probably needs exactly this kind of thing to help it not be too abstract. I am going to thus table that for the moment and leave this as a note to return to after hitting the rest of this PR’s comments and reworking the opening example.
Use the original transitional paragraph and structure, adding to it instead of rewriting it entirely. HT @timClicks (Tim McNamara <paperless@timmcnamara.co.nz>) for pointing out how my rephrasing here made it worse!
- Drop the history lesson and comparisons to other approaches. Focus on what async gives us instead. - Simplify and clarify the - Talk about “the network” instead of “network sockets” and simplify the example code to match.
- Swap the ordering to match the order in the text (concurrent is first, at least for now). - Make them go left-to-right instead of top-to-bottom for compactness in the text. - Fix rendering issues resulting from the VS Code extension doing things that normal `dot` does not, for who knows what reasons.
f6d2e30
to
ddad349
Compare
A working draft of a new chapter on async and await! If you have been invited to give comments, please do so! (If you’re not someone we have actively invited, we kindly ask you to hold off: although we are working in public, we cannot absorb too much feedback at once. Thanks!)
View the rendered version of this PR at https://rust-lang.github.io/book/ch17-00-async-await.html.
Note
This branch does not include changes to the listings for the chapters after this new chapter. As @carols10cents put it:
Additionally, we will need to make some other small changes to other chapters (and possibly some significant ones to the “final project” chapter) to account for having this new material in place. Those are all also out of scope for this PR!