Rust-clippy: New lint: Pass references when possible for large data types?

[alxest]

As discussed here, it is often more efficient to pass arguments by reference instead of value.
However, current compiler does not optimize such (and there seems to be good reason for that).
Therefore, it would be appropriate to warn user on such cases.

[flip1995]

As stated here, we already have the opposite lint https://github.com/rust-lang/rust/issues/52274#issuecomment-404508450. The code there should be reusable.

Also there was recently a blog post in TWIR or on reddit benchmarking this for small types: https://www.forrestthewoods.com/blog/should-small-rust-structs-be-passed-by-copy-or-by-borrow/ Maybe @forrestthewoods wants to do the same for huge types, so that we get a good default configuration for this lint? :smile:

[cauebs]

I think I'll give this one a go. I'm just now studying the codebase, and will probably need some pointers.

What should be the criteria for this? How to tell if a type is large enough? Just not being Copy is not enough, surely.

Maybe something like what's on large_enum_variant?

[ebroto]

Maybe something like what's on large_enum_variant?

I think that would be a safe default. The value should be configurable, though.

Also, given the destiny of the opposite lint (#5410), I would make this lint pedantic.

[cauebs]

If someone is willing to help a newbie, I'm having trouble getting my lint to actually run.

I've generated it with cargo dev new-lint, files were created, all fine. But I've made it just output a lint unconditionally and the stderr still matches the empty .stderr, unless I add some garbage to it.

I'm running it with env TESTNAME="pass_large_type_by_move" cargo uitest, and I can see the correct test running.

What am I missing?

[ebroto]

@cauebs I think you may be missing registering your new pass here:
https://github.com/rust-lang/rust-clippy/blob/9408c68c1da55f8a47cb21e761953db30aea3d39/clippy_lints/src/lib.rs#L1130

A previous comment suggested that code from trivially_copy_pass_by_ref should be reused here, so you should probably put your new lint in that module, and rename the module accordingly.

[cauebs]

Would it be more appropriate to implement both in the same struct, or separately, just sharing some code from floating functions?

edit: figured it out myself. It seems fine to just merge both in the same struct. PR submitted below, any feedback is welcome ;)