Needless clone: the most common review comment in Rust

If you read Rust pull requests for a week, “does this need to be a clone?” will be the comment you see most. It is also the topic where clippy helps you least: the lint that would catch it, redundant_clone, lives in the nursery and is off by default. So the compiler is silent, clippy is silent, the tests pass, and the function is six times slower than it should be.

Measured on this toolchain, scanning 100 000 records:

6.3× from one .clone().

What to write

pub struct Record { pub id: u32, pub name: String, pub tags: Vec<String> }

pub fn summarise<'a>(records: &'a [Record], tag: &str) -> Vec<(u32, &'a str)>

Return (id, name) for every record whose tags contain tag, sorted by ascending id. Ids are unique.

Read that signature again

The natural way to write it — fn summarise(records: &[Record], tag: &str) -> Vec<(u32, &str)> — does not compile, and the reason is worth the detour. Lifetime elision gives every input reference its own lifetime, and with two input lifetimes and no &self, there is no rule that says which one the output borrows from. You get E0106 and a request to say what you mean.

What you mean is: the strings come out of records, not out of tag. So name that lifetime and use it on the output. The explicit 'a is not ceremony; it is the one piece of information the compiler genuinely cannot guess.

And note what the return type buys you as a design: because it is &'a str and not String, a solution that clones cannot typecheck its way out. You have to confront the borrow.

The gate

One test runs 50 000 records. The whole call must allocate a number of times that does not grow with the input — a constant, not one per record. The output vector’s own doubling growth is fine and is inside the budget.

The obvious first draft, let tags = r.tags.clone(); before testing it, costs one Vec allocation plus one String allocation per tag, per record. Fifty thousand records is two hundred thousand allocations to answer a question that needed none.

Why beginners clone, and what to do instead

There are exactly four reasons, and each has a correct fix that is not cloning.

1. To escape E0502 / E0499 — “cannot borrow as mutable because it is also borrowed as immutable”. You are reading a collection while trying to modify it. The fix is usually to restructure so the read finishes first (non-lexical lifetimes end borrows at last use, so a smaller scope often just works), to use split_at_mut for disjoint halves, or to collect the decisions in one pass and apply them in a second.

2. To escape E0507 — “cannot move out of borrowed content”. You wanted the value and only had a reference. Ask whether you need the value at all: &str instead of String, iter() instead of into_iter(), a reference in the return type instead of an owned copy. When you genuinely need to take ownership out of a struct you own, std::mem::take or Option::take moves it without allocating.

3. To satisfy 'static — a thread, a callback, a boxed error. Here the clone is often real, but so is Arc: one allocation shared, rather than one per holder.

4. To put something in two places. If the aliasing is genuine — a value that two parts of the program must both see and one must mutate — then Rc<RefCell<T>> is the honest tool and cloning the Rc is cheap and correct. Do not let “clones are bad” push you into a worse design.

The honest counterpoint

Cloning is not a sin. Cloning a three-word String header once, outside a loop, at program startup, is free and arguing about it wastes more time than it costs. The rule is not “never clone”; it is never clone per element in a hot loop, and always know which one you are doing.

Why clippy’s silence is misleading

clone_on_copy fires when you .clone() an i32, and it fires reliably — which teaches people that clippy watches their clones. It does not. It watches the free ones. redundant_clone, the lint that would catch a cloned Vec or String, needs -W clippy::nursery and is known to have false positives. The expensive clones are yours to find.

The lints that are on and worth knowing here: unnecessary_to_owned (you called .to_vec() or .to_string() on something that was about to be borrowed anyway), iter_overeager_cloned (.cloned().filter(..) clones everything and then throws most of it away — filter first), redundant_iter_cloned. Pedantic but valuable: implicit_clone, assigning_clones, cloned_instead_of_copied, clone_on_ref_ptr.

Remember the grade is compile + tests + clippy -D warnings.