Unreachable expression in guard-0.3.1, Rust 1.15

[brson]

https://github.com/durka/guard 8f9753302f549826800820dc4a34ad8d01dccd66

Not on stable.

101 brian@ip-10-145-43-250:~/dev/guard⟫ rustc +beta -Vv
rustc 1.15.0-beta.3 (a035041ba 2017-01-07)
binary: rustc
commit-hash: a035041ba450ce3061d78a2bdb9c446eb5321d0d
commit-date: 2017-01-07
host: x86_64-unknown-linux-gnu
release: 1.15.0-beta.3
LLVM version: 3.9

Many errors like this:

error: unreachable expression
   --> src/lib.rs:111:56
    |
111 |                                                        match _x {}
    |                                                        ^^^^^^^^^^^
...
467 |         guard!(let Some((a, b)) else { panic!() } = if false { opt } else { opt }); println!("{} {}", a, b);
    |         --------------------------------------------------------------------------- in this macro invocation

error: unreachable expression
   --> src/lib.rs:105:60
    |
105 |                                                            match _x {}
    |                                                            ^^^^^^^^^^^
...
468 |         guard!(let Some((a, b)) if b > 0 else { panic!() } = opt);                  println!("{} {}", a, b);
    |         ---------------------------------------------------------- in this macro invocation

error: unreachable expression
   --> src/lib.rs:111:56
    |
111 |                                                        match _x {}
    |                                                        ^^^^^^^^^^^
...
468 |         guard!(let Some((a, b)) if b > 0 else { panic!() } = opt);                  println!("{} {}", a, b);
    |         ---------------------------------------------------------- in this macro invocation

error: aborting due to 52 previous errors

cc @durka

[durka]

The root cause is that this code

    let x = if true {
        42
    } else {
        [panic!()]
    };

typechecks on beta and nightly but not stable. In my macro I am trying to make sure the code before match _x {} diverges. In stable, I have to put in the empty match to satisfy the typechecker, but in newer compilers the diverging code itself is enough. I believe this is due to improvements by @canndrew. Previously this was handled by the crate's "nightly" feature, but now that it has reached beta it is a problem.

cc @eddyb

[durka]

That being said, I brought this upon myself with #![cfg_attr(test, deny(warnings))]. Allowing unreachable-code warnings fixes the build at the price of a few warnings.

[eddyb]

@nikomatsakis Is there a good reason for denying/allowing that code?

[durka]

@eddyb remember this?

[eddyb]

Yes, and I relaxed it afterwards by doing the checking in typeck, in a principled fashion.
That said, I thought I limited unreachable expressions to !.

[nikomatsakis]

@eddyb I am somewhat surprised to learn that this type-checks, although I can imagine how it might come about.

[canndrew]

Well, part of the promise of the ! was RFC was always better dead code detection. We don't make any promises about not breaking code with deny(warnings).

Is there a good reason for denying/allowing that code?

The fact that we can allow it seems like a good reason to allow it.

[durka]

We don't make any promises about not breaking code with deny(warnings).

Yes, I agree. It's annoying that there's no way to write code that is warning-free on all channels but it doesn't count as a regression.

[arielb1]

@durka

#[allow(unknown_lints, unreachable_patterns)] is your friend.

[eddyb]

Oh the rule is that an expected type can't differ unless it's !. But there's no expected type here.
And the else block ends up having an diverging variable as its type. Which is needed for compatibility.
At least I seem to recall we can't avoid that (and if we do it, this code compiles).

[nikomatsakis]

To be clear, while I was somewhat surprised, I also think it's ok and makes sense. We've traditionally not considered regressions due to lints triggering as being "real regressions" -- since we cap lints on dependencies they don't affect anyone downstream, to be sure. I think that same policy should apply here, hence no action needed (as long as we're satisfied with how the code is behaving).