Skip to content
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

ACP: replace use of Pointee trait with a ptr::Metadata type #246

Open
CAD97 opened this issue Jul 10, 2023 · 0 comments
Open

ACP: replace use of Pointee trait with a ptr::Metadata type #246

CAD97 opened this issue Jul 10, 2023 · 0 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries

Comments

@CAD97
Copy link

CAD97 commented Jul 10, 2023

Proposal

Problem statement

ptr::metadata(*const T) currently returns <T as Pointee>::Metadata. Builtin-blanket-implemented traits are "spooky" and not super well supported, especially with associated types. mem::discriminant encapsulates the builtin trait type projection behind the mem::Discriminant type; doing so for ptr::metadata as well seems prudent.

API sketch

(reordered for improved reading order)

Updating the signature of the existing unstable items:

// mod core::ptr

pub fn metadata<T: ?Sized>(ptr: *const T) -> Metadata<T>;
pub fn from_raw_parts<T: ?Sized>(data: *const (), meta: Metadata<T>) -> *const T;
pub fn from_raw_parts_mut<T: ?Sized>(data: *const (), meta: Metadata<T>) -> *const T;

pub struct Metadata<T: ?Sized>(<T as Pointee>::Metadata);

// impl for Metadata<T>: Copy, Eq, Ord, Hash

impl<T: ?Sized> *const T {
    pub fn to_raw_parts(self) -> (*const (), Metadata<T>);
}

impl<T: ?Sized> *mut T {
    pub fn to_raw_parts(self) -> (*mut (), Metadata<T>);
}

impl<T: ?Sized> NonNull<T> {
    pub fn from_raw_parts(data: NonNull<()>, meta: Metadata<T>) -> NonNull<T>;
    pub fn fn to_raw_parts(self) -> (NonNull<()>, Metadata<T>);
}

Motivating examples or use cases

The primary motivation is that working with Metadata<T> is much more straightforward than with <T as Pointee>::Metadata. When working with generic decomposed pointer of unknown pointee kind, all you want is "the pointer metadata for T" opaquely, and access to knowing the specific metadata type for specific input types isn't particularly useful.

A secondary motivation is that by encapsulating more, it becomes easier to stabilize. Being a built-in, auto-implemented trait, Pointee is fundamentally more involved than a relatively simple Metadata struct. Relatedly, the Pointee trait has potential implications on custom DSTs / pointee kinds, if we want to support those, which can be deferred for longer by using an opaque Metadata struct.

Another potential benefit (and bias of the author) is the ability to extend pointer unsizing coercions to user defined pointerish types. It's not practical to provide unsizing support directly for <T as Pointee>::Metadata, but it can be added relatively straightforwardly for a proper Metadata struct, since it ties the pointer metadata strongly back to the pointee type. Providing unsizing in this way enables user defined pointerish types to get unsizing behavior without needing to carry an actual (potentially dummy) pointer, enabling size optimization for creative custom containers.

(As an example, coercing generational arena handles from pointing to distinct concrete types to some unifiable dyn type.)

It's the ACP author's opinion that having a typed Metadata struct provides a useful middle ground both a more strongly typed pointer metadata API and more loosely/openly typed solution. Metadata can offer conversions to/from specific pointer metadata kinds as we commit to them specifically being used. Additionally, the author believes there is meaningful semantic difference between "slice pointer metadata (length)" and usize and what operations should be immediately/easily accessible to either.

Even without potential lang benefits (i.e. unsizing), the API is in the ACP author's opinion improved by using this "facade" type. (Again, making a comparison to the same done for mem::discriminant.)

Potential answers to unresolved tracking issue questions

(Presented for a holistic picture, not necessarily as requisite answers for this ACP)

  • Lang:
    • Is it UB to have an improper vtable pointer? Decided w.r.t. trait upcasting, a proper vtable pointer is a safety invariant. (IIRC, nonnull and aligned are validity invariants, but not 100% certain.) Pointer metadata APIs must preserve this.
    • Safety of DynMetadata::size_of, etc. Deferred to later separate stabilization of DynMetadata. Given the trait/vtable upcasting decision, though, probably yes (assuming DynMetadata continues to carry a pointee type).
      • Note however, that size_of_val_raw must remain unsafe, since constructing a pointer to oversized slice is possible safely. Plus, custom pointee kinds might be more similar to [T] than dyn Trait and not have "proper" metadata as a pointer safety requirement.
  • Lib:
    • *mut (), u8, or Opaque for the raw pointer part? Should be chosen to match the allocation APIs, so likely *mut u8. This avoids a notable footgun when working with *mut () (that pointer offsetting is a no-op for ZSTs) and doesn't burden the API with the orthogonal complexity of thin-unsized (extern) types, despite using one being more theoretically correct in the abstract.
    • Should ptr::from_raw_parts be unsafe? No; Metadata<T> should always be a valid/safe metadata for *const T. Unsafety, if present, should be in the construction of Metadata<T> not from a pointer.
    • Should Sized imply Thin? Yes is the only reasonable answer imho. No is the more conservative answer which preserves possibilities for custom pointee kinds, but 0 as *mut T is already possible (and in a const context) given a T: Sized bound, so the possibility space is already constrained beyond any useful point. (The metadata type would need to provide a const default which is not incorrect for any pointer.)
    • Should DynMetadata take a type parameter? Deferred by encapsulation; Metadata<T> provides the strongly typed container for any pointee type. This makes an answer of no easier to provide, since the strong type is already provided, but does make conversion back unsafe.
  • Naming:
    • DynMetadata: Deferred. If without a type parameter, VTable or VTablePtr begin to make more sense.
    • Location of items: This ACP proposes a minimal stabilization surface of just the names listed above, which all clearly fit in core::ptr.

The author still wishes that slice-tailed pointer construction describing a too-large pointee could have been made (impossible, unsafe, or to) panic, but admits that this is a) long decided by ptr::slice_from_raw_parts and b) highly impractical in the face of as casts between slice-tail-having types.

Alternatives

Keep the existing unstable functions with the current signatures, but introduce/use a type alias Metadata<T> = <T as Pointee>::Metadata. This provides the readability benefits but leads to the API being a false friend to mem::Discriminant, which does use an opaque type to encapsulate DiscriminantKind.

Keep existing signatures, but introduce a separate typed Metadata struct. The struct is a useful container for manipulating split pointers, but relegating it to an optional addon API limits its utility, even if it would still provide the other benefits.

Don't change anything. The unstable status quo is functional, but doesn't seem like it's going to make progress towards stabilization soon. And a lot of interesting ecosystem potential is gated behind the ability to tear apart pointers, or at least the ability to combine one pointer's address+provenance with another's metadata.

Links and related work

The base temperature as I measure it is that either the API or the lang parts don't quite justify themselves independently, but I believe they definitely do together. But I do still personally think the API part does stand on its own independently as well, with the lang part just as an additional side benefit.

A "no, we don't think so" would also be graciously accepted as a closure of this ACP, and permit me to stop thinking about this extension. 🙂

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@CAD97 CAD97 changed the title ACP: replace Pointee trait with a Discriminant type ACP: replace Pointee trait with a ptr::Metadata type Jul 30, 2023
@CAD97 CAD97 changed the title ACP: replace Pointee trait with a ptr::Metadata type ACP: replace use of Pointee trait with a ptr::Metadata type Jul 30, 2023
@dtolnay dtolnay added the api-change-proposal A proposal to add or alter unstable APIs in the standard libraries label Nov 23, 2023
@Amanieu Amanieu added the I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. label Sep 17, 2024
@Amanieu Amanieu removed the I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries
Projects
None yet
Development

No branches or pull requests

3 participants