Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False-positive with the unreachable code lint #88393

Open
bjorn3 opened this issue Aug 27, 2021 · 5 comments
Open

False-positive with the unreachable code lint #88393

bjorn3 opened this issue Aug 27, 2021 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-unreachable_code Lint: unreachable_code T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Aug 27, 2021

Given the following code: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=67b068c7dded060fd71f7e5a5ca91278

#![allow(deprecated, invalid_value)]

enum Void {}

fn main() {
    if false {
        unsafe { std::mem::uninitialized::<Void>(); }
    }
    
    println!();
}

The current output is:

warning: unreachable expression
  --> src/main.rs:10:5
   |
7  |         unsafe { std::mem::uninitialized::<Void>(); }
   |                  --------------------------------- any code following this expression is unreachable
...
10 |     println!();
   |     ^^^^^^^^^^^ unreachable expression
   |
   = note: `#[warn(unreachable_code)]` on by default
note: this expression has type `Void`, which is uninhabited
  --> src/main.rs:7:18
   |
7  |         unsafe { std::mem::uninitialized::<Void>(); }
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: this warning originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `playground` (bin "playground") generated 1 warning

I expected no lint to trigger as the println!() is actually reachable. This problem doesn't occur for panic!().

@bjorn3 bjorn3 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 27, 2021
@jonas-schievink jonas-schievink added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. labels Aug 27, 2021
@pierwill
Copy link
Member

@rustbot claim

@pierwill
Copy link
Member

Hi @bjorn3! Can you elaborate a little on what makes println!() reachable here?

@bjorn3
Copy link
Member Author

bjorn3 commented Oct 18, 2021

println!() is reachable as the std::mem::uninitialized::<Void>(); is never executed due to if false {}.

@pierwill
Copy link
Member

@rustbot release-assignment

@tabokie
Copy link
Contributor

tabokie commented Oct 25, 2021

I've located the issue to be in the liveness check:

hir::ExprKind::If(ref cond, ref then, ref else_opt) => {
//
// (cond)
// |
// v
// (expr)
// / \
// | |
// v v
// (then)(els)
// | |
// v v
// ( succ )
//
let else_ln =
self.propagate_through_opt_expr(else_opt.as_ref().map(|e| &**e), succ);
let then_ln = self.propagate_through_expr(&then, succ);
let ln = self.live_node(expr.hir_id, expr.span);
self.init_from_succ(ln, else_ln);
self.merge_from_succ(ln, then_ln);
self.propagate_through_expr(&cond, ln)
}

Here a diverge point sits inside a conditional block, but this information is lost when calling propagate_through_expr, which then calls warn_about_unreachable.

A simple fix for this would be skipping the check for conditional neighbors. But that will actually bring out another false negative case:

#![allow(deprecated, invalid_value)]

enum Void {}

fn main() {
    let b = false;
    // should warn:
    // any code following this `match` expression is unreachable, as all arms diverge
    match b {
        false => unsafe { std::mem::uninitialized::<Void>(); }
        _ => unreachable!(),
    }
    println!();
}

I'm quite new to the compiler code, but this case seems difficult to fix because the exhaustiveness check takes place in a earlier(?) pass which doesn't have the complete type information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-unreachable_code Lint: unreachable_code T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants