Rust: Nightly: unsafe code is allowed outside `unsafe` blocks when the code is unreachable

[Thiez]

I'm not sure if this is a feature or a bug, but the compiler behaves differently for the following program on stable, beta, and nightly:

fn main() {
    return;
    let _ = ();
    *(&true as *const bool);
}

On stable:

error[E0133]: dereference of raw pointer requires unsafe function or block
 --> src/main.rs:4:5
  |
4 |     *(&true as *const bool);
  |     ^^^^^^^^^^^^^^^^^^^^^^^ dereference of raw pointer

error: aborting due to previous error

On beta:

warning: unreachable statement
 --> src/main.rs:3:5
  |
3 |     let _ = ();
  |     ^^^^^^^^^^^
  |
  = note: #[warn(unreachable_code)] on by default

error[E0133]: dereference of raw pointer requires unsafe function or block
 --> src/main.rs:4:5
  |
4 |     *(&true as *const bool);
  |     ^^^^^^^^^^^^^^^^^^^^^^^ dereference of raw pointer

error: aborting due to previous error

On nightly:

warning: unreachable statement
 --> src/main.rs:3:5
  |
3 |     let _ = ();
  |     ^^^^^^^^^^^
  |
  = note: #[warn(unreachable_code)] on by default

Note that wrapping the final statement in an unsafe block will allow the program to compile on all three versions, but on nightly the compiler will complain:

warning: unnecessary `unsafe` block
 --> src/main.rs:4:5
  |
4 |     unsafe { *(&true as *const bool); }
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unnecessary `unsafe` block
  |
  = note: #[warn(unused_unsafe)] on by default

I suppose that the compiler is technically correct, because dead code ~tells no tales~ cannot cause memory unsafety, so no unsafe block is needed... but the current situation does seem like an accident, as the compiler does perform other checks on dead code (typechecking, borrow-checking, ...).

[hanna-kruppe]

Probably due to #44700, cc @arielb1

[nikomatsakis]

cc @rust-lang/lang as well

This is clearly a result of doing unsafe checking on MIR. My preference would be to continue issuing the error, but we have sometimes decided to let errors in unreachable code slide -- but that was primarily around the borrow check, i.e., checks that are very flow-sensitive. Effect check doesn't seem to fall into that category. Probably if we just perform the unsafe check before we remove dead code, it'd be fine? But @arielb1 can correct.

[nikomatsakis]

triage: P-high

We have to reach a decision of some kind.

[joshtriplett]

I think it's entirely wrong to fail to issue the warning, here. Even if code is unreachable, we shouldn't allow incorrect usage/non-usage of unsafe.

Among other things, that might also break code in if cfg!(...) blocks.

[Thiez]

I think the problem is not so much that the compiler dosen't mind unreachable unsafe code outside of unsafe blocks (although that is a bit silly), but that it gives "unnecessary unsafe block" warnings for correct unsafe{ ... } blocks that happen to be unreachable.

[arielb1]

Probably if we just perform the unsafe check before we remove dead code, it'd be fine?

Yeah it shouldn't be a problem to fix this

[kennytm]

Affects 1.22 beta too.

[nikomatsakis]

Fix is in queue, nice work @arielb1.