very slow derived PartialEq/PartialOrd implementation of enum

[oli-obk]

The following code goes into an infinite loop or at least takes more than 10 seconds without optimizations. With optimizations it runs and the assertion fails. If anything but 0 and 0x8000_0000_0000_0000 are chosen for the discriminants, the code finishes instantly even in debug mode. Playground

#[derive(PartialEq, PartialOrd)]
enum Eu64 {
    Au64 = 0,
    Bu64 = 0x8000_0000_0000_0000
}

fn main() {
    assert!(Eu64::Bu64 > Eu64::Au64);
}

There's something odd going on anyway, as the discriminant is inferred to be isize instead of u64 (This unit test seems to suggest it should be u64)

cc #24290 @pnkfelix

[durka]

Expanded:

enum Eu64 { Au64 = 0, Bu64 = 9223372036854775808, }

impl ::std::cmp::PartialOrd for Eu64 {
    #[inline]
    fn partial_cmp(&self, __arg_0: &Eu64)
     -> ::std::option::Option<::std::cmp::Ordering> {
        /* elided */
    }
    #[inline]
    fn lt(&self, __arg_0: &Eu64) -> bool {
        /* elided */
    }
    #[inline]
    fn le(&self, __arg_0: &Eu64) -> bool {
        /* elided */
    }
    #[inline]
    fn gt(&self, __arg_0: &Eu64) -> bool {
        {
            let __self_vi =
                unsafe { ::std::intrinsics::discriminant_value(&*self) } as
                    i32;
            let __arg_1_vi =
                unsafe { ::std::intrinsics::discriminant_value(&*__arg_0) } as
                    i32;
            if true && __self_vi == __arg_1_vi {
                match (&*self, &*__arg_0) {
                    (&Eu64::Au64, &Eu64::Au64) => false,
                    (&Eu64::Bu64, &Eu64::Bu64) => false,
                    _ => unsafe { ::std::intrinsics::unreachable() }
                }
            } else { __self_vi.gt(&__arg_1_vi) }
        }
    }
    #[inline]
    fn ge(&self, __arg_0: &Eu64) -> bool {
        /* elided */
    }
}

impl ::std::cmp::PartialEq for Eu64 {
    /* elided */
}

Red flags:

• the discriminant is casted to i32, rather than isize or even u64!
• the logic is confusing. If the (truncated) discriminants are equal, it ignores them and checks the variants. The generated code would presumably make more sense if there were data in the variants.

Other than that, I don't see what's causing the slowdown. (edit: was wrong) The full expanded code exhibits the same behavior.

With the cast to i32 in mind, the assertion failure is expected. It looks like the deriving code looks for a #[repr] attribute and otherwise defaults to i32. This makes some sense since derivings have only syntactic information... but not a lot of sense.

(edit again) It has to cast in case the repr is signed, because otherwise the comparison will be backwards. But I don't see how to get it right, because at deriving time there isn't enough information to infer the repr (might have to look up consts, etc). Maybe if we redesign the discriminant_value intrinsic like this:

trait DiscriminantValue<T> {
    type Discriminant: Eq + Ord;
    fn value(&self) -> Self::Discriminant;
}

[durka]

In a debugger, it is always stuck on the line (of the expanded source):

80                          (&Eu64::Bu64, &Eu64::Bu64) => false,

Disassembly shows it stuck on this instruction:

=> 0x0000555555558b39 <+169>:   jmp    0x555555558b39 <oli_exp::Eu64.::std::cmp::PartialOrd::gt+169>

Well then... I don't know x86 assembly but that looks like an infinite loop to me! I dumped the assembly from rustc and found this gem:

.LBB7_6:
.LBB7_7:
        jmp     .LBB7_6

So it seems there is a codegen bug in addition to the discriminant type issue.

[durka]

Oh, suddenly it makes sense. LBB7_6 (or 7) must correspond to the catchall arm of the match, which is unreachable. It's unreachable because the discriminants compared equal, but we already know that's a false positive caused by integer truncation. So it triggers undefined behavior.

[durka]

Talked with @talchas on IRC. Turns out if you give a value for an enum variant, it has to be the same type as the given #[repr], or isize if no attribute. Therefore, perhaps the only change necessary is to change the default cast from i32 to isize in the derived code.

[talchas]

Obnoxiously

enum Foo { A = 0, X = 0x8000_0000_0000_0000 }
fn main() {
    println!("{} {}", Foo::X as u64, unsafe { std::intrinsics::discriminant_value(&Foo::X) });
}

both compiles and prints "0 9223372036854775808" on 32-bit (same with as i64 and equivalents with using 0x1000... instead of 0x8000). And of course Foo is 8 bytes wide (it's even tested!).

So values that don't fit in an i32 give silently crazy behavior on 32-bit in other places.