-
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
Add a stack-pin!
-ning macro to core::pin
.
#93176
Add a stack-pin!
-ning macro to core::pin
.
#93176
Conversation
@rustbot modify labels: +T-libs-api +A-async-await +F-decl_macro +A-pin |
This comment has been minimized.
This comment has been minimized.
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.
👍, but I think this deserves a beginner-friendly explanation on top of why this is needed/useful, e.g: https://docs.rs/tokio/latest/tokio/macro.pin.html. Also, this needs a tracking issue.
This comment has been minimized.
This comment has been minimized.
013f7c4
to
0d35532
Compare
cc @rust-lang/wg-async-foundations |
0d35532
to
1ebf3d6
Compare
What led to the decision to call this Personally, I think that since there are known use-cases for |
(That said, it also requires move-access to a value in order to use this macro even in the |
@cramertj Right now I just followed the more pervasive name out there, but I have no opinions on this matter whatsoever. FWIW, now that the macro is so ergonomic, there is an argument to be made for So with all that being said, I personally lean ever so slightly towards |
This comment has been minimized.
This comment has been minimized.
1ebf3d6
to
b8d598f
Compare
cc @nikomatsakis This could maybe use a more direct reference to the |
This comment has been minimized.
This comment has been minimized.
@eddyb agreed, I searched among the RFCs and surprisingly there was no normative stuff. But that reference link you provided is great! I'll add a permalink to it to the comments tomorrow |
Hmm, isn't it super-hard for these rules to change? Both reducing and enlarging the scope of temporaries can break code out there, both at compile-time (if borrows are involved) or at runtime (if lock guards are involved). |
This looks awesome! I suppose I'd have a slight preference towards |
I would prefer
|
I know that people sometimes claim that |
This comment has been minimized.
This comment has been minimized.
f9e0a07
to
98e7378
Compare
Co-Authored-By: Mara Bos <[email protected]>
Since `decl_macro`s and/or `Span::def_site()` is deemed quite unstable, no public-facing macro that relies on it can hope to be, itself, stabilized. We circumvent the issue by no longer relying on field privacy for safety and, instead, relying on an unstable feature-gate to act as the gate keeper for non users of the macro (thanks to `allow_internal_unstable`). This is technically not correct (since a `nightly` user could technically enable the feature and cause unsoundness with it); or, in other words, this makes the feature-gate used to gate the access to the field be (technically unsound, and in practice) `unsafe`. Hence it having `unsafe` in its name. Back to the macro, we go back to `macro_rules!` / `mixed_site()`-span rules thanks to declaring the `decl_macro` as `semitransparent`, which is a hack to basically have `pub macro_rules!` Co-Authored-By: Mara Bos <[email protected]>
This thus still makes it technically possible to enable the feature, and thus to trigger UB without `unsafe`, but this is fine since incomplete features are known to be potentially unsound (labelled "may not be safe"). This follows from the discussion at rust-lang#93176 (comment)
98e7378
to
c93968a
Compare
@bors r+ |
📌 Commit bf2a9dc has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (6421a49): comparison url. Summary: This benchmark run did not return any relevant results. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
`core::pin::pin!` was unstably added by [rust-lang/rust#93176][1]. This replaces `pin_utils::pin_mut!`. [1]: rust-lang/rust#93176
…, r=Amanieu Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.
…, r=Amanieu,dtolnay Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.
…, r=Amanieu,dtolnay Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.
Rollup merge of rust-lang#119562 - LegionMammal978:rename-pin-pointer, r=Amanieu,dtolnay Rename `pointer` field on `Pin` A few days ago, I was helping another user create a self-referential type using `PhantomPinned`. However, I noticed an odd behavior when I tried to access one of the type's fields via `Pin`'s `Deref` impl: ```rust use std::{marker::PhantomPinned, ptr}; struct Pinned { data: i32, pointer: *const i32, _pin: PhantomPinned, } fn main() { let mut b = Box::pin(Pinned { data: 42, pointer: ptr::null(), _pin: PhantomPinned, }); { let pinned = unsafe { b.as_mut().get_unchecked_mut() }; pinned.pointer = &pinned.data; } println!("{}", unsafe { *b.pointer }); } ``` ```rust error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> <source>:19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^ error[E0277]: `Pinned` doesn't implement `std::fmt::Display` --> <source>:19:20 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^^^^^^^^^^^^^ `Pinned` cannot be formatted with the default formatter | = help: the trait `std::fmt::Display` is not implemented for `Pinned` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) ``` Since the user named their field `pointer`, it conflicts with the `pointer` field on `Pin`, which is public but unstable since Rust 1.60.0 with rust-lang#93176. On versions from 1.33.0 to 1.59.0, where the field on `Pin` is private, this program compiles and prints `42` as expected. To avoid this confusing behavior, this PR renames `pointer` to `__pointer`, so that it's less likely to conflict with a `pointer` field on the underlying type, as accessed through the `Deref` impl. This is technically a breaking change for anyone who names their field `__pointer` on the inner type; if this is undesirable, it could be renamed to something more longwinded. It's also a nightly breaking change for any external users of `unsafe_pin_internals`.
core::pin::pin!
#93178pin!
allows pinning a value to the stack. Thanks to being implemented in the stdlib, which gives access tomacro
macros, and to the private.pointer
field of thePin
wrapper, it was recently discovered (archive link), contrary to popular belief, that it is actually possible to implement and feature such a macro:or, directly:
For context, historically, this used to require one of the two following syntaxes:
This macro thus allows, for instance, doing things like:
And so on, and so forth.
I don't think such an API can get better than that, barring full featured language support (
&pin
references or something), so I see no reason not to start experimenting with featuring this in the stdlib already 🙂r? @joshtriplett
Docs preview
video1167605346.mp4
Implementation
The implementation ends up being dead simple (so much it's embarrassing):
and voilà!
Comments and context
This is
Pin::new_unchecked(&mut { $value })
, so, for starters, let'sreview such a hypothetical macro (that any user-code could define):
Safety:
type P = &mut _
. There are thus no pathologicalDeref{,Mut}
impls that would breakPin
's invariants.{ $value }
is braced, making it a block expression, thus moving the given$value
, and making it become an anonymous temporary.By virtue of being anonynomous, it can no longer be accessed, thus preventing any attemps to
mem::replace
it ormem::forget
it, etc.This gives us a
pin!
definition that is sound, and which works, but only in certain scenarios:If the
pin!(value)
expression is directly fed to a function call:let poll = pin!(fut).poll(cx);
If the
pin!(value)
expression is part of a scrutinee:Alas, it doesn't work for the more straight-forward use-case:
let
bindings.This makes such a macro incredibly unergonomic in practice, and the reason most macros out there had to take the path of being a statement/binding macro (e.g.,
pin!(future);
) instead of featuring the more intuitive ergonomics of an expression macro.Luckily, there is a way to avoid the problem. Indeed, the problem stems from the fact that a temporary is dropped at the end of its enclosing statement when it is part of the parameters given to function call, which has precisely been the case with our
Pin::new_unchecked()
!For instance,
becomes:
However, when using a literal braced struct to construct the value, references to temporaries can then be taken. This makes Rust change the lifespan of such temporaries so that they are, instead, dropped at the end of the enscoping block.
For instance,
becomes:
which is exactly what we want.
Finally, we don't hit problems w.r.t. the privacy of the
pointer
field, or the unqualifiedPin
name, thanks todecl_macro
s being fully hygienic (def_site
hygiene).TODO
Pin
invariants thanks to the macro (e.g., try to access the private.pointer
field, or see what happens if such a pin is used outside its enscoping scope (borrow error));pin!
to the prelude: this may require to be handled with some extra care, as it may lead to issues reminiscent of those ofassert_matches!
: Is there a gentler way to land the assert_matches macro? #82913