Rust-clippy: needless_pass_by_value makes no sense for C-like enums

[vthriller]

enum Foo { Bar, Baz }

fn f(x: Foo) -> bool {
  match x { 
    Foo::Bar => true,
    Foo::Baz => false,
  }
}

fn main() {
    f(Foo::Bar);
}

Here,Foo can be easily copied (hey, we can even cast it to i32!), so there's no point in referencing and dereferencing it all over the code.

[vthriller]

Also, #[derive(Clone, Copy)] for the enum (which some might add because of #![warn(missing_copy_implementations)]) eliminates this warning, although I did not spot any difference in the output assembly for both versions.

[sinkuu]

Deriving Copy for Foo or taking it by reference could improve the usability of the function. No performance difference here, so this is only a stylistic issue.

// deriving `Copy`
lelt x = Foo::Bar;
f(x);
f(x); // ok

// taking by reference
let x = Foo::Bar;
f(&x);
f(&x); // ok

You can allow(needless_pass_by_value) if you don't need it. This lint is prone to false-positives:(

[oli-obk]

This lint is prone to false-positives

can you elaborate? We'd like to eliminate them.

[sinkuu]

Oops, I didn't mean false-positives. I think there are many _true_-positives in places where one don't care care about this (tests, private helper that aren't called multiple times, etc.). They should be fine with #[allow].

[oli-obk]

How about we suggest either using a reference OR implementing Copy?

[vthriller]

Sounds good enough to me (especially now that I suddenly realized that C-like enums that don't feature discriminators explicitly (Foo = 0x123) can easily be turnen into regular enums in the future, and there's already good reasons to not impl Copy such enums implicitly—and I'll stop here because the rest is hardly about the clippy but now at least you get some idea why I filed this issue in the first place).

[oli-obk]

Ok, so to fix this issue, one needs to check if the type is local to the crate. If it is, then a second suggestion should be added (adding #[derive(Copy, Clone)]). It might not be possible to do the second suggestion, since the type might have fields that are !Copy, but that's what the first suggestion is there for.

Bonus points for detecting the case where the second suggestion is not applicable and not producing it in that case

[crumblingstatue]

I don't want to open a new issue for this, but does needless_pass_by_value make sense for reference counted types like Rc or Arc? Wouldn't passing these by reference create another layer of indirection, which could be a performance hit?

[Manishearth]

Sometimes you need to, because sometimes you need the ability to clone it into a new one.

Ideally &RcBox should be something you can do, but it's not (with the stdlib Rc)