Non-exhaustive patterns error when matching reference to empty enum

[HactarCE]

matching a value of an empty enum is valid, however matching a reference to a value of an empty enum produces E0004: non-exhaustive patterns.

I tried this code (playground link):

enum A {}

fn f(a: &A) -> usize {
    match a {}
}

I expected to see this happen: The code should compile with no issue, and the function should never actually execute because A has no variants and thus cannot be instantiated.

Instead, this happened:

error[E0004]: non-exhaustive patterns: type `&A` is non-empty
 --> src/lib.rs:4:11
  |
1 | enum A {}
  | --------- `A` defined here
...
4 |     match a {}
  |           ^
  |
  = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
  = note: the matched value is of type `&A`

Meta

The bug exists on 1.47.0 and 1.49.0-nightly (2020-10-18 b1496c6) running on the playground. I first found this bug on 1.46.0.

rustc --version --verbose:

rustc 1.46.0 (04488afe3 2020-08-24)
binary: rustc
commit-hash: 04488afe34512aa4c33566eb16d8c912a3ae04f9
commit-date: 2020-08-24
host: x86_64-pc-windows-msvc
release: 1.46.0
LLVM version: 10.0

[jonas-schievink]

This is currently working as intended, but maybe we should say so in the diagnostic

[Nadrieril]

Why is this the intended behavior?
I find it notably surprising because I can usually replace match *x {...} with match x {...}, and the commiler takes care to insert (de)references magically. This would be an exception.

[jonas-schievink]

I don't recall, but you might be able to find some of the old discussions

[camelid]

I was just reading some discussion on the exhaustive_patterns issue: IIUC, the reason that &Void (where Void is an uninhabited type) is considered inhabited is because unsafe code can construct an &-reference to an uninhabited type (e.g. with ptr::null() or mem::transmute()). I don't know enough to be sure though.

[camelid]

Though unsafe code can also construct an uninhabited without using references; e.g.:

use std::mem;

#[derive(Debug)]
enum Void {}

fn main() {
    println!("start");

    let x: Void = unsafe { mem::transmute(()) };
    println!("made Void value");

    println!("{:?}", x);
    println!("printed Void value");

    println!("end");
}

(Playground)

Output:

start
made Void value

Errors:

   Compiling playground v0.0.1 (/playground)
warning: unused variable: `x`
 --> src/main.rs:9:9
  |
9 |     let x: Void = unsafe { mem::transmute(()) };
  |         ^ help: if this is intentional, prefix it with an underscore: `_x`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: the type `Void` does not permit zero-initialization
 --> src/main.rs:9:28
  |
9 |     let x: Void = unsafe { mem::transmute(()) };
  |                            ^^^^^^^^^^^^^^^^^^
  |                            |
  |                            this code causes undefined behavior when executed
  |                            help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
  |
  = note: `#[warn(invalid_value)]` on by default
  = note: enums with no variants have no valid value

warning: 2 warnings emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.63s
     Running `target/debug/playground`
timeout: the monitored command dumped core
/playground/tools/entrypoint.sh: line 11:     7 Illegal instruction     timeout --signal=KILL ${timeout} "$@"

---

So maybe what I said in #78123 (comment) is not correct?

[Nadrieril]

I think the difference is that creating a value of type Void like you did is Undefined Behavior, but creating a &Void isn't. Try running your code using the MIRI tool (available on the playground under Tools), it usually tells you when you do UB

[camelid]

Indeed, transmuting to &Void didn't make Miri report UB (though attempting to print the value did) but transmuting to Void did.

[Nadrieril]

Nice. Indeed dereferencing a reference to unintialized memory is also UB.

Instructions to fix this:

• add a test case named after this issue in the src/test/ui/pattern/usefulness folder
• find where the error message is emitted (somewhere in rustc_mir_build/.../check_match.rs)
• check when the type is ty::Ref and the subtype is uninhabited (which you can test using cx.tcx.is_ty_uninhabited_from(cx.module, sub_ty, cx.param_env))
• in the above case, add a note to the error message, saying something like "references are always considered inhabited". I'm not sure if we want to mention why.
• maybe add some related positive and negative tests to make sure we only emit the new note when relevant
• maybe test and handle cases like &&&Void as well?

@rustbot modify labels: +E-easy +E-mentor

[HactarCE]

I think the difference is that creating a value of type Void like you did is Undefined Behavior, but creating a &Void isn't. Try running your code using the MIRI tool (available on the playground under Tools), it usually tells you when you do UB

On the contrary, constructing a &T which does not reference a valid T is instant UB according to the Rustonomicon. A value of type &Void is just as invalid as a value of type Void.

[Nadrieril]

Oh shoot, I understood the opposite. Then I've misunderstood what @nikomatsakis says in their never patterns blog post:

Unfortunately, when it comes to unsafe code, there is a general desire to treat any reference &T “with suspicion”. Specifically, we don’t want to make the assumption that this is a reference to valid, initialized memory unless we see an explicit dereference by the user.

Maybe that post is old and things have changed since? Or maybe unintialized memory does not count as an invalid value?

[RalfJung]

The rules for validity of reference are not finally decided yet, but still being discussed at rust-lang/unsafe-code-guidelines#77. To not preempt the conclusion of this discussion, the docs declare as much UB as is possible -- we can always relax UB later, but introducing more UB is a breaking change.

[Nadrieril]

Ok, I understand that if we want to have the option to relax this later, then we can't have the compiler rely on it. So we must treat &! as inhabited. Otherwise we would accept the following but it would become unsound if we relax the UB:

fn foo(x: Option<&!>) -> u64 {
	match x {
		None => 42,
	}
}

So exhaustive_patterns should stay as it currently is and my instructions above are still the way to go.

[GroteGnoom]

@rustbot claim

[Nadrieril]

Thanks for taking this on @GroteGnoom ! Feel free to ask me questions here or on zulip.

[GroteGnoom]

Thanks for the warm welcome :)

I do have questions, but I already made a preliminary commit to check if I'm going in the right direction:
GroteGnoom@998bf0a (edited because I missed a file)

Is that ok, or is it better if I ask questions before committing, or just make a PR and adapt it?

[Nadrieril]

That commit looks like what we want yes :) Go ahead and make a PR, it's easier to comment on specific bits of the code

[jonhoo]

For those who (like me) find this after googling "references are always considered inhabited" (the compiler's help message), the current discussion on this topic is happening in rust-lang/unsafe-code-guidelines#413.

[dhedey]

I've added a comment here: rust-lang/unsafe-code-guidelines#413 (comment)

Which suggests that we consider making it UB to construct such a reference, so that this macro code doesn't confusingly start breaking for empty enums:

impl MyGeneratedEnum {
    fn get_x(&self) {
        match self {
            $(
                Self::$variant(v) => v.get_x(),
            )*
        }
    }
}