Fix in clippy::unchecked_duration_subtraction produces unclearer panic

[umgefahren]

Summary

The fix for a case of clippy::unchecked_duration_substraction seems to result in a less helpful panic message then without applying the fix.

Example

Considering the reproducer; without the fix we get the following runtime panic error:

thread 'main' panicked at 'overflow when subtracting duration from instant', library/std/src/time.rs:424:9

which is very descriptive on what happened.

Applying the fix, we get a way less descriptive error:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/main.rs:2:81

It's way harder from this to figure out what happened. So I think one would be better off just leaving the line alone. Considering this either don't apply a fix or move the lint to the pedantic group, but that's just from this little experience. Maybe I missed something.

Linking the issue in which we discovered this problem: libp2p/rust-libp2p#3220 (comment)

Reproducer

I tried this code:

let delta = std::time::Instant::now() - std::time::Duration::MAX;
println!("Delta: {delta:?}");

I expected to see this happen:

No changes were applied.

Instead, this happened:

Got changed to:

let delta = std::time::Instant::now().checked_sub(std::time::Duration::MAX).unwrap();
println!("Delta: {delta:?}");

Version

rustc 1.68.0-nightly (dfe3fe710 2022-12-09)
binary: rustc
commit-hash: dfe3fe710181738a2cb3060c23ec5efb3c68ca09
commit-date: 2022-12-09
host: x86_64-apple-darwin
release: 1.68.0-nightly
LLVM version: 15.0.6

Additional Labels

@rustbot label +L-suggestion +L-correctness +I-false-positive +C-bug +C-question +C-enhancement

[Alexendoo]

Nominating this for discussion at the next clippy meeting (16:00 UTC on the 13th, https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy)

[thomaseizinger]

The outcome of the meeting seems to be that only the suggestion should be removed: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Meeting.202022-12-13/near/315627353

Does that mean the lint will still be a warn-by-default? I think that is misleading. What is the point of even having the concise - notation if clippy unconditionally flags it as bad?

[thomaseizinger]

We are working around this issue now because it will hit stable in ~2 weeks at which point our required clippy CI check will start failing: libp2p/rust-libp2p#3321

[thomaseizinger]

We are working around this issue now because it will hit stable in ~2 weeks at which point our required clippy CI check will start failing: libp2p/rust-libp2p#3321

So we can't work around this because stable clippy doesn't know about the lint. What is the recommended way of proceeding here? Currently we are stuck in a limbo state where our CI is telling us that it will be broken¹ in ~ 2 weeks time and the suggested fix makes the error message worse.

Footnotes

1. The idea of running beta clippy is that we can fix upcoming lint issues before they block our CI pipeline on Rust release day. ↩

[Alexendoo]

You can do #[allow(clippy::unchecked_duration_subtraction, unknown_lints)]

We discussed this again and decided to move the lint to pedantic - #10194. I've nominated it for a beta backport but I don't know how that will work timing wise if it's early enough to change before it hits stable