Rename pointer field on Pin by LegionMammal978 · Pull Request #119562 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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:
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 }); }
error[E0658]: use of unstable library feature 'unsafe_pin_internals' --> :19:30 | 19 | println!("{}", unsafe { *b.pointer }); | ^^^^^^^^^
error[E0277]: Pinned doesn't implement std::fmt::Display
--> :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 #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.
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the library team, which will review and decide on the PR/issue.
labels
Relevant to the library API team, which will review and decide on the PR/issue.
and removed T-libs
Relevant to the library team, which will review and decide on the PR/issue.
labels
This comment has been minimized.
This comment has been minimized.
joboet added a commit to joboet/rust that referenced this pull request
joboet added a commit to joboet/rust that referenced this pull request
joboet added a commit to joboet/rust that referenced this pull request
📌 Commit 985402d has been approved by Amanieu
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…, 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:
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 });
}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.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
I've updated the expression in src/etc/natvis/libcore.natvis to account for the change in field name. I believe it should work, but I'm not able to test it with my current setup. Anyway, I've been thinking it might be better to hold off on merging this PR until #119615 is resolved one way or another, since that PR would then also have to revert this PR, and all the tests would have been touched for nothing.
The internal, unstable field of Pin can conflict with fields from the
inner type accessed via the Deref impl. Rename it from pointer to
__pointer, to make it less likely to conflict with anything else.
📌 Commit bc3fb52 has been approved by Amanieu,dtolnay
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
@dtolnay Has #119615 been decided against? As I said earlier, I'd like to avoid needless reverts in the near future if that ends up as the solution.
I am personally on board with #119615, but it has been waiting 2 weeks for feedback from petrochenkov, and I don't think it makes sense to block this fix on that. Inserting a revert into that PR later is not a big deal.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…, 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:
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 });
}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.
bors added a commit to rust-lang-ci/rust that referenced this pull request
bors added a commit to rust-lang-ci/rust that referenced this pull request
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
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:
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 });
}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.
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the library API team, which will review and decide on the PR/issue.