"discriminant_value" intrinsic returns u64, cannot fit all discriminants

[RalfJung]

Ever since we had enums with repr(u128)/repr(i128), the type of the discriminant_value intrinsic was no longer adequate: it returns a u64, which simply cannot fit all discriminant values.

@eddyb says the type of the intrinsic should be adjusted. This will affect both its codegen and Miri implementation.

This issue has been assigned to @lcnr via this comment.

[eddyb]

cc @rust-lang/libs I wonder if we're safe to change std::mem::discriminant.
We might need a crater run as I suspect there are people who transmute the result to u64.

[sfackler]

A crater run would be fine, but we should not block a change on people doing things like that IMO.

[dtolnay]

I wouldn't have any sympathy for people transmuting to u64.

If you want to be slick, you could make std::mem::Discriminant<T> have the same size as T's discriminant, rather than always 8 bytes. This supports enums with a 128 bit discriminant without increasing the size of Discriminant in the common case.

[lcnr]

Would the following work?

Add a (potentially hidden) permanently unstable trait

trait DiscriminantKind {
    // the traits are required by `mem::Discriminant`
    type Discriminant: Clone + Copy + Debug + Eq + PartialEq + Hash + Send + Sync + Unpin;
}

which is automatically implemented for every type and change intrinsics::discriminant to return DiscriminantKind::Discriminant instead.

mem::Discriminant should now be able to use <T as DiscriminantKind>::Discriminant as its only field.

This allows for mem::Discriminant to be smaller in most cases while still being fairly simple to implement afaict.

[nagisa]

The Discriminant type was originally made opaque due to exactly this issue IIRC (and a couple other reasons). If only there was a way to prohibit transmuting it, or mark the type Unsized...

---

@lcnr There’s no point in making the intrinsic hide its implementation details, as the intrinsics are inherently unstable.

[lcnr]

The reason why I considered hiding it is to not pollute rustdoc by adding this (for the user irrelevant) impl to every type.

[lcnr]

@rustbot claim

[SimonSapin]

Rather than perma-unstable, I think the trait could be private to libcore. (The Discriminant struct doesn’t need a trait bound since the trait needs to be implemented for all types anyway: for example std::mem::discriminant(&true) compiles today.)

An associated type in a trait is required because that’s how the compiler supports having a field of different types in struct Discriminant<T> { … } depending on T, but these are implementation details that don’t need to be visible in any public API.

[lcnr]

@SimonSapin While it is not required as a bound for Discriminant, it is required for the return type of intrinsics::discriminant_value which is public