ICE matching on a non-[u8] const array

[Nadrieril]

This produces an ICE (playground link):

fn main() {
    const CONST: &[(); 1] = &[()];
    match &[()] {
        &[()] => {}
        CONST => {}
    }
}

It points to this assert firing:

rust/src/librustc_mir/hair/pattern/_match.rs

Lines 1739 to 1745 in d801458

	let data: &[u8] = match (const_val_val, &const_val.ty.kind) {
	(ConstValue::ByRef { offset, alloc, .. }, ty::Array(t, n)) => {
	assert_eq!(*t, tcx.types.u8);
	let n = n.eval_usize(tcx, param_env);
	let ptr = Pointer::new(AllocId(0), offset);
	alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap()
	}

This assert assumes that if we match on a &[T; n] with a const, then T must be u8. I'm not sure where this assumption comes from (I'd guess byte-array patterns), but it's clearly wrong. Note that this also fails with any other non-u8 type here instead of ().
Note that this does not fail when the arrays are not behind references, or if we use slices instead of arrays.

EDIT: removed unnecessary slice_patterns feature gate

[jonas-schievink]

The feature gate isn't needed to trigger the ICE

[Nadrieril]

Oh, well spotted. And it appears that fails on stable too then

[Mark-Simulacrum]

cc @pnkfelix as it's possible this is related to your structural match work I guess?

[hellow554]

Funny enough: this doesn't ICE

const CONST: [(); 1] = [()];
match &[()] {
    &[()] => {}
    &CONST => {}
}

but instead shows the correct "unreachable" warning

[Centril]

Reduced:

fn main() {
    const CONST: &[(); 0] = &[];
    match &[] {
        &[] => {}
        CONST => {}
    }
}

[pnkfelix]

This assert assumes that if we match on a &[T; n] with a const, then T must be u8

What the heck?!?

[pnkfelix]

triage: P-high. Removing nomination. Self-assigning.

[oli-obk]

That's a leftover from matching on byte strings, which used to be one of the few constants one could actually match on.

[Nadrieril]

I think it's one more of theses cases where we don't know how to handle ConstValue::ByRef properly. slice_pat_covered_by_const shouldn't even exist: specialization should be able to specialize consts by unpacking a pattern into its subpatterns, like we specialize everything else.

[pnkfelix]

I've figured out a way to eliminate the ICE, but I do not think my code has the desired semantics. That is, I would imagine we would want this to be accepted:

const CONST: &[(); 1] = &[()];
fn main() {
    match &[()] {
        ::CONST => {}
    }
}

but my current draft code rejects the above as non-exhaustive.

On the other hand, that does match the current behavior of the compiler, for better or for worse.

Update: Or maybe I've forgotten the desirata for how match usefulness and consts are supposed to interact...?

[pnkfelix]

Reduced:

fn main() {
    const CONST: &[(); 0] = &[];
    match &[] {
        &[] => {}
        CONST => {}
    }
}

This was not a stable reduction; on the current nightly, the above length-zero ZST array is accepted with no ICE, while the original length-one ZST array still causes an ICE.

[pnkfelix]

Yet another fun variation on the test:

fn main() {
    const CONST: &[Option<()>; 1] = &[Some(())];
    match &[Some(())] {
        &[None] => {}
        CONST => {}
        &[Some(())] => {}
    }
}

yields:

error: internal compiler error: src/librustc_mir/hair/pattern/_match.rs:2271: `constructor_covered_by_range` called with Variant(DefId(2:29423 ~ core[f952]::option[0]::Option[0]::Some[0]))

[oli-obk]

slice_pat_covered_by_const shouldn't even exist: specialization should be able to specialize consts by unpacking a pattern into its subpatterns, like we specialize everything else.

The reason it exists is that we don't want to unpack a multi megabyte byte string you may have gotten from include_bytes!. That would be super inefficient and likely just cause an out of memory abort.

The reason why I believe we should also do this optimization for other types is that you can easily transmute a multi megabyte byte string to another array type in a constant, and then you have the same problem all over again. (Or you init with a repeat expression).

I agree that it would be nicer and less fragilr if we just unpacked everything.

[oli-obk]

Btw, #67192 addresses a few of the ICEs around this

[Alexendoo]

Since #76376 the original and reduced versions no longer ICE, but

fn main() {
    const CONST: &[Option<()>; 1] = &[Some(())];
    match &[Some(())] {
        &[None] => {}
        CONST => {}
        &[Some(())] => {}
    }
}

still does

[Alexendoo]

The above no longer ICEs after #70743, could do with a test