false positive "unreachable expression"

[matthiaskrgr]

pub fn main() {
     std::process::Command::new("test").output().unwrap_or(panic!("oh no"));
}

rustc warns that the entire line is an unreachable expression

warning: unreachable expression
 --> src/main.rs:3:6
  |
3 |      std::process::Command::new("test").output().unwrap_or(panic!("oh no"));
  |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(unreachable_code)] on by default

however the code is clearly executed as can be see on the playground

   Compiling playground v0.0.1 (/playground)
warning: unreachable expression
 --> src/main.rs:2:6
  |
2 |      std::process::Command::new("test").output().unwrap_or(panic!("oh no"));
  |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(unreachable_code)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.60s
     Running `target/debug/playground`
thread 'main' panicked at 'oh no', src/main.rs:2:60
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

If the line was truly unreachable, there would be no panic...

This bug appears on stable, beta and nightly

[jonas-schievink]

The .unwrap_or is the unreachable part, so perhaps the error should be rephrased and pointed at that function call only

[matthiaskrgr]

Hmm, so the span is wrong?

warning: unreachable expression
 --> src/main.rs:2:6
  |
2 | /      std::process::Command::new("test")
3 | |      .output()
4 | |      .unwrap_or(
5 | |          panic!("oh no")
6 | |      );
  | |______^
  |

[Mark-Simulacrum]

the order of execution is panic! and then everything else, so in some sense everything but the panic is unreachable. I'm not sure what a good span for that would be.

[jonas-schievink]

The method receiver is evaluated first, so the Command::new and output() calls still happen. The span can point just at the .unwrap_or.

[Mark-Simulacrum]

Ah, hm, I didn't expect that to be the case, but it makes sense. Then this seems relatively straightforward as to what the expected output is at least.

[clarfonthey]

I feel like this should also add a note that the panic is called before everything else. It could also suggest unwrap_or_else but that seems more like a clippy lint.

[ayuusweetfish]

Similar case:

pub fn main() {
    loop {
        std::process::Command::new("test").output().unwrap_or(continue);
//      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// warning: unreachable expression
        println!("done");
    }
}

Maybe we can simply add a note that unwrap_or() won't be called because evaluation of its arguments leaves the current code path. I agree that unwrap_or_else suggestion is indeed too specific and may be better suited to clippy.

If this is acceptable, I would like to pick it up! As long as it's fine for me to progress more slowly than normal 😝

[clarfonthey]

GitHub wasn't clever enough to read that the PR said "might close" instead of "close." The PR seems to imply more work can be done on this.