Lint telling me to needlessly increase memory footprint

[amab8901]

Summary

clippy::needless_pass_by_value needlessly tells the user to increase the memory footprint by choosing reference (8 bytes) rather than cloned/owned value when the latter is less than 8 bytes. When the latter is less than 8 bytes (might be different than 8 bytes depending on the hardware's register size, etc), the lint should instead advise the reference to be removed. And if both options have the same memory footprint, then lint should be silent.

Lint Name

needless_pass_by_value

Reproducer

I tried this code:

#![warn(clippy::needless_pass_by_value)]

fn main() {}

pub fn lol_func(lol: Lol) -> Lol {
    lol.clone()
}

#[derive(Clone)]
pub enum Lol {
    Variant1,
    Variant2,
}

I saw this warning happen:

consider taking a reference instead: `&Lol`clippy[needless_pass_by_value](https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value)
main.rs(5, 22): original diagnostic
this argument is passed by value, but not consumed in the function body
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_valueclippy[Click for full compiler diagnostic](rust-analyzer-diagnostics-view:/diagnostic%20message%20%5B0%5D?0#file%3A%2F%2F%2Fhome%2Famir-abdin%2Fsrc%2Fnice%2Fnice%2Fsrc%2Fmain.rs)
main.rs(10, 1): consider marking this type as `Copy`
main.rs(1, 9): the lint level is defined here
main.rs(5, 22): consider taking a reference instead: `&Lol`

I expected to see this happen:

"Lol" takes only 1 byte, but reference takes 8 bytes. This lint is telling me to choose 8 bytes (reference) over 1 byte (clone) which is harmful advice. Therefore I expect the following:

1. Clippy checks the memory footprint of reference
2. Clippy checks the memory footprint of the argument type (in this case lol: Lol)
3. Compare
4. if reference has lower memory footprint, then activate lint. If clone has lower memory footprint, then warn about the need to remove reference and use clone. If both have the same memory footprint, then silence the lint.

Version

rustc 1.77.0-nightly (d5fd09972 2024-01-22)
binary: rustc
commit-hash: d5fd0997291ca0135401a39dff25c8a9c13b8961
commit-date: 2024-01-22
host: x86_64-unknown-linux-gnu
release: 1.77.0-nightly
LLVM version: 17.0.6

Additional Labels

No response

[antonilol]

taking a non Copy type by value when it could have been by reference could lead users into cloning from a reference when this is not necessary, this is bad because clone may be expensive, if Clone is not expensive (and can be done with bitwise copy), then make is Copy
on x86 both Lol and &Lol fit in one register (same memory footprint), using &Lol in a function is one memory access, but if the function calling it makes the reference, this is easily optimized out

[lukaslueg]

I'm unsure if not linting would be desirable: If the value is smaller than a reference (which is platform-dependent to begin with), having anything to gain is very strongly dependent on what the compiler did at the specific call-site, most importantly considering inlining. Passing a one-byte type by value instead of by reference does not present the compiler with an opportunity to save seven bytes, e.g. if the value is passed by register (which consumes the full register anyway).
IMHO its more desirable to enhance a developer's muscle memory with respect to not passing owned values if ownership is not required by the callee, and leave possible stack-allocation savings down in call-convention-hell.

[amab8901]

I'm unsure if not linting would be desirable: If the value is smaller than a reference (which is platform-dependent to begin with), having anything to gain is very strongly dependent on what the compiler did at the specific call-site, most importantly considering inlining. Passing a one-byte type by value instead of by reference does not present the compiler with an opportunity to save seven bytes, e.g. if the value is passed by register (which consumes the full register anyway). IMHO its more desirable to enhance a developer's muscle memory with respect to not passing owned values if ownership is not required by the callee, and leave possible stack-allocation savings down in call-convention-hell.

if developing the user's muscle memory is the goal, then why can't we include the step of "check argument's size before deciding whether to pass reference"?

You have a good point when you say the reference size is platform dependent. How about taking the range of all possible reference sizes on modern platforms? Then the linter could say "remove reference" if the argument size is below the range of possible reference sizes. And linter would say "add reference" if the argument size is above the range of possible reference sizes.

[amab8901]

I'm unsure if not linting would be desirable: If the value is smaller than a reference (which is platform-dependent to begin with), having anything to gain is very strongly dependent on what the compiler did at the specific call-site, most importantly considering inlining. Passing a one-byte type by value instead of by reference does not present the compiler with an opportunity to save seven bytes, e.g. if the value is passed by register (which consumes the full register anyway). IMHO its more desirable to enhance a developer's muscle memory with respect to not passing owned values if ownership is not required by the callee, and leave possible stack-allocation savings down in call-convention-hell.

if developing the user's muscle memory is the goal, then why can't we include the step of "check argument's size before deciding whether to pass reference"?

You have a good point when you say the reference size is platform dependent. How about taking the range of all possible reference sizes on most modern platforms? Then the linter could say "remove reference" if the argument size is below the range of possible reference sizes. And linter would say "add reference" if the argument size is above the range of possible reference sizes.

[antonilol]

i think there are only a handful reasons to make a type that is smaller than usize non-Copy (there are little things you can own without a pointer).
one i could come up with is handles to a global resource that is reference counted (handles can't be copied bit by bit, so no Copy and there would be special logic on creation, clone and drop). in that case the type is likely a zst. any function with special logic that accepts the value but doesn't use it can suppress the warning with drop(value).

if the type is non-Copy and equal to or larger in size than usize then it is wasteful only for the resources owned by it, the value itself is passed by pointer anyway (boxed slices behave differently for some reason (pointer metadata special handling?), so technically "equal to or larger in size than 2 usizes"), so from a parameter perspective there is no difference between passing &Vec<u8> or Vec<u8>, both are one pointer to the (pointer, capacity, length) struct.