Unreachable code warning for std::unreachable!

44279dc
Opened by Arun Kulshreshtha at 2020-12-04 16:23:43

Presently, rustc will emit a warning when it detects unreachable code. Ordinarily, this is great, but in the case of std::unreachable!, it isn't particularly helpful.

For example, consider the following code:

#![deny(unreachable_code)]

fn main() {
    return;
    
    unreachable!();
}

(Playground link)

This results in:

error: unreachable statement
 --> src/main.rs:6:5
  |
6 |     unreachable!();
  |     ^^^^^^^^^^^^^^^
  |
note: lint level defined here
 --> src/main.rs:1:9
  |
1 | #![deny(unreachable_code)]
  |         ^^^^^^^^^^^^^^^^
  = note: this error originates in a macro outside of the current crate

In this case the warning isn't particularly helpful given that I've explicitly marked the code as unreachable. It seems like it would be reasonable to avoid emitting a warning in this case.

In particular, this is problematic because #![deny(unreachable_code)] is implied by #![deny(warnings)]. I'm working in a large codebase that makes heavy use of #![deny(warnings)]. As a result, I'm not able to use std::unreachable!() without explicitly allowing unreachable code, which isn't desirable.

Oftentimes I'd like to use std::unreachable!() in places where unreachability is statically provable, but not intuitively obvious to a human reading the code. For now I've settled on just using comments instead.

  1. Having an unreachable!() call in places which are statically guaranteed to be unreachable is misleading IMHO. Similarly, making a binding mut when not necessary is misleading even when it's not self-evident to every programmer that the variable is only assigned once. While in both cases a reader might be surprised by the lack of unreachable/mut, the fact that the compiler accepts the code without it is a useful clue that it isn't actually required.

    Hanna Kruppe at 2017-12-04 21:52:29

  2. I don't think unreachable!() and spurious mut are analogous. mut suggests the intention to mutate something, and forewarns the reader, so having a spurious mut is a spurious warning.

    It's more like an assert!() which can be statically proved to always pass - it's always intended as a guard to maintain an invariant in the face of future code changes. It would be rude of the compiler to complain that it's always true - it should just quietly optimize the whole thing away.

    An unreachable!() statement is an assertion about a control flow invariant. Sure we can prove now that it will never be reached, but it also means that it should remain unreachable after code changes. We should be able to leave an unreachable!() statement so that if that block ever becomes dynamically reachable, we can know about it.

    Jeremy Fitzhardinge at 2017-12-05 21:52:10

  3. I just bumped into this issue myself.

    As jsgf says it's good to have a way to ensure and document an invariant, however, in this case I think we could and should go further: A proven_unreachable() macro that emits a warning if, and only if, rustc can't prove that the code indeed is unreachable. The current lint could then point the user there if it hits an unreachable unreachable!().

    ksf at 2020-12-04 16:23:43