unreachable pattern not warned when used in sufficiently complex macro / procmacro hack

[drahnr]

I tried this code:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=49f774bfbf796f141b31e716fba52820

with expansion:

https://gist.github.com/bkchr/92b754ec82efcc115637ab5533e79ff4

use futures::{future, select}; // 0.3.6

pub enum Foo {
    Bar,
    Baz,
}

/* this function does produce warnings
fn match_foo(foo: Foo) -> i32 {
    match foo {
       x => 1,
        _ => unreachable!("why you do this rust?"),
    }
}
*/

pub async fn foo(foo: Foo) {
    let mut a = future::ready(4);
    
    use Foo::Bar;
    let res = select! {
        // but this doesn't        
        a_res = a => match foo {
            Bar => 1,
            _ => 4,
            _ => unreachable!("why you do this rust?"),
        },
    };

    assert_eq!(res, 4, "oh");
}


fn main() {
    futures::executor::block_on(async move {
        foo(Foo::Baz).await
    });
}

I expected to see this happen:

Be warned about the unreachable pattern / enum variant.

Instead, this happened:

No warning was displayed despite an unreachable pattern being present.

Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.79s
     Running `target/debug/playground`
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `4`: oh', src/main.rs:29:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

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

Credit for example @ordian , initial finding by @coriolinus

[tesuji]

Can confirm the warning[E0170] doesn't trigger for the code above.
The code could be simplify more by remove unused assert:

use futures::{future, select}; // 0.3.6

pub enum Foo {
    Bar,
    Baz,
}

/* this function does produce warnings
fn match_foo(foo: Foo) -> i32 {
    match foo {
       Bar => 1,
        _ => unreachable!("why you do this rust?"),
    }
}
*/

pub async fn asdf(foo: Foo) {
    let mut a = future::ready(4);

    select! {
        // but this doesn't
        a_res = a => match foo {
            Bar => 1,
            _ => 4,
            // _ => unreachable!("why you do this rust?"),
        },
    };
}

fn main() {
    futures::executor::block_on(asdf(Foo::Baz));
}

@rustbot prioritize

[camelid]

@drahnr By the way, not sure if you realized this but matching on Bar treats Bar as a variable not the enum variant Foo::Bar.

[Aaron1011]

This appears to be due to the select! macro using proc-macro-hack. As the name would suggest, proc-macro-hack does a number of hacky things, which appears to include sending input tokens through a generated macro_rules! macro. This applies a mark to the token spans, causing the lint to be suppressed by this check:

rust/compiler/rustc_middle/src/lint.rs

Lines 245 to 260 in 7f58716

	if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
	// Any suggestions made here are likely to be incorrect, so anything we
	// emit shouldn't be automatically fixed by rustfix.
	err.allow_suggestions(false);
	// If this is a future incompatible lint it'll become a hard error, so
	// we have to emit *something*. Also, if this lint occurs in the
	// expansion of a macro from an external crate, allow individual lints
	// to opt-out from being reported.
	if future_incompatible.is_none() && !lint.report_in_external_macro {
	err.cancel();
	// Don't continue further, since we don't want to have
	// `diag_span_note_once` called for a diagnostic that isn't emitted.
	return;
	}
	}

This should be fixed when dtolnay/proc-macro-hack#62 is implemented. I don't think there's anything that can be done on the rustc side.

[drahnr]

@drahnr By the way, not sure if you realized this but matching on Bar treats Bar as a variable not the enum variant Foo::Bar.

I think that was a result of minimization removing a use Foo::Bar; from the scope. Doesn't really change much for the repro.

[apiraino]

Assigning P-low as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[ordian]

This should be fixed when dtolnay/proc-macro-hack#62 is implemented. I don't think there's anything that can be done on the rustc side.

According to this comment, this should have been fixed by dtolnay/proc-macro-hack#66, which was released in 0.5.19.
But with

❯ cargo update -p proc-macro-hack 
    Updating crates.io index
    Updating proc-macro-hack v0.5.18 -> v0.5.19

I am still not getting the warnings.

[Aaron1011]

It looks like that change requires explicit opt-in via #[proc_macro_hack(only_hack_old_rustc)] in the proc macro crate.

[Aaron1011]

This now produces a warning if you change the Cargo.toml dependency to futures = { git = "https://github.com/rust-lang/futures-rs" }:

warning: unreachable pattern
  --> src/main.rs:26:13
   |
26 |             _ => unreachable!("why you do this rust?"),
   |             ^
   |
   = note: `#[warn(unreachable_patterns)]` on by default

warning: unused variable: `a_res`
  --> src/main.rs:23:9
   |
23 |         a_res = a => match foo {
   |         ^^^^^ help: if this is intentional, prefix it with an underscore: `_a_res`
   |
   = note: `#[warn(unused_variables)]` on by default