Unreachable outer expressions should warn

[jruderman]

fn bye() -> ! { fail }

fn warns() { bye(); log(error, 3); }

fn does_not_warn() { log(error, bye()); }

[catamorphism]

@jruderman I think the current behavior is correct. All of the statements in does_not_warn are reachable (there's only one statement). It happens to be the case that the statement always terminates abnormally, but the reachability analysis doesn't warn about that, only about statements that aren't reachable at all.

If you disagree, can you elaborate?

[jruderman]

I feel like "statement with parts that are never evaluated" is roughly equivalent to "sequence of statements, some of which are never evaluated". It's strange that only the latter warns. What's special about separate statements?

[catamorphism]

We're approximating an undecidable problem here, so where we draw the line is always going to be arbitrary. Since it's always going to be an approximation, we're focusing on catching the most likely mistakes. It's my feeling that "unreachable statement" reflects an error that's common for people to make (I know I've hit that error while writing code), whereas "unreachable subexpression" is less common, especially in an imperative language. But I'm happy to hear other opinions.

[graydon]

Seems to me like we ought to be able to calculate this. Or rather, seems to me like we already are, or ought to be, since it's part of the CFG. Is it just a matter of figuring out how to emit a warning?

[graydon]

switched milestone

[pcwalton]

I don't believe this is backwards incompatible, renominating.

[graydon]

just a bug, removing milestone/nomination.

[emberian]

Visiting for triage. FWIW, I agree with OP

[huonw]

triage: still an issue, updated example:

fn bye() -> ! { fail!() }

fn warns() { bye(); 1 + 1; }

fn does_not_warn() { 1 + bye(); }

I don't have a particular opinion, although having bar(foo()) as similar to let r = foo(); bar(r); as possible seems like reasonable goal.

[emberian]

Visiting again for triage.

[steveklabnik]

Triage: current code:

fn bye() -> ! { panic!() }

fn warns() { bye(); 1 + 1; }

fn does_not_warn() { 1 + bye(); }

fn main() {}

current warnings:

hello.rs:1:1: 1:27 warning: function is never used: `bye`, #[warn(dead_code)] on by default
hello.rs:1 fn bye() -> ! { panic!() }
           ^~~~~~~~~~~~~~~~~~~~~~~~~~
hello.rs:3:1: 3:29 warning: function is never used: `warns`, #[warn(dead_code)] on by default
hello.rs:3 fn warns() { bye(); 1 + 1; }
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
hello.rs:3:21: 3:27 warning: unreachable statement, #[warn(unreachable_code)] on by default
hello.rs:3 fn warns() { bye(); 1 + 1; }
                               ^~~~~~
hello.rs:5:1: 5:34 warning: function is never used: `does_not_warn`, #[warn(dead_code)] on by default
hello.rs:5 fn does_not_warn() { 1 + bye(); }
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~