Remove a buggy cast from CheckEnums

[saethlin]

This is a fix for #159433.

Based on the MIR that @theemathas reported:

bb2: {
    _1 = Option::<NonZero<i32>>::Some(move _2);
    _4 = copy _1 as u32 (Transmute);
    _5 = copy _4 as i32 (IntToInt);
    _6 = copy _5 as u128 (IntToInt);
    _7 = Sub(copy _6, const 0_u128);
    _8 = Le(copy _7, const 4294967295_u128);
    assert(copy _8, "trying to construct an enum from an invalid value {}", copy _6) -> [success: bb3, unwind unreachable];
}

I think the problem here is that on the full-size discriminant path we just have an unnecessary cast. We transmuted to u32 to read the discriminant value, then we should just cast directly to u128 to do the comparison. There's no need to get sidetracked by the fact that there is an i32 field.

We do still need the extra cast along our field-reading path that uses [MaybeUninit<u8>; N], so the fix here is to move the code into that arm of the if.

[lukaslueg]

The following reproducer is a variant that still breaks for me with the same error after applying this PR. Notice that Payload is larger than a NonZeroI32; as far as I understand this puts us into the // The discriminant is less wide than the operand branch, while the original reproducer is in the else branch.

use std::{mem, num::NonZeroI32};

#[repr(C)]
struct Payload {
    pad: u8,
    value: NonZeroI32,
}

enum Thing {
    Value(Payload),
    Empty,
}

fn main() {
    let value = Thing::Value(Payload {
        pad: 0,
        value: NonZeroI32::new(-1).unwrap(),
    });
    let _ = unsafe { mem::transmute::<Thing, Thing>(value) };
}

[scottmcm]

It's not obvious to me that this should be going to u128 ever?

Seems like it should be emitting u32 - u32 <= u32 for a i32 or u32 scalar, like the assume does in transmute codegen:

rust/compiler/rustc_codegen_ssa/src/traits/builder.rs

Lines 270 to 289 in 421875f

	/// Emits an `assume` that the integer value `imm` of type `ty` is contained in `range`.
	///
	/// This *always* emits the assumption, so you probably want to check the
	/// optimization level and `Scalar::is_always_valid` before calling it.
	fn assume_integer_range(&mut self, imm: Self::Value, ty: Self::Type, range: WrappingRange) {
	let WrappingRange { start, end } = range;
	// Perhaps one day we'll be able to use assume operand bundles for this,
	// but for now this encoding with a single icmp+assume is best per
	// <https://github.com/llvm/llvm-project/issues/123278#issuecomment-2597440158>
	let shifted = if start == 0 {
	imm
	} else {
	let low = self.const_uint_big(ty, start);
	self.sub(imm, low)
	};
	let width = self.const_uint_big(ty, u128::wrapping_sub(end, start));
	let cmp = self.icmp(IntPredicate::IntULE, shifted, width);
	self.assume(cmp);
	}

[scottmcm]

suggestion: while these UI tests are good to have, I think we should have a mir-opt test showing and testing the MIR generated for various cases too.

There's a bunch of codegen-llvm tests around transmutes if you want inspiration of things to try.

View changes since the review

[saethlin]

It's not obvious to me that this should be going to u128 ever?

It's u128 so that we can have a monomorphic panic entrypoint that prints the invalid value.

[theemathas]

See also #143273, which fixes a similar-looking issue.