Clippy declaring closure as unnecessary lazy operation

help
1

I get the following clippy suggestion:

warning: unnecessary closure used with `bool::then`
  --> src/percentage.rs:8:3
   |
8  | /         (min..=max)
9  | |             .contains(&percentage)
10 | |             .then(|| Self((percentage - min) / (max - min)))
   | |______________----------------------------------------------^
   |                |
   |                help: use `then_some(..)` instead: `then_some(Self((percentage - min) / (max - min)))`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
   = note: `#[warn(clippy::unnecessary_lazy_evaluations)]` on by default

This seems odd to me. I see the minus, plus and division operations as "functions", and I thought using closures here would delay the execution of these functions until they are needed (lazy evaluation). Am I misunderstanding something?

1 Like
2

That does seem overeager, especially since any of them could even panic.

3

Because you're only doing "cheap" operations, clippy's preference is to avoid the closure. Doing arithmetic on primitive number types is so well understood by the compiler/optimizer that if you don't actually use the result the compiled program can easily avoid computing it, and also in the other direction, that even if the operation isn't performed in the source it might still be computed speculatively because that's easier than not.

If the operations could overflow, though, those side effects are properly deferred, and this would be a false positive on the lint.

2 Likes
4

Here is a program demonstrating that Clippy's suggestion changes the behavior:

pub fn foo_lazy(x: i32) -> Option<i32> {
    (x > 0).then(|| 100 / x)
}

pub fn foo_eager(x: i32) -> Option<i32> {
    (x > 0).then_some(100 / x)
}

#[test]
fn test_lazy() {
    assert_eq!(foo_lazy(0), None, "lazy");
}
#[test]
fn test_eager() {
    assert_eq!(foo_eager(0), None, "eager");
}

Clippy warns on foo_lazy, and suggests converting it to be equivalent to foo_eager, but the tests show that one returns None where wanted and the other panics.

1 Like
5

It's been reported, but not fixed:

2 Likes
6

More generally:

1 Like
7

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.