Consider not transmuting mem::Discriminant at all.

[eddyb]

The current version of this crate relies on transmuting mem::Discriminant<T> values:

enum-ordinalize/src/lib.rs

Lines 423 to 445 in 80b7469

	let ordinal = if_rust_version! { >= 1.45 {
	if _has_repr {
	quote! {
	#[inline]
	pub fn ordinal(&self) -> #variant_type {
	unsafe {
	::core::mem::transmute(::core::mem::discriminant(self))
	}
	}
	}
	} else {
	quote! {
	#[inline]
	pub fn ordinal(&self) -> #variant_type {
	let n: usize = unsafe {
	::core::mem::transmute(::core::mem::discriminant(self))
	};
	n as #variant_type
	}
	}
	}
	} else {

But it's clear from the code that this pathway was kept despite it having broken at least once. Why was it kept, for performance?

IIRC (from conversations with @nox, I believe), LLVM can optimize identity switches, you just have to get the values right (we added the ability to specify explicit discriminants in enums with data, to allow "lining up" two enums for that optimization).

Please consider not using unsafe code to get around standard library stability, or at least putting it behind a nightly-only feature (which could be made to only compile on nightly by use of #![cfg_attr(feature = "nightly", feature(...))]).

cc @Hoverbear (who stumbled over an older version of this crate being broken, we assume)

[eddyb]

Just confirmed that this example:

pub enum Foo {
    A,
    B(i32),
    C(String),
    D([u8; 200])
}

pub fn discr(foo: &Foo) -> isize {
    match foo {
        Foo::A => 0,
        Foo::B(_) => 1,
        Foo::C(_) => 2,
        Foo::D(_) => 3,
    }
}

compiles to:

  %0 = getelementptr inbounds %Foo, %Foo* %foo, i64 0, i32 0, i64 0
  %1 = load i8, i8* %0, align 8, !range !2
  %_2 = zext i8 %1 to i64
  ret i64 %_2

Even if I change the 2 to a 5, LLVM ends up using a [i64 0, i64 1, i64 5, i64 3] "switch table" instead of a switch.

Maybe when the transmute was originally written this optimization wasn't available?

[magiclen]

I thought mem::transmute(mem::discriminant(...)) was faster than match, but clearly I was wrong. Thank you for pointing that out.

This crate has discarded the use of mem::discriminant.

[Hoverbear]

Re ba02f3a :)

[Hoverbear]

Thanks for the rapid fix, @magiclen !

[eddyb]

Thanks! I'm closing this issue, since this seems resolved now.