or_fun_call isn't always a performance lint

[hrxi]

Replacing .unwrap_or(Vec::new()) with .unwrap_or_default() doesn't do anything for performance, they produce identical machine code with optimizations: https://rust.godbolt.org/z/hzTdazGqW.

Perhaps the lint category could be changed or it could be changed to not detect cases where it's not actually a performance warning?

[fedemartinezdev]

@rustbot claim

[fedemartinezdev]

Hi @hrxi !
I am new to contributing to rust-clippy.
Could you tell me where can I find the associated lint so I can begin to take a look? Thanks !

[xFrednet]

Hey @fedemartinezdev, welcome to Clippy 👋. In general we have Clippy's lint list which I use a lot. In this case, I searched for unwrap_or_default and found or_fun_call which is the mentioned lint. This issue requires some detective work to see if any of the cases are still better performance wise or if the entire lint group should be changed. For that, you might have to look at the implementation, to see how the lint internally works 🙃 See clippy_lints/src/methods/or_fun_call.rs

[nikomatsakis]

I ran into this same issue! It's not just Vec::new, I think the same would be true of HashMap::new and other standard library collections.

[hrxi]

I just ran into more clippy suggestions where it doesn't improve performance:

String::from_utf8(vec).or(Err(Enum::InvalidUtf8))

Or a couple of others, the pattern is the same, .or(EnumVariant(PlainOldData)). Changing this to or_else doesn't do anything for performance.

[smoelius]

@hrxi Do you still have the full example? I am having trouble reproducing the behavior you described. For example, the following does not seem to trigger or_fun_call:

enum Enum {
    InvalidUtf8,
}
fn foo() -> Result<String, Enum> {
    let vec = Vec::new();
    String::from_utf8(vec).or(Err(Enum::InvalidUtf8))
}

[hrxi]

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8df0162a2915322edfefceb3ca706ca7

#![warn(clippy::or_fun_call)]

use std::borrow::Cow;
fn foo() -> Result<String, Cow<'static, str>> {
    let vec = Vec::new();
    String::from_utf8(vec).or(Err(Cow::from("abc")))
}

fn main() { let _ = foo(); }

    Checking playground v0.0.1 (/playground)
warning: use of `or` followed by a function call
 --> src/main.rs:6:28
  |
6 |     String::from_utf8(vec).or(Err(Cow::from("abc")))
  |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|_| Err(Cow::from("abc")))`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::or_fun_call)]
  |         ^^^^^^^^^^^^^^^^^^^

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

Haven't tried to minimize it.

[smoelius]

Thanks!

[smoelius]

I think my question was imprecise, though. I was looking for an example that produces equivalent code even when or_else is used. I have little experience with this sort of thing, but the just given example seems to produce different code when or_else is used: https://rust.godbolt.org/z/7dqzxMjYo

Am I making a mistake?

For context, I am trying to understand what is needed to move or_fun_call out of nursery. #10120, if merged, would mean things like .unwrap_or(Vec::new()) and .unwrap_or(HashMap::new()) are handled by a different lint. So I am wondering what false positives would then still remain for or_fun_call.

[hrxi]

It seems you're not compiling with optimizations (-O).

I shortened the code a little and get equivalent assembly, the or_else version even seems (inconsequentially) longer: https://rust.godbolt.org/z/cj1EYhv4a

[smoelius]

Thanks again!