Let's talk about pointer::set_ptr_value

internals
1

#75091 was introduced with the following API. Let 's address the open questions.

impl<T: ?Sized> *mut T {
    pub fn set_ptr_value(mut self, val: *mut u8) -> Self;
}

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

This API has been found to be difficult to grasp. I believe this is because it is actually suboptimal. The operation uses the provenance and pointer value of the argument, and metadata of self. To me, this is the wrong way around. The 'object identity' of the result is provided by the argument. A better formulation would be to have it be provided be the receiver. Thus, I would propose to change it like so:

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

    // With obvious parallel to:
    // pub const fn cast<U>(self) -> *mut U;
}

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

    // pub const fn cast<U>(self) -> *const U;
}

Note that this has the effect of 'resolving' the outstanding question by rather eliminating the need to agree on a single 'address'-pointer entirely.

A further comment asks if pointer metadata would make this method redundant. I don't think that this is the case. Firstly, the methods do not mention Metadata at all. Secondly, this can be chained while splitting and recombination is needlessly hard to read. Let's compare actual usage:

Currently:

//! library/alloc/src/rc.rs: 896
// Reverse the offset to find the original RcBox.
let rc_ptr = unsafe { (ptr as *mut RcBox<T>).set_ptr_value((ptr as *mut u8).offset(-offset)) };

With proposed change in API:

//! library/alloc/src/rc.rs: 896
// Reverse the offset to find the original RcBox.
let rc_ptr = unsafe { (ptr as *mut u8).offset(-offset).cast_to(ptr as *mut RcBox<T>) };

Using metadata introduces new complexity with no readability benefit:

//! library/alloc/src/rc.rs: 896
// Reverse the offset to find the original RcBox.
let rc_ptr = unsafe {
    let addr = (ptr as *mut u8).offset(-offset) as *mut ();
    // Note: since metadata is not variant, we need this cast anyways.
    let meta = ptr::metadata(ptr.cast_to(ptr as *mut RcBox<T>));
    ptr::from_raw_parts_mut(addr, meta);
}

I also have at least two distinct use cases in crates that require the use of set_ptr_value, but not the full metadata interface. I'd be very glad if they could transition to a sanctioned way of modifying unsized pointers quickly rather than have to wait on the many, many more open questions on ptr_metadata.

  • A custom, macro-based equivalent of unsize (Another feature with not expectation of quick stabilization) so that no-std smart-pointers can hold dyn-traits, which is highly beneficial for embedded due to a number of reasons. (Ask if further elaboration is beneficial).
  • A struct around T: Read that is optionally dyn Seek/dyn BufRead with no static trait bound on those. Rather, only fn as_seek(&mut self) -> Option<&mut dyn Seek> is offered. This is easily achieved by storing an additional field of type Option<*mut dyn Seek> in the struct and setting the pointer address to the value field's address on calling as_seek.
1 Like
2

I think this is orthogonal to the order of the arguments. set_ptr_value could solve this problem the same way by being generic over the type pointed by val. Similarly cast_to would have the same problem if it was defined on *const u8/*mut u8 instead of being generic over T. Anyway, this solution seems reasonable to me, except the T: ?Sized bound: it makes T no longer "just a pointer".

The name cast_to still feels a bit weird, it doesn't really make sense to me to cast a pointer to another (existing) pointer. I feel like the could be a better name, for example with_metadata_of.

3

The same could have be said about the original cast. Interestingly the issue of sized receiver pointee on the original NonNull::cast came up but instead @dtolnay had suggesting relaxing it to any ?Sized.

This is indeed a shift in how the operation is framed. Instead of 'change the pointer address', it rather addresses this open question: Would it be possible to add a cast_unsized or similar method that converts into a fat pointer?. And in answering this it is similar to Layout::new/Layout::for_val in that it introduces a fat-pointer to serve as the template for the pointer metadata. I find this much more natural than a constructor (which with_* naming would imply).

That said, bikeshedding to make the naming more closely aligned to mem::size_of_val_raw, Layout::for_value_raw, etc. would be welcome. Just the particle raw seems superficial in the context of raw pointers.

4

To be clear, I wasn't proposing a constructor, just a different name.

5

The name cast_to strikes me as really confusing. In the phrase "cast X to Y" I would expect Y to refer to a type.

I agree that with_metadata_of is a better name. The with_ prefix does usually refer to a constructor, but we already have Path::with_file_name and Path::with_extension as precedent for using it for functions that return "self but with some aspect altered".

Why "variant"? The issue with from_raw doesn't have to do with lifetime variance, does it?

It seems like the issue is that T::Metadata and RcBox<T>::Metadata are different types, even though they are effectively guaranteed to be the same thing, or at least convertible to each other, due to the ability to cast from *mut T to *mut RcBox<T> and vice versa.

Ideally it would be possible to do something like

ptr::from_raw_parts_mut(addr, ptr::metadata(ptr).into())

which I think does it more clear what's going on than the current implementation.

But I can buy that spec'ing and implementing an API allowing those metadata types to be converted to each other (or treated as equal) would take longer than stabilizing this function.

3 Likes
6

Thanks for the name recommendation, it's merged.

2 Likes
7

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.