Here's some literature:
The API guidelines are specifically about public interfaces, but they're usually good advice for private interfaces as well.
If a function requires ownership of an argument, it should take ownership of the argument rather than borrowing and cloning the argument.
// Prefer this:
fn foo(b: Bar) {
/* use b as owned, directly */
}
// Over this:
fn foo(b: &Bar) {
let b = b.clone();
/* use b as owned after cloning */
}
If a function does not require ownership of an argument, it should take a shared or exclusive borrow of the argument rather than taking ownership and dropping the argument.
// Prefer this:
fn foo(b: &Bar) {
/* use b as borrowed */
}
// Over this:
fn foo(b: Bar) {
/* use b as borrowed, it is implicitly dropped before function returns */
}
This is not really correct since you should still pass small Copy types by value even if you don't need ownership, but your type isn't small.
What it does
Checks for functions taking arguments by value, where the argument type is Copy and large enough to be worth considering passing by reference. Does not trigger if the function is being exported, because that might induce API breakage, if the parameter is declared as mutable, or if the argument is a self.
Why is this bad?
Arguments passed by value might result in an unnecessary shallow copy, taking up more space in the stack and requiring a call to memcpy, which can be expensive.
Example
#[derive(Clone, Copy)]
struct TooLarge([u8; 2048]);
fn foo(v: TooLarge) {}
Use instead:
fn foo(v: &TooLarge) {}
Configuration
This lint has the following configuration variables:
avoid-breaking-exported-api: Suppress lints whenever the suggested change would cause breakage for other crates. (default: true)pass-by-value-size-limit: The minimum size (in bytes) to consider a type for passing by reference instead of by value. (default: 256)
Clippy puts the cutoff at 256 bytes, which is of course not a precise number, but is definitely less than your type. It also brings up another reason: passing by reference will make it harder to accidentally copy a value. The other way is to wrap a Copy type in a non-Copy wrapper:
struct E(FixedWeight);
fn dec_write_e(e: E)
Then you can have the initialization return E instead of FixedWeight. This also helps you keep e and s from being accidentally swapped.
You should think of taking ownership when e logically cannot be used twice, regardless of how it's actually being used. Usually this happens when you're modifying e, but since that's not the case, perhaps you'd take ownership if it's some one-time initialization seed like a UUID, or if you're transforming it into a different representation that should be used instead of the original. Even so, since it's Copy, taking a reference isn't that much different.
It sounds like taking s by reference doesn't have any downsides.
