Skip to content

Appease Clippy's unnecessary_lazy_evaluations in Rust 1.60 #1406

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

Closed
wants to merge 1 commit into from

Conversation

david-perez
Copy link
Contributor

In Rust 1.60 we get Clippy lint warnings that make the build fail.

warning: unnecessary closure used to substitute value for `Option::None`
   --> inlineable/src/json_errors.rs:98:25
    |
98  |       if let Some(code) = error_type_from_header(headers)
    |  _________________________^
99  | |         .map_err(|_| DeserializeError::custom("X-Amzn-Errortype header was not valid UTF-8"))?
100 | |         .or_else(|| code.as_deref())
    | |____________________________________^
    |
    = 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
help: use `or` instead
    |
98  ~     if let Some(code) = error_type_from_header(headers)
99  +         .map_err(|_| DeserializeError::custom("X-Amzn-Errortype header was not valid UTF-8"))?.or(code.as_deref())

Since len is cheap and deref is supposed to be cheap, these resolve
to reading a pointer in assembly and they don't need to be lazily
evaluated.

https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
rust-lang/rust-clippy#8109
https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/eager_or_lazy.rs

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

In Rust 1.60 we get Clippy lint warnings that make the build fail.

```
warning: unnecessary closure used to substitute value for `Option::None`
   --> inlineable/src/json_errors.rs:98:25
    |
98  |       if let Some(code) = error_type_from_header(headers)
    |  _________________________^
99  | |         .map_err(|_| DeserializeError::custom("X-Amzn-Errortype header was not valid UTF-8"))?
100 | |         .or_else(|| code.as_deref())
    | |____________________________________^
    |
    = 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
help: use `or` instead
    |
98  ~     if let Some(code) = error_type_from_header(headers)
99  +         .map_err(|_| DeserializeError::custom("X-Amzn-Errortype header was not valid UTF-8"))?.or(code.as_deref())
```

Since `len` is cheap and `deref` is supposed to be cheap, these resolve
to reading a pointer in assembly and they don't need to be lazily
evaluated.

https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
rust-lang/rust-clippy#8109
https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/eager_or_lazy.rs
@david-perez david-perez requested a review from a team as a code owner May 24, 2022 12:23
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 6.37% 81360.75 76485.52
Total requests 6.31% 7323907 6889432
Total errors NaN% 0 0
Total successes 6.31% 7323907 6889432
Average latency ms 44.87% 1.13 0.78
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 12.91% 21.69 19.21
Stdev latency ms 54.47% 1.9 1.23
Transfer Mb 6.31% 761.32 716.16
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@david-perez
Copy link
Contributor Author

david-perez commented May 24, 2022

But CI of course doesn't like this until we upgrade from 1.58.

@Velfi
Copy link
Contributor

Velfi commented May 24, 2022

We can up the MSRV when they release 1.62 in a few weeks and then merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants