Incorrect "unreachable match arm" warning

[RalfJung]

The following code generates a warning:

fn main() {
    #[derive(Copy, Clone)]
    enum Void {}
    union Uninit<T: Copy> {
        value: T,
        uninit: (),
    }
    unsafe {
        let x: Uninit<Void> = Uninit { uninit: () };
        match x.value {
            _ => println!("hi from the void!"),
        }
    }
}

warning: unreachable pattern
  --> src/main.rs:12:13
   |
12 |             _ => println!("hi from the void!"),
   |             ^
   |
   = note: `#[warn(unreachable_patterns)]` on by default

This warning is wrong: running the code in Miri shows that this arm is reachable and there is no UB.

[RalfJung]

Here's another variant of the false warning:

fn main() {
    #[derive(Copy, Clone)]
    enum Void {}
    
    let ptr = 1 as *const Void;
    unsafe {
        match *ptr {
            _ => println!("hello from the Void"),
        }
    }
}

[RalfJung]

In fact the warning is actively harmful -- by removing the arm, UB is introduced into this well-defined code!

[RalfJung]

There was a now-deleted comment saying that the warnings appear even without the feature gate -- that seems right, the lint is actually already misfiring. But at least currently when people then try to remove the "unreachable" arm they get a build error.

Cc @Nadrieril @oli-obk (pattern match lint issue)

[Nadrieril]

But at least currently when people then try to remove the "unreachable" arm they get a build error.

They don't (playground)

[Nadrieril]

There's a special case in exhaustiveness checking to allow match x {} with x: Void. Specifically for ! and enums without variants. It's the only place we allow omitting arms for empty types outside of the exhaustive_patterns feature.

match x {} with x: Void is allowed on stable today, and is relied on; we can't remove that.

What could we do? Could we detect the unsafe block? Or detect pointer dereference/union field access?

[RalfJung]

They don't

Oh... I thought this needs the exhaustive_patterns feature. That seems bad, IMO we should not accept this code without some sort of explicit "never pattern".

[RalfJung]

Or detect pointer dereference/union field access?

That makes most sense to me.

[Nadrieril]

Let me check with you to be sure: in the following, the let y = .. is already UB, right?

unsafe {
    let x: Uninit<Void> = Uninit { uninit: () };
    let y = x.value;
    match y {
    }
}

and in here, the braces also would cause UB?

unsafe {
    let x: Uninit<Void> = Uninit { uninit: () };
    match {x.value} {
    }
}

So for the purposes of exhaustivenesss/unreachability I only need to check if the match scrutinee is literally a union field access expression or a pointer dereference expression? Anything else?

[RalfJung]

Let me check with you to be sure: in the following, the let y = .. is already UB, right?

Yes.

and in here, the braces also would cause UB?

I think braces mean we do a load from the place, create a new temporary place, and then match on that. If that's accurate then yes that would also be UB already at the {x.value}.

So for the purposes of exhaustivenesss/unreachability I only need to check if the match scrutinee is literally a union field access expression or a pointer dereference expression?

Yes.

Anything else?

I have some concerns around dereferencing references (not just raw pointers) as well. The reference says that references have to be recursively valid, but that's not actually the semantics I think we should end up with. But I haven't found a clear example yet so it's possible that this is fine. We should IMO start with the places where we clearly allow invalid values: raw pointers and unions.

[Nadrieril]

I have some concerns around dereferencing references (not just raw pointers) as well

Even exhaustive_patterns treats &! as inhabited, so at least there's no problem to fix here. For the purposes of this issue, raw pointers and unions seem to be the thing we care about