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

Duration's constructors should not trigger or_fun_call #7483

Open
asomers-ax opened this issue Jul 23, 2021 · 1 comment
Open

Duration's constructors should not trigger or_fun_call #7483

asomers-ax opened this issue Jul 23, 2021 · 1 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

Comments

@asomers-ax
Copy link

Lint name: or_fun_call

I tried this code:

let mut tick_rate = cli.time.unwrap_or(Duration::from_secs(1));

I expected to see this happen: nothing, since Duration::from_secs is a const fn. it's cheaper to use unwrap_or than unwrap_or_else, which requires creating a closure. I understand the rationale for closing #6943: Clippy can't be certain that the function will be evaluated at compile time. However, I would argue that since Clippy can neither be certain that the function will be evaluated at runtime, it should not make a recommendation one way or the other in this case. This is a case where the programmer probably knows more than Clippy does.

Instead, this happened:

warning: use of `unwrap_or` followed by a function call
   --> src/main.rs:358:34
    |
358 |     let mut tick_rate = cli.time.unwrap_or(Duration::from_secs(1));
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Duration::from_secs(1))`

Meta

  • cargo clippy -V: e.g. clippy 0.0.212 (f455e46 2020-06-20)
  • rustc -Vv:
rustc 1.55.0-nightly (952fdf2a1 2021-07-05)
binary: rustc
commit-hash: 952fdf2a1119affa1b37bcacb0c49cf9f0168ac8
commit-date: 2021-07-05
host: x86_64-unknown-freebsd
release: 1.55.0-nightly
LLVM version: 12.0.1
@asomers-ax asomers-ax 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 Jul 23, 2021
@Jarcho
Copy link
Contributor

Jarcho commented Nov 16, 2021

This will be partially fixed in #7639. const functions for which all arguments are constants will no longer suggest switching to or from a closure. In cases where the arguments are not constants a closure will still be suggested. e.g.

let x = 0;
foo.unwrap(Duration::from_secs(x));

Will still suggest the use of unwrap_or_else

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

No branches or pull requests

2 participants