Rename `pointer` field on `Pin` #119562

LegionMammal978 · 2024-01-04T00:03:02Z

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 #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.

LegionMammal978 · 2024-01-04T00:05:58Z

@rustbot label +T-libs-api -T-libs

library/core/src/pin.rs

bors · 2024-01-08T03:05:34Z

☔ The latest upstream changes (presumably #119722) made this pull request unmergeable. Please resolve the merge conflicts.

Amanieu · 2024-01-08T18:15:18Z

@bors r+

bors · 2024-01-08T18:15:21Z

📌 Commit 985402d has been approved by Amanieu

It is now in the queue for this repository.

…, 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`.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#117744 (Add -Zuse-sync-unwind) - rust-lang#118649 (Make inductive cycles in coherence ambiguous always) - rust-lang#118979 (Use `assert_unsafe_precondition` for `char::from_u32_unchecked`) - rust-lang#119562 (Rename `pointer` field on `Pin`) - rust-lang#119619 (mir-opt and custom target fixes) - rust-lang#119632 (Fix broken build for ESP IDF due to rust-lang#119026) - rust-lang#119712 (Adding alignment to the cases to test for specific error messages.) - rust-lang#119734 (Miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2024-01-09T04:32:20Z

@bors r-
I think this failed in #119755 (comment)

LegionMammal978 · 2024-01-09T05:48:46Z

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.

bors · 2024-01-16T14:30:43Z

☔ The latest upstream changes (presumably #119672) made this pull request unmergeable. Please resolve the merge conflicts.

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.

dtolnay · 2024-01-26T04:40:27Z

@bors r=Amanieu,dtolnay

bors · 2024-01-26T04:40:29Z

📌 Commit bc3fb52 has been approved by Amanieu,dtolnay

It is now in the queue for this repository.

LegionMammal978 · 2024-01-26T05:15:22Z

@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.

dtolnay · 2024-01-26T05:23:50Z

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.

…, 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`.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#103522 (stabilise array methods) - rust-lang#113489 (impl `From<&[T; N]>` for `Cow<[T]>`) - rust-lang#119562 (Rename `pointer` field on `Pin`) - rust-lang#119800 (Document `rustc_index::vec::IndexVec`) - rust-lang#120368 (llvm-wrapper: remove llvm 12 hack) - rust-lang#120378 (always normalize `LoweredTy` in the new solver) - rust-lang#120382 (Classify closure arguments in refutable pattern in argument error) - rust-lang#120389 (Add fmease to the compiler review rotation) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#103522 (stabilise array methods) - rust-lang#113489 (impl `From<&[T; N]>` for `Cow<[T]>`) - rust-lang#119342 (Emit suggestion when trying to write exclusive ranges as `..<`) - rust-lang#119562 (Rename `pointer` field on `Pin`) - rust-lang#119800 (Document `rustc_index::vec::IndexVec`) - rust-lang#120205 (std: make `HEAP` initializer never inline) - rust-lang#120277 (Normalize field types before checking validity) - rust-lang#120311 (core: add `From<core::ascii::Char>` implementations) - rust-lang#120366 (mark a doctest with UB as no_run) - rust-lang#120378 (always normalize `LoweredTy` in the new solver) - rust-lang#120382 (Classify closure arguments in refutable pattern in argument error) - rust-lang#120389 (Add fmease to the compiler review rotation) r? `@ghost` `@rustbot` modify labels: rollup

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`.

rustbot assigned Amanieu Jan 4, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 4, 2024

rustbot added T-libs-api 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 Jan 4, 2024

This comment has been minimized.

Sign in to view

Nilstrieb reviewed Jan 4, 2024

View reviewed changes

library/core/src/pin.rs Outdated Show resolved Hide resolved

LegionMammal978 force-pushed the rename-pin-pointer branch from 0750d0d to 48fbe38 Compare January 4, 2024 15:40

This comment has been minimized.

Sign in to view

LegionMammal978 force-pushed the rename-pin-pointer branch from 48fbe38 to 816f1a7 Compare January 4, 2024 17:37

joboet added a commit to joboet/rust that referenced this pull request Jan 5, 2024

add regression test to adress the issue outlined in rust-lang#119562

7baae7c

joboet added a commit to joboet/rust that referenced this pull request Jan 5, 2024

add regression test to adress the issue outlined in rust-lang#119562

89f56a8

joboet mentioned this pull request Jan 5, 2024

Make pointer field of Pin private #119615

Closed

joboet added a commit to joboet/rust that referenced this pull request Jan 5, 2024

add regression test to adress the issue outlined in rust-lang#119562

a7ec2a5

LegionMammal978 force-pushed the rename-pin-pointer branch from 816f1a7 to 985402d Compare January 8, 2024 03:36

bors added S-waiting-on-bors 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 Jan 8, 2024

matthiaskrgr mentioned this pull request Jan 8, 2024

Rollup of 8 pull requests #119755

Closed

bors added S-waiting-on-author 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 Jan 9, 2024

LegionMammal978 force-pushed the rename-pin-pointer branch from 985402d to 1e03ed6 Compare January 9, 2024 05:44

LegionMammal978 force-pushed the rename-pin-pointer branch from 1e03ed6 to 126cbc5 Compare January 9, 2024 17:03

Rename pointer field on Pin

bc3fb52

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.

LegionMammal978 force-pushed the rename-pin-pointer branch from 126cbc5 to bc3fb52 Compare January 16, 2024 19:58

dtolnay assigned dtolnay and unassigned Amanieu Jan 26, 2024

dtolnay approved these changes Jan 26, 2024

View reviewed changes

bors added S-waiting-on-bors 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 Jan 26, 2024

matthiaskrgr mentioned this pull request Jan 26, 2024

Rollup of 8 pull requests #120394

Closed

matthiaskrgr mentioned this pull request Jan 26, 2024

Rollup of 12 pull requests #120401

Merged

bors merged commit 346397d into rust-lang:master Jan 27, 2024
11 checks passed

rustbot added this to the 1.77.0 milestone Jan 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename `pointer` field on `Pin` #119562

Rename `pointer` field on `Pin` #119562

LegionMammal978 commented Jan 4, 2024 •

edited

LegionMammal978 commented Jan 4, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Jan 8, 2024

Amanieu commented Jan 8, 2024

bors commented Jan 8, 2024

matthiaskrgr commented Jan 9, 2024

LegionMammal978 commented Jan 9, 2024 •

edited

bors commented Jan 16, 2024

dtolnay commented Jan 26, 2024

bors commented Jan 26, 2024

LegionMammal978 commented Jan 26, 2024 •

edited

dtolnay commented Jan 26, 2024

Rename pointer field on Pin #119562

Rename pointer field on Pin #119562

Conversation

LegionMammal978 commented Jan 4, 2024 • edited

LegionMammal978 commented Jan 4, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Jan 8, 2024

Amanieu commented Jan 8, 2024

bors commented Jan 8, 2024

matthiaskrgr commented Jan 9, 2024

LegionMammal978 commented Jan 9, 2024 • edited

bors commented Jan 16, 2024

dtolnay commented Jan 26, 2024

bors commented Jan 26, 2024

LegionMammal978 commented Jan 26, 2024 • edited

dtolnay commented Jan 26, 2024

Rename `pointer` field on `Pin` #119562

Rename `pointer` field on `Pin` #119562

LegionMammal978 commented Jan 4, 2024 •

edited

LegionMammal978 commented Jan 9, 2024 •

edited

LegionMammal978 commented Jan 26, 2024 •

edited