Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-exhaustive patterns error when matching reference to empty enum #78123

Closed
HactarCE opened this issue Oct 19, 2020 · 17 comments · Fixed by #80651
Closed

Non-exhaustive patterns error when matching reference to empty enum #78123

HactarCE opened this issue Oct 19, 2020 · 17 comments · Fixed by #80651
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@HactarCE
Copy link
Contributor

HactarCE commented Oct 19, 2020

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
@HactarCE HactarCE added the C-bug Category: This is a bug. label Oct 19, 2020
@jonas-schievink
Copy link
Contributor

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

@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. labels Oct 19, 2020
@Nadrieril
Copy link
Member

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
Copy link
Contributor

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

@camelid
Copy link
Member

camelid commented Dec 2, 2020

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
Copy link
Member

camelid commented Dec 2, 2020

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
Copy link
Member

Nadrieril commented Dec 2, 2020

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
Copy link
Member

camelid commented Dec 2, 2020

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

@Nadrieril
Copy link
Member

Nadrieril commented Dec 2, 2020

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

@rustbot rustbot added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Dec 2, 2020
@HactarCE
Copy link
Contributor Author

HactarCE commented Dec 2, 2020

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
Copy link
Member

Nadrieril commented Dec 3, 2020

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
Copy link
Member

RalfJung commented Dec 3, 2020

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
Copy link
Member

Nadrieril commented Dec 3, 2020

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
Copy link

@rustbot claim

@Nadrieril
Copy link
Member

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

@GroteGnoom
Copy link

GroteGnoom commented Jan 3, 2021

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
Copy link
Member

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

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2021
Add note to non-exhaustive match on reference to empty

Rust prints "type `&A` is non-empty" even is A is empty.
This is the intended behavior, but can be confusing.
This commit adds a note to non-exhaustive pattern errors if they are a
reference to something uninhabited.

I did not add tests to check that the note is not shown for
non-references or inhabited references, because this is already done
in other tests.

Maybe the added test is superfluous, because
`always-inhabited-union-ref` already checks for this case.

This does not handle &&Void or &&&void etc. I could add those as special
cases as well and ignore people who need quadruple
references.

Fixes rust-lang#78123
@bors bors closed this as completed in 998bf0a Jan 4, 2021
jocelyn-stericker added a commit to jocelyn-stericker/rubato that referenced this issue Apr 25, 2021
Rust only allows an empty match statement in code that will not be run,
that is when matching something that is not inhabited.

References to enums are always considered inhabited, because it may
avoid some undefined behaviour in unsafe code. See
rust-lang/rust#78123

The solution in this PR is to match the enum rather than the reference
to the enum.

I also added CI for wasm32-unknown-unknown to combat regression.
jocelyn-stericker added a commit to jocelyn-stericker/rubato that referenced this issue Apr 25, 2021
Rust only allows an empty match statement in code that will not be run,
that is when matching something that is not inhabited.

References to enums are always considered inhabited, because it may
avoid some undefined behaviour in unsafe code. See
rust-lang/rust#78123

The solution in this PR is to match the enum rather than the reference
to the enum.

I also added CI for wasm32-unknown-unknown to combat regression.
onkoe added a commit to onkoe/pisserror that referenced this issue Jul 8, 2024
if you use references, the compiler forces you to match on NOTHING: rust-lang/rust#78123

so yeah this fixes #5 woo hoo!
@jonhoo
Copy link
Contributor

jonhoo commented Aug 28, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants