rust-lang / rust-lang/rust

ptr-metadata and DynMetadata generics

Open
#161,779 3 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

F-ptr_metadata F-try_as_dyn needs-triage
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Background

While looking into https://github.com/rust-lang/rust/issues/144361 (try_as_dyn) and figuring out where its currently really blocked, I noticed that pointer metadata (https://github.com/rust-lang/rust/issues/81513) right now has a few inconsistencies that I believe block a proper try_as_dyn implementation as well as having already produced a hacky API surface on type_info (https://github.com/rust-lang/rust/issues/146922).

DynMetadata<Dyn> generics are a lie

The struct intentionally uses a PhantomData<Dyn> to justify the Dyn generic and in reality only holds a NonNull<VTable>. I think that the Dyn type parameter is meant to be used for core::ptr::from_raw_parts since it actually influences the return T here (as in, you can only produce a fat pointer to a Dyn if you actually supply metadata for a Dyn).

However, the kind of metadata is the same for every dyn - a vtable. The pointer "value" you pass to core::ptr::from_raw_parts is, by design, already erased and not type safe. core::ptr::from_raw_parts already documents this:

This function is safe but the returned pointer is not necessarily safe to dereference. For slices, see the documentation of slice::from_raw_parts for safety requirements. For trait objects, the metadata must come from a pointer to the same underlying erased type.

a) DynMetadata<Dyn> is inconsistent with other metadata type safety

Rust today prevents you from constructing a *const dyn A with a DynMetadata<B> using core::ptr::from_raw_parts. It however doesn't prevent you from constructing a *const [u32] using the metadata from a [u16]. I think the latter here is actually the problem, but it also shows that just because 2 types have the same metadata type (and I'd argue different dyn T's have the same metadata type, just not the same value), their metadata can't necessarily be used interchangeably (modulo that this already was always the case for the thin part of the pointer).

Long story short: from_raw_parts prevents you from swapping Metadata from dyn A und dyn B, but not from [A] and [B], while it is probably desirable to check this in both cases (and optionally override it).

b) DynMetadata<Dyn> generic is being abused already

The documentation of DynMetadata<Dyn> reads:

It is possible to name this struct with a type parameter that is not a dyn trait object (for example DynMetadata) but not to obtain a meaningful value of that struct.

This can be interpreted two ways:

  1. a DynMetadata<T> (where T is not dyn) is uninhabited (as in, it is literally impossible to obtain a value of DynMetadata<*const ()>, because *const () is not dyn and thus no valid value of that type can exist)
  2. a DynMetadata<T> (where T is not dyn) is not forbidden from existing, just useless. This makes it questionable what the methods (eg. size_of, layout and so on) would return, but in theory them returning invalid values would be ok.

My reading here is that the Intention is 1., but nothing enforces that and in fact, this is already broken today: TraitImpl::<T>::get_vtable returns a DynMetadata<T>, and TypeId::trait_info_of_trait_type_id returns a TraitImpl<*const ()>. Invoking get_vtable on this then gives you a DynMetadata<*const ()> (this also breaks 2.):

struct TheStruct;
trait TheTrait {}

impl TheTrait for TheStruct {}

let impl_of_trait = const {
    let the_struct_id = TypeId::of::<TheStruct>();
    let the_trait_id = TypeId::of::<dyn TheTrait>();

    the_struct_id.trait_info_of_trait_type_id(the_trait_id)
};

// Prints a valid VTable even though this is DynMetadata<*const ()>
println!("VTable: {:?}", impl_of_trait.unwrap().get_vtable());

Playground link

DynMetadata should not be generic

The following would solve both a) and b) I think (although I'm not sure what it might break otherwise). Introduce a new struct and change core::ptr::from_raw_parts (and adjacent methods):

struct PointerMetadata<T: ?Sized> {
    metadata: <T as Pointee>::Metadata
}

pub const fn from_raw_parts<T>(
    data_pointer: *const ()
    metadata: PointerMetadata<T>, // Use the wrapper
) -> *const T
where
    T: ?Sized;

and remove the generics from DynMetadata and TraitImpl.

This concretely gives a 3 things:

  1. TypeId::trait_info_of_trait_type_id doesn't have to fabricate a *const () generic as a wildcard anymore, it simply returns an unparameterized TraitImpl
  2. DynMetadata as a value does not inconsistently state anything about its associated type anymore and also becomes storeable in non-generic contexts. If you can already store a DynMetadata<T> because you have access to T, chances are you could have stored a *const dyn T already. In the scenario where you'd likely want to store a DynMetadata separately you probably don't have access to that T
  3. PointerMetadata<T> stays associated with its original type, for example a slice of [A] even if [A]'s metadata is usize. For unsafe constructs PointerMetadata<T> can then be allowed to decompose into its Metadata type PointerMetadata::<T>::into_raw() -> <T as Pointee>::Metadata

Sorry for the wall of text, I hope I did not miss some prior discussion about this exact topic.

TL;DR: DynMetadata<Dyn> having a generic parameter prohibits some use cases and forces hacks

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the linked pointer-metadata and try_as_dyn issues, then inspect the documented APIs for core::ptr::from_raw_parts, DynMetadata, TraitImpl, and TypeId::trait_info_of_trait_type_id. Reproduce the provided Playground example and map the proposed PointerMetadata design against the existing behavior. Done means reaching agreement on the API and its compatibility implications.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.