Clippy claims subtraction closure is unnecessary in bool.then()

help
1

I have the code:

        (self.available_memory < self.min_available_memory_limit)
            .then(|| self.min_available_memory_limit - self.available_memory)

which returns the amount the memory limit was broken by, or None if it wasn't broken. For this code, clippy suggests:

warning: unnecessary closure used with `bool::then`
   --> control_server/src/memory_cleaner/mod.rs:100:9
    |
100 | /         (self.available_memory < self.min_available_memory_limit)
101 | |             .then(|| self.min_available_memory_limit - self.available_memory)
    | |______________---------------------------------------------------------------^
    |                |
    |                help: use `then_some(..)` instead: `then_some(self.min_available_memory_limit - self.available_memory)`
    |
    = 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

However, I don't understand why the evaluations shouldn't be lazy. I know that the compiler will probably optimize this away anyways (the integer comparison might be implemented with a subtraction anyways, so it would only need to do the subtraction once in total), but I don't think the clippy lint is aware of that. From my understanding, anytime you perform some computation (the subtraction in this case), you should put that in a closure using .then over .then_some, because the latter is eagerly evaluated, while the former is only evaluated when it is actually called.
Can someone explain why I should use .them_some here?

2

Clippy rules always include a link to the explanation for them. In this case it says the following:

This lint suggests changing the following functions, when eager evaluation results in simpler code.

Which I can perfectly understand. A substraction is not a good reason for lazy evaluation.

3 Likes
3

But why is a subtraction not a good reason for lazy evaluation? It's not a constant, which is why I thought it would be a good candidate for lazy evaluation.

4

Indeed your lazy call gets optimized away. But unnecessary_lazy_evaluations is a style lint and has nothing to do with performance. While the goal of style lints is to enhance the readability of your code, coding style can be somewhat subjective, so I don't see a problem when you add a #[allow(...)] here if you find the lint does not conform with your preferred coding style.

3 Likes
5

Because your lazy evaluation it is optimized away by the compiler anyway

6

As much as I agree with simplifying code over trying to micro-optimize subtractions, there still is a bug. If the subtraction is unsigned, it overflows, so in debug mode, the lint introduces a panic: Playground

4 Likes
7

Very good point, the values are indeed unsigned, and that was part of the reason I introduced the .then in the first place. Thanks for pointing that out!
Do you see a better way to write this without passing a subtraction in a closure? Maybe the std lib has something similar I haven't found yet.

8

It's just checked_sub().

6 Likes
9

Great point, that's what I'll use of course!

10

By the way, here's the clippy issue for that:

3 Likes
11

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.