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

unnecessary_lazy_evaluations suggestion causes panic on bool::then #9422

Closed
barryfam opened this issue Sep 3, 2022 · 5 comments · Fixed by #11002
Closed

unnecessary_lazy_evaluations suggestion causes panic on bool::then #9422

barryfam opened this issue Sep 3, 2022 · 5 comments · Fixed by #11002
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@barryfam
Copy link

barryfam commented Sep 3, 2022

Summary

Clippy does not consider the possibility of arithmetic underflow in the non-lazy suggestion

Lint Name

unnecessary_lazy_evaluations

Reproducer

fn f(x: usize) -> Option<usize>
{
    (x >= 5).then(|| x - 5)
    // (x >= 5).then_some(x - 5)  // clippy suggestion panics
}

fn main() {
    let x = f(3);
    println!("{:?}", x);
}

clippy output:

    Checking playground v0.0.1 (/playground)
warning: unnecessary closure used with `bool::then`
 --> src/main.rs:3:5
  |
3 |     (x >= 5).then(|| x - 5)
  |     ^^^^^^^^^--------------
  |              |
  |              help: use `then_some(..)` instead: `then_some(x - 5)`
  |
  = note: `#[warn(clippy::unnecessary_lazy_evaluations)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations

warning: `playground` (bin "playground") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.68s

Version

rustc 1.65.0-nightly (8c6ce6b91 2022-09-02)
binary: rustc
commit-hash: 8c6ce6b91b172f77c795a74bfeaf74b865146b3f
commit-date: 2022-09-02
host: x86_64-unknown-linux-gnu
release: 1.65.0-nightly
LLVM version: 15.0.0

Additional Labels

No response

@barryfam barryfam added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Sep 3, 2022
@clarkmoody
Copy link

I was hit by this bug as well.

@s-cerevisiae
Copy link

warning: unnecessary closure used with `bool::then`
// ...
(x != 0).then(|| 1 / x)
^^^^^^^^^--------------
         |
         help: use `then_some(..)` instead: `then_some(1 / x)`

It also suggests to use then_some when it's possible to divide by zero, which is apparently incorrect.

@alokedesai
Copy link

What's the status of this? Any aim to fix this in future release? The fact that clippy auto-fixes this makes this especially problematic--any auto fix should be guaranteed to be a functional no-op, which is not the case here

zaeleus added a commit to zaeleus/noodles that referenced this issue Jan 25, 2023
…ons lint

Otherwise, this panics when `id` is 0. This is a known false positive in
clippy (rust-lang/rust-clippy#9422).
@ia0
Copy link

ia0 commented Aug 28, 2023

This is more general than bool::then(). This also triggers with Option::unwrap_or():

x.checked_sub(y).unwrap_or_else(|| y - x) // correct but linted
x.checked_sub(y).unwrap_or(y - x) // incorrect suggested fix

@y21
Copy link
Member

y21 commented Aug 28, 2023

The code that clippy uses for figuring out if a closure is necessary or not is shared between all of these lints, so a fix to bool::then will also apply to unwrap_or_else, map_or_else, or_insert_with, etc.
There's an open PR at #11002 that makes clippy avoid suggesting eagerness (then -> then_some) for arithmetic operations in closures (unless it can prove it cannot panic in some obvious cases)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants