-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 std::pin module docs and rename std::pin::Pinned to PhantomPinned #55992
Conversation
//! | ||
//! Note that pinning and `Unpin` only affect the pointed-to type. For example, whether | ||
//! or not `Box<T>` is `Unpin` has no affect on the behavior of `Pin<Box<T>>`. Similarly, | ||
//! `Pin<Box<T>>` and `Pin<&mut T>` are always `Unpin` themselves, even though the |
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.
Shouldn't this be Box<T>
and &mut T
? We have an impl Unpin for Box<T> where T: !Unpin
, right?
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.
Those are also Unpin
, but I was specifically referring to the Pin
-wrapped types to clarify that Pin
does not make something !Unpin
.
//! changing the location of the underlying data, [`Pin`] prohibits accessing the | ||
//! underlying pointer type (the `&mut` or `Box`) directly, and provides its own set of | ||
//! APIs for accessing and using the value. [`Pin`] also guarantees that no other | ||
//! functions will move the pointed-to value. This allows for the creation of |
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 we may want to say "no later code", or something along those lines, instead of "other functions". The contract around Pin
talks about what cannot happen "in the future", but the current text doesn't currently say that.
Ping from triage @withoutboats: This PR requires your review. |
Ping from triage @withoutboats / @rust-lang/libs: This PR requires your review. |
☔ The latest upstream changes (presumably #56578) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping @rust-lang/libs can one of y'all make a decision here? I don't think there's much to be said that hasn't been already on the stabilization thread. |
@rfcbot fcp merge To confirm we're all on board with the renaming, but I suspect this won't take long |
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Ok great! @cramertj want to rebase this and I'll r+? |
@alexcrichton done! |
@bors: r+ |
📌 Commit 709b751 has been approved by |
Expand std::pin module docs and rename std::pin::Pinned to PhantomPinned cc #49150, #55766 r? @withoutboats
☀️ Test successful - status-appveyor, status-travis |
cc #49150, #55766
r? @withoutboats