Better explanation why futures need to be pinned #1687
Conversation
src/async/pitfalls/pin.md
Outdated
| trait. When the async code awaits another future, the `Future` trait holds the | ||
| local variables that would normally be located on the stack. |
There was a problem hiding this comment.
I don't think this should be linked to await specifically. My understanding is that the async function body is compiled to a struct which impl Future. What was local variables in the function body become struct fields.
So what you see as a local variable in the code is never actually a variable in the code after the compiler transformation. Thus, I would not say that this happens when you call await since it happened already in the immediate desugaring.
An example of this desugaring can be seen in Async fn as a Future. The State0, State1, and Terminated enum variants correspond to different parts of the now-gone async function body. The variants hold the local variables that are alive in the different parts of the body.
|
Hi @proski, I really like the new language here, I think it's a nice improvement! One thing to keep in mind is that the pages are meant to be slides, so they should be brief. Longer explanations are best put in the notes at the bottom of the page (so they can be collapsed during normal teaching). So I would suggest having one 3-line paragraph as the intro text and then expand on that in the notes. |
proski
force-pushed
the
pin-future
branch
2 times, most recently
from
183df78 to
bae5185
Compare
|
I reduced the top text to 3 paragraphs and moved some noted to the end. Thank you for the link to the Tokio documentation, it demystified so many things for me! I'm not sure why the "Course Schedule Updates" check is failing. I'll keep retrying until the CI check is fixed. |
Cool, I think it looks very nice now! Mentioning "self-referential" is great since it can give people a jumping-off point for further investigation. As @djmitche mentioned in #1678, we cannot expect to teach complex topics like this in detail on slides, but we can at least give people the relevant words to search for later. Could you include a link to https://doc.rust-lang.org/std/pin/ somewhere (if we don't have it)? That should help people who want more background information.
Yeah, me too 😄
Sorry, it's broken on our end... I noticed this yesterday in #1576. We'll probably have to remove the check, but please just ignore it for now. |
The GitHub action fails because it cannot run with the necessary
permissions: it requires write access to be able to post a comment,
but when triggered from a fork, the token is limited to read access.
The error is from the thollander/actions-comment-pull-request action:
No comment has been found with asked pattern. Creating a new comment.
Error: Resource not accessible by integration
The downgrading of the token is explained on:
https://docs.github.com/en/actions/security-guides/automatic-token-authentication
where “Maximum access for pull requests from public forked
repositories” says that the pull-requests permission is capped at read
in the typical case for us.
I kept the underlying functionality since we might want to use it for
something else (perhaps track the duration in the README).
Fixes the errors seen on the recent PRs: #1693, #1687, and likely
others.
The GitHub action fails because it cannot run with the necessary
permissions: it requires write access to be able to post a comment,
but when triggered from a fork, the token is limited to read access.
The error is from the thollander/actions-comment-pull-request action:
No comment has been found with asked pattern. Creating a new comment.
Error: Resource not accessible by integration
The downgrading of the token is explained on:
https://docs.github.com/en/actions/security-guides/automatic-token-authentication
where “Maximum access for pull requests from public forked
repositories” says that the pull-requests permission is capped at read
in the typical case for us.
I kept the underlying functionality since we might want to use it for
something else (perhaps track the duration in the README).
Fixes the errors seen on the recent PRs: #1693, #1687, and likely
others.
The GitHub action added in #1576 fails because it cannot run with the necessary permissions: it requires write access to be able to post a comment, but when triggered from a fork, the token is limited to read access. The error is from the thollander/actions-comment-pull-request action: No comment has been found with asked pattern. Creating a new comment. Error: Resource not accessible by integration The downgrading of the token is explained on: https://docs.github.com/en/actions/security-guides/automatic-token-authentication where “Maximum access for pull requests from public forked repositories” says that the pull-requests permission is capped at read in the typical case for us. I kept the underlying functionality since we might want to use it for something else (perhaps track the duration in the README). Fixes the errors seen on the recent PRs: #1693, #1687, and likely others.
Co-authored-by: Martin Geisler <[email protected]>
|
Thanks for this, let's merge this now and iterate more on it later (if people feel it's needed). @fw-immunant, you'll be teaching the Concurrency class soon, so you might have opnions on this too. |
Attempt to address #1677.
Expert review is needed. The new text is my best guess based on the original text and other explanations I could find online.
A few things to note:
Pinis a protective wrapper around a pointer, not a mechanism that changes the pointer or the pointed object.Box::pin()can be replaced.