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

Arms permitted when matching on uninhabited types #55123

Open
varkor opened this issue Oct 16, 2018 · 8 comments
Open

Arms permitted when matching on uninhabited types #55123

varkor opened this issue Oct 16, 2018 · 8 comments
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@varkor
Copy link
Member

varkor commented Oct 16, 2018

There seem to be two issues here:

pub enum Void {}

pub fn foo(x: Void) {
  match x {
    _ => {} // This arm shouldn't be permitted.
  };
  let _ = (); // This should be warned as unreachable, but isn't.
}

On the other hand, the following code does warn:

match () {
	() => {} // okay
	_ => {} // unreachable pattern
}
@varkor
Copy link
Member Author

varkor commented Oct 16, 2018

The second issue is a direct result of the first, where "number of arms" is treated as a heuristic for divergence, rather than the type of the expression (this could be an issue in the future with regards to an explicit ! pattern):
https://github.com/rust-lang/rust/blob/3b88a54ebcf1d704fddd464e9d5225530cfb475b/src/librustc_typeck/check/_match.rs#L611-L615

@estebank estebank added the A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. label Oct 16, 2018
@varkor
Copy link
Member Author

varkor commented Oct 16, 2018

This is due to:

fn is_uninhabited(&self, ty: Ty<'tcx>) -> bool {
if self.tcx.features().exhaustive_patterns {
self.tcx.is_ty_uninhabited_from(self.module, ty)
} else {
false
}
}
I think the solution here is to stabilise exhaustive_patterns.

@jonas-schievink jonas-schievink added 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. C-bug Category: This is a bug. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels May 22, 2020
@Nadrieril
Copy link
Member

I don't think exhaustive_patterns would fix the second issue: it would detect the arm as unreachable, but afaik that bit of match checking does not count towards counting the match as divergent.

bors added a commit to rust-lang-ci/rust that referenced this issue Nov 18, 2020
Handle empty matches cleanly in exhaustiveness checking

This removes the special-casing of empty matches that was done in `check_match`. This fixes most of rust-lang#55123.
Somewhat unrelatedly, I also made `_match.rs` more self-contained, because I think it's cleaner.

r? `@varkor`
`@rustbot` modify labels: +A-exhaustiveness-checking
@Nadrieril
Copy link
Member

The second issue got fixed in #78995. I'm wondering about the first one: is matching on an empty enum very different from getting a value of an empty type as input? My hunch would be that if we lint any code after match x {} to be unreachable, why not lint the whole function body after fn foo(x: Void) as unreachable? Is there a particular reason that either is more natural/useful for an user?

pub enum Void {}

pub fn foo(x: Void) {
  let _ = (); // Frankly this also could be warned as unreachable.
  match x {};
  let _ = (); // This could be warned as unreachable, but isn't.
}

@varkor
Copy link
Member Author

varkor commented Nov 22, 2020

I agree that these are essentially the same issue. I actually submitted a PR a couple of years ago to address this, by making the uninhabitedness checking more eager (see #47291), but parts of it were split out to err on the side of being more conservative. I can't quite remember the status: probably in order to make these changes, there would need to be a discussion into how eagerly one wants to mark unreachable code based on uninhabited types. Probably this issue can be closed, and potentially a new one opened to explore reachability based on inhabitedness.

@camelid
Copy link
Member

camelid commented Mar 20, 2021

It looks like the let _xyz line isn't linted as unreachable because MIR for it is not generated (even in debug mode).

pub enum Void {}

pub fn foo(x: Void) {
  match x {
    _ => {} // This arm shouldn't be permitted.
  };
  let _xyz = (); // This should be warned as unreachable, but isn't.
}

@EFanZh
Copy link
Contributor

EFanZh commented Dec 27, 2022

Not sure if related, but I encountered two surprising error:

enum Empty {}

fn foo(x: &Empty) {
    match x {} // ERROR: missing match arm.
}
enum Empty1 {}

enum Empty2 {}

fn foo(x: (Empty1, Empty2)) {
    match x {} // ERROR: missing match arm.
}

I have to add a _ => {} arm for the code to compile.

@Nadrieril
Copy link
Member

Nadrieril commented Jan 4, 2023

@EFanZh this is because in an unsafe block, you could match on a &Empty or a (Empty1, u8) without causing UB (if you're careful). Because of that we can't yet allow matching without any arms on those types.
We want to make this possible though, but that'll take some work. More details here: #51085

@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants