Tracking Issue for [const T|mut T]::with_metadata_of #75091

oliver-giersch · 2020-08-03T07:34:16Z

Feature gate: #![feature(set_ptr_value)]

Public API

impl<T: ?Sized> *mut T {
    pub fn with_metadata_of<U: ?Sized>(self, val: *const U) -> *mut U;
}

impl<T: ?Sized> *const T {
    pub fn with_metadata_of<U: ?Sized>(self, val: *const U) -> *const U;
}

Steps / History

Implementation: adds [*mut|*const] ptr::set_ptr_value #74774
Change type and add #[must_use]: Requested changes to [*mut T|*const T]::set_ptr_value #75407
Reverse argument order and rename to with_metadata_of: Refactor set_ptr_value as with_metadata_of #95249
Const argument for mutable with_metadata_of: Adjust argument type for mutable with_metadata_of (#75091) #103346
Final commenting period (FCP)
Stabilization PR

Unresolved Questions

Correct argument type -- *mut ()? *mut u8? *mut Opaque?
Is this even needed/useful given that there now also is Tracking Issue for pointer metadata APIs #81513?

The text was updated successfully, but these errors were encountered:

…fJung Requested changes to [*mut T|*const T]::set_ptr_value This is a follow-up to PR rust-lang#74774 (tracking issue rust-lang#75091), acting on some change requests made after approval: - adds `#[must_use]` attribute - changes type of `val` pointer argument from `()` to `u8` - adjusts documentation mentioning pointer provenance

SimonSapin · 2021-04-16T16:14:19Z

As discussed on Zulip this could be replaced with ptr::from_raw_parts(val, ptr::metadata(self)), tracked in #81513

Refactor set_ptr_value as with_metadata_of Replaces `set_ptr_value` (rust-lang#75091) with methods of reversed argument order: ```rust impl<T: ?Sized> *mut T { pub fn with_metadata_of<U: ?Sized>(self, val: *mut U) -> *mut U; } impl<T: ?Sized> *const T { pub fn with_metadata_of<U: ?Sized>(self, val: *const U) -> *const U; } ``` By reversing the arguments we achieve several clarifications: - The function closely resembles `cast` with an argument to initialize the metadata. This is easier to teach and answers a long outstanding question that had restricted cast to `Sized` pointee targets. See multiples reviews of <rust-lang#47631> - The 'object identity', in the form of provenance, is now preserved from the receiver argument to the result. This helps explain the method as a builder-style, instead of some kind of setter that would modify something in-place. Ensuring that the result has the identity of the `self` argument is also beneficial for an intuition of effects. - An outstanding concern, 'Correct argument type', is avoided by not committing to any specific argument type. This is consistent with cast which does not require its receiver to be a 'raw address'. Hopefully the usage examples in `sync/rc.rs` serve as sufficient examples of the style to convince the reader of the readability improvements of this style, when compared to the previous order of arguments. I want to take the opportunity to motivate inclusion of this method _separate_ from metadata API, separate from `feature(ptr_metadata)`. It does _not_ involve the `Pointee` trait in any form. This may be regarded as a very, very light form that does not commit to any details of the pointee trait, or its associated metadata. There are several use cases for which this is already sufficient and no further inspection of metadata is necessary. - Storing the coercion of `*mut T` into `*mut dyn Trait` as a way to dynamically cast some an arbitrary instance of the same type to a dyn trait instance. In particular, one can have a field of type `Option<*mut dyn io::Seek>` to memorize if a particular writer is seekable. Then a method `fn(self: &T) -> Option<&dyn Seek>` can be provided, which does _not_ involve the static trait bound `T: Seek`. This makes it possible to create an API that is capable of utilizing seekable streams and non-seekable streams (instead of a possible less efficient manner such as more buffering) through the same entry-point. - Enabling more generic forms of unsizing for no-`std` smart pointers. Using the stable APIs only few concrete cases are available. One can unsize arrays to `[T]` by `ptr::slice_from_raw_parts` but unsizing a custom smart pointer to, e.g., `dyn Iterator`, `dyn Future`, `dyn Debug`, can't easily be done generically. Exposing `with_metadata_of` would allow smart pointers to offer their own `unsafe` escape hatch with similar parameters where the caller provides the unsized metadata. This is particularly interesting for embedded where `dyn`-trait usage can drastically reduce code size.

ChayimFriedman2 · 2022-03-29T07:17:43Z

Even if we want to keep these methods I think it's better to write them in terms of the metadata API as it's easy to reason about and less error prone than the current implementation.

The method takes two pointer arguments: one `self` supplying the pointer value, and a second pointer supplying the metadata. The new parameter type more clearly reflects the actual requirements. The provenance of the metadata parameter is disregarded completely. Using a mutable pointer in the call site can be coerced to a const pointer while the reverse is not true. An example of the current use: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = val as *const _ as *mut T; let ptr = uninit.as_ptr().with_metadata_of(ptr); ``` This could then instead be simplified to: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = uninit.as_ptr().with_metadata_of(&**val); ```

…nter_argument, r=dtolnay Adjust argument type for mutable with_metadata_of (rust-lang#75091) The method takes two pointer arguments: one `self` supplying the pointer value, and a second pointer supplying the metadata. The new parameter type more clearly reflects the actual requirements. The provenance of the metadata parameter is disregarded completely. Using a mutable pointer in the call site can be coerced to a const pointer while the reverse is not true. In some cases, the current parameter type can thus lead to a very slightly confusing additional cast. [Example](HeroicKatora/static-alloc@cad9377). ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = val as *const _ as *mut T; let ptr = uninit.as_ptr().with_metadata_of(ptr); ``` This could then instead be simplified to: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = uninit.as_ptr().with_metadata_of(&**val); ``` Tracking issue: rust-lang#75091 `@dtolnay` you're reviewed rust-lang#95249, would you mind chiming in?

…nter_argument, r=dtolnay Adjust argument type for mutable with_metadata_of (rust-lang#75091) The method takes two pointer arguments: one `self` supplying the pointer value, and a second pointer supplying the metadata. The new parameter type more clearly reflects the actual requirements. The provenance of the metadata parameter is disregarded completely. Using a mutable pointer in the call site can be coerced to a const pointer while the reverse is not true. In some cases, the current parameter type can thus lead to a very slightly confusing additional cast. [Example](HeroicKatora/static-alloc@cad9377). ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = val as *const _ as *mut T; let ptr = uninit.as_ptr().with_metadata_of(ptr); ``` This could then instead be simplified to: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = uninit.as_ptr().with_metadata_of(&**val); ``` Tracking issue: rust-lang#75091 ``@dtolnay`` you're reviewed rust-lang#95249, would you mind chiming in?

Rollup of 7 pull requests Successful merges: - rust-lang#102602 (Slightly tweak comments wrt `lint_overflowing_range_endpoint`) - rust-lang#103190 (rustdoc: render bounds of cross-crate GAT params) - rust-lang#103224 (Allow semicolon after closure within parentheses in macros) - rust-lang#103280 ((rust-lang#102929) Implement `String::leak` (attempt 2)) - rust-lang#103329 (Add a forgotten check for NonNull::new_unchecked's precondition) - rust-lang#103346 (Adjust argument type for mutable with_metadata_of (rust-lang#75091)) - rust-lang#103360 (Reduce false positives in msys2 detection) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…ation__--this-can-be-simplified, r=scottmcm Simplify some pointer method implementations - Make `pointer::with_metadata_of` const (+simplify implementation) (cc rust-lang#75091) - Simplify implementation of various pointer methods r? `@scottmcm` ---- `from_raw_parts::<T>(this, metadata(self))` was annoying me for a while and I've finally figured out how it should _actually_ be done.

…ation__--this-can-be-simplified, r=scottmcm Simplify some pointer method implementations - Make `pointer::with_metadata_of` const (+simplify implementation) (cc rust-lang#75091) - Simplify implementation of various pointer methods r? ``@scottmcm`` ---- `from_raw_parts::<T>(this, metadata(self))` was annoying me for a while and I've finally figured out how it should _actually_ be done.

…ation__--this-can-be-simplified, r=scottmcm Simplify some pointer method implementations - Make `pointer::with_metadata_of` const (+simplify implementation) (cc rust-lang#75091) - Simplify implementation of various pointer methods r? ```@scottmcm``` ---- `from_raw_parts::<T>(this, metadata(self))` was annoying me for a while and I've finally figured out how it should _actually_ be done.

…ment, r=dtolnay Adjust argument type for mutable with_metadata_of (#75091) The method takes two pointer arguments: one `self` supplying the pointer value, and a second pointer supplying the metadata. The new parameter type more clearly reflects the actual requirements. The provenance of the metadata parameter is disregarded completely. Using a mutable pointer in the call site can be coerced to a const pointer while the reverse is not true. In some cases, the current parameter type can thus lead to a very slightly confusing additional cast. [Example](HeroicKatora/static-alloc@cad9377). ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = val as *const _ as *mut T; let ptr = uninit.as_ptr().with_metadata_of(ptr); ``` This could then instead be simplified to: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = uninit.as_ptr().with_metadata_of(&**val); ``` Tracking issue: rust-lang/rust#75091 ``@dtolnay`` you're reviewed #95249, would you mind chiming in?

WaffleLapkin · 2023-11-07T22:22:23Z

This needs a better motivational example. From current docs:

This function is primarily useful for allowing byte-wise pointer
arithmetic on potentially fat pointers:

#![feature(set_ptr_value)]
use core::fmt::Debug;
let mut arr: [i32; 3] = [1, 2, 3];
let mut ptr = arr.as_mut_ptr() as *mut dyn Debug;
let thin = ptr as *mut u8;
unsafe {
    ptr = thin.add(8).with_metadata_of(ptr);
    assert_eq!(*(ptr as *mut i32), 3);
    println!("{:?}", &*ptr); // will print "3"
}

This is superceeded by byte_add and co (#96283)

dtolnay · 2023-11-07T22:32:35Z

dyn-clone perhaps? dtolnay/dyn-clone#8

HeroicKatora · 2023-11-16T21:28:02Z

flexible-io is the implementation of the solution devised a while ago to the problem and another instance which originally got me invested in this PR. It relies on with_metadata_of. The full metadata API might be an optimization for later, but is way more complex than necessary here.

bjorn3 · 2024-04-29T17:12:59Z

That will use the provenance of from rather than the provenance of onto, which will cause UB if you dereference the resulting pointer, but the provenance of from doesn't allow accessing this address. Miri is entirely correct in rejecting this. With rustc it may work by accident or you may get a miscompilation.

GnomedDev · 2024-09-24T10:39:45Z

For the "Is this even needed/useful given that there now also is #81513?", I feel like this is super useful even with or without the metadata APIs.

Without (as stable is now) it allows for working with metadata in a limited fashion without assuming layout (which people are, and will do if not given the correct tools).
With, this is still useful as a higher level helper function, and doesn't block any of the metadata API stuff as a with_metadata function could be added to attach a Pointee::Metadata to an existing pointer in future.

I'd love to see this stabilized, if that is a good resolution to the last question needed answering.

Noratrieb · 2024-09-27T15:35:34Z

@rust-lang/libs-api see above

scottmcm · 2024-10-02T22:30:48Z

Hmm, I think the primary alternative to this would be something like rust-lang/libs-team#246 -- instead of copying the metadata from a full other pointer, just remove the blocker to letting you pass around the metadata directly.

It's not obvious to me that people really ever want this version where you need to pass a full pointer, just to ignore its address. Or is there some situation I'm not thinking of where that's really better than doing a let (p, m) = pointer.parts(); then p.whatever().with_metadata(m)?

GnomedDev · 2024-10-03T07:48:53Z

As I said above @scottmcm:

Without pointer metadata: This is a simpler API that can be stable now to allow people to do what they are already doing but soundly (without assuming fat pointer layout).
With pointer metadata: This is a helper function for the situation where someone does have a full pointer and can avoid splitting it into parts just to add the metadata on.

My motivating example is https://github.com/andylokandy/smallbox which has been assuming fat pointer layout for years and I recently added a nightly feature which uses this API and strict provenance. Since strict provenance is in the process of being stabilized, this being stabilized as well would be amazing for smallbox.

RalfJung · 2024-10-04T07:00:05Z

@HeroicKatora IMO it makes little sense to have a function with_metadata_of<U: ?Sized>(self, other: Metadata<U>) -> *mut U -- that should be called with_metadata! The of part refers to "with the metadata of another pointer"; if we are directly providing the metadata, we should just say "with this metadata", and there's no "of" involved any more.

This tracking issue is specifically about copying the metadata from one pointer to another. Your proposal would be part of #81513.

HeroicKatora · 2024-10-04T10:09:35Z

I understood @scottmcm as a new question, separate from #81513 itself, to explore whether rust-lang/libs-team#246 changes something about the unresolved redundancy question in the issue. It is not accepted, so not yet a part of #81513 to my understanding (and not linked there either). Maybe I'm misreading the process here.

Neither the API of this (#75091) nor the sketch based on rust-lang/libs-team#246 would require stabilization of the Metadata trait itself. I think this affects the merit of the simplicity argument. The representation without address is superior for some library use cases. (It would definitely occur in practice, infact I'm sure my libraries would always prefer the address-less value). I concur, those methods should be named with_metadata, not with_metadata_of, so there's no necessary overlap in the interface and it would of course be possible to have both sets of methods.

Are you suggesting the redundancy even with the ACP is slight enough that it would still not be problematic, and there's enough merit to consuming a pointer directly? Definitely a reasonable position to take. I just intended for it to be clarified that the ACP presents a (middle ground) alternative not yet explored in comments in this thread. If libs is in favor of potentially having both that's fine 👍

RalfJung · 2024-10-04T10:23:37Z

All I am saying is, we should not have functions called with_metadata_of that take a pointer and a metadata. That name should only be used for functions that take two pointers.

This issue here is specifically about the two-pointer version of the APIs, which avoids stabilizing any new type. with_metadata in the form you suggested IMO makes a lot of sense (it nicely matches with_addr as well), but such an API should be proposed in #81513 since it clearly requires talking about "a metadata value matching this pointer type". Whether that is expressed as Metadata<T> or <T as Pointee>::Metadata is a completely orthogonal question.

IOW, rust-lang/libs-team#246 is a modification of #81513, but has absolutely no bearing on this issue.

…tation, r=Mark-Simulacrum Expand set_ptr_value / with_metadata_of docs In preparation of a potential FCP, intends to clean up and expand the documentation of this operation. Rewrite these blobs to explicitly mention the case of a sized operand. The previous made that seem wrong instead of emphasizing it is nothing but a simple cast. Instead, the explanation now emphasizes that the address portion of the argument, together with its provenance, is discarded which previously had to be inferred by the reader. Then an example demonstrates a simple line of incorrect usage based on this idea of provenance. Tracking issue: rust-lang#75091

Rollup merge of rust-lang#131339 - HeroicKatora:set_ptr_value-documentation, r=Mark-Simulacrum Expand set_ptr_value / with_metadata_of docs In preparation of a potential FCP, intends to clean up and expand the documentation of this operation. Rewrite these blobs to explicitly mention the case of a sized operand. The previous made that seem wrong instead of emphasizing it is nothing but a simple cast. Instead, the explanation now emphasizes that the address portion of the argument, together with its provenance, is discarded which previously had to be inferred by the reader. Then an example demonstrates a simple line of incorrect usage based on this idea of provenance. Tracking issue: rust-lang#75091

…=Mark-Simulacrum Expand set_ptr_value / with_metadata_of docs In preparation of a potential FCP, intends to clean up and expand the documentation of this operation. Rewrite these blobs to explicitly mention the case of a sized operand. The previous made that seem wrong instead of emphasizing it is nothing but a simple cast. Instead, the explanation now emphasizes that the address portion of the argument, together with its provenance, is discarded which previously had to be inferred by the reader. Then an example demonstrates a simple line of incorrect usage based on this idea of provenance. Tracking issue: rust-lang/rust#75091

oliver-giersch added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Aug 3, 2020

jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 3, 2020

KodrAus added Libs-Tracked Libs issues that are tracked on the team's project board. A-raw-pointers Area: raw pointers, MaybeUninit, NonNull labels Aug 6, 2020

oliver-giersch mentioned this issue Aug 11, 2020

Requested changes to [*mut T|*const T]::set_ptr_value #75407

Merged

CAD97 mentioned this issue Jan 7, 2021

Re-stabilize Weak::as_ptr and friends for unsized T #80764

Merged

HeroicKatora mentioned this issue Mar 23, 2022

Refactor set_ptr_value as with_metadata_of #95249

Merged

HeroicKatora mentioned this issue Oct 21, 2022

Adjust argument type for mutable with_metadata_of (#75091) #103346

Merged

dtolnay changed the title ~~Tracking Issue for [*const T|*mut T]::set_ptr_value~~ Tracking Issue for [*const T|*mut T]::with_metadata_of Oct 21, 2022

Dylan-DPC mentioned this issue Oct 22, 2022

Rollup of 7 pull requests #103393

Closed

Dylan-DPC mentioned this issue Oct 22, 2022

Rollup of 7 pull requests #103398

Merged

WaffleLapkin mentioned this issue Oct 28, 2022

Simplify some pointer method implementations #103701

Merged

This comment was marked as off-topic.

Sign in to view

This comment has been minimized.

Sign in to view

RalfJung mentioned this issue Oct 4, 2024

Tracking Issue for pointer metadata APIs #81513

Open

3 tasks

HeroicKatora mentioned this issue Oct 6, 2024

Expand set_ptr_value / with_metadata_of docs #131339

Merged

luca3s mentioned this issue Oct 19, 2024

Add T: ?Sized to every Smart Pointer in the Library glowcoil/basedrop#8

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracking Issue for [const T|mut T]::with_metadata_of #75091

Tracking Issue for [const T|mut T]::with_metadata_of #75091

oliver-giersch commented Aug 3, 2020 •

edited by dtolnay

Loading

SimonSapin commented Apr 16, 2021

ChayimFriedman2 commented Mar 29, 2022

WaffleLapkin commented Nov 7, 2023

dtolnay commented Nov 7, 2023

HeroicKatora commented Nov 16, 2023 •

edited

Loading

This comment was marked as off-topic.

bjorn3 commented Apr 29, 2024

GnomedDev commented Sep 24, 2024

Noratrieb commented Sep 27, 2024

scottmcm commented Oct 2, 2024

GnomedDev commented Oct 3, 2024 •

edited

Loading

This comment has been minimized.

RalfJung commented Oct 4, 2024 •

edited

Loading

HeroicKatora commented Oct 4, 2024 •

edited

Loading

RalfJung commented Oct 4, 2024 •

edited

Loading

Tracking Issue for [*const T|*mut T]::with_metadata_of #75091

Tracking Issue for [*const T|*mut T]::with_metadata_of #75091

Comments

oliver-giersch commented Aug 3, 2020 • edited by dtolnay Loading

Public API

Steps / History

Unresolved Questions

SimonSapin commented Apr 16, 2021

ChayimFriedman2 commented Mar 29, 2022

WaffleLapkin commented Nov 7, 2023

dtolnay commented Nov 7, 2023

HeroicKatora commented Nov 16, 2023 • edited Loading

This comment was marked as off-topic.

bjorn3 commented Apr 29, 2024

GnomedDev commented Sep 24, 2024

Noratrieb commented Sep 27, 2024

scottmcm commented Oct 2, 2024

GnomedDev commented Oct 3, 2024 • edited Loading

This comment has been minimized.

RalfJung commented Oct 4, 2024 • edited Loading

HeroicKatora commented Oct 4, 2024 • edited Loading

RalfJung commented Oct 4, 2024 • edited Loading

Tracking Issue for [const T|mut T]::with_metadata_of #75091

Tracking Issue for [const T|mut T]::with_metadata_of #75091

oliver-giersch commented Aug 3, 2020 •

edited by dtolnay

Loading

HeroicKatora commented Nov 16, 2023 •

edited

Loading

GnomedDev commented Oct 3, 2024 •

edited

Loading

RalfJung commented Oct 4, 2024 •

edited

Loading

HeroicKatora commented Oct 4, 2024 •

edited

Loading

RalfJung commented Oct 4, 2024 •

edited

Loading