Incorrect "unreachable pattern" warning with opaque constant

[Nadrieril]

I tried this code (playground link):

#[derive(PartialEq)]
struct Opaque(i32);

impl Eq for Opaque {}

const FOO: Opaque = Opaque(42);

fn foo() {
    match Opaque(0) {
        FOO => {},
        _ => {}
    }
}

This emits an "unreachable pattern" warning on the second branch of the match, even though it is clearly reachable. This gives the same warning for current nightly and beta, as well as stable 1.47.0 and 1.40.0.

This was uncovered while discussing #77390.

Meta

rustc --version --verbose:

rustc 1.47.0 (18bf6b4f0 2020-10-07)
binary: rustc
commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
commit-date: 2020-10-07
host: x86_64-unknown-linux-gnu
release: 1.47.0
LLVM version: 11.0

[Nadrieril]

@rustbot modify labels: +A-exhaustiveness-checking

[bugadani]

How is the second branch reachable?

[Nadrieril]

A value like Opaque(0) would reach it. I do realize that using FOO as the match scrutinee is confusing, I'll change that

[bugadani]

With this example, I get an error: error: to use a constant of type `Opaque` in a pattern, `Opaque` must be annotated with `#[derive(PartialEq, Eq)]

I believe the warning is raised because the compiler then assumes you use FOO as a catch-all name instead?

[Nadrieril]

Oh, that's an interesting hypothesis. That would mean some part of the code sees this as a const pattern (hence the error) and some other part as a variable binding (hence the warning). I would find that quite surprising honestly.

[Nadrieril]

Nah, the bit that emits the error is in rustc_mir_build/src/thir/pattern/const_to_pat.rs, and the one that emits the warning is rustc_mir_build/src/thir/pattern/check_match.rs. I'm mostly sure that those two take the same inputs and in particular would not reclassify the const as a variable binding

[bugadani]

But if I derive Eq instead of implementing it like you did, it's all good: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c3d83bafde050c32204321a21cf44689

[Nadrieril]

Yeah, something happens when the const is considered opaque which triggers this behavior. I'm not sure what exactly, @oli-obk do you know? Maybe returning Err after emitting the error in const-to-pat somehow transforms the pattern into a wildcard?

[Nadrieril]

Oh wait that's exactly what happens :D

rust/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs

Lines 337 to 348 in 6af9846

	let msg = format!(
	"to use a constant of type `{}` in a pattern, \
	`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
	path, path,
	);
	self.saw_const_match_error.set(true);
	if self.include_lint_checks {
	tcx.sess.span_err(span, &msg);
	} else {
	tcx.sess.delay_span_bug(span, &msg)
	}
	PatKind::Wild

@oli-obk I believe to solve that particular issue we'd need to return something else than PatKind::Wild here

[Nadrieril]

I gave it more thought, and the correct approach seems instead to not even start exhaustiveness checking if const_to_pat emits an unrecoverable error.

[oli-obk]

How about we introduce a PatKind::Error to make these hacky PatKind::Wild situations obvious? Then exhaustiveness can just bail out and assume an error has already been reported.

[Nadrieril]

There's already code in check_match to track errors and skip exhaustiveness, we could reuse that:

rust/compiler/rustc_mir_build/src/thir/pattern/check_match.rs

Line 182 in 2b76c48

// Bail out early if lowering failed.

I'll add that to my queue of things to do

[Nadrieril]

yeah ok there's a complicated path from const_to_pat to check_match. PatKind::Error sounds much more feasible.
EDIT: ugh maybe not, it's a mess

[Dylan-DPC]

The code no longer emits this warning for this case.

[Nadrieril]

It does, I just tested with the same playground link. But it also emits an error so maybe the warning doesn't matter too much. I just think it's a bit confusing.

Full output:

error: to use a constant of type `Opaque` in a pattern, `Opaque` must be annotated with `#[derive(PartialEq, Eq)]`
  --> src/lib.rs:10:9
   |
10 |         FOO => {},
   |         ^^^

warning: unreachable pattern
  --> src/lib.rs:11:9
   |
10 |         FOO => {},
   |         --- matches any value
11 |         _ => {}
   |         ^ unreachable pattern
   |
   = note: `#[warn(unreachable_patterns)]` on by default