Unhelpful suggestions on reference mutability

[Walther]

This code example is minimized from a wild goose chase I ran into when implementing a solution for one of the Advent of Code puzzles.

The error messages and suggestions were not helpful in solving the issue. Some of them were actively confusing - from the programmer's perspective, the variables clearly need mutability in some form - yet the compiler kept insisting the mut keywords should be removed.

Code

fn main() {
    // const INPUT: &str = include_str!("input.txt");
    const INPUT: &str = r#"Hello
Hola
Bonjour
Ciao
"#;
    let parsed = parse(INPUT);

    let value = part1(&parsed);
    println!("Part 1: {value}");

    let value = part2(&parsed);
    println!("Part 2: {value}");
}

struct Ferris {
    greeting: String,
    count: usize,
}

impl Ferris {
    pub fn new(greeting: &str) -> Self {
        Ferris {
            greeting: greeting.to_string(),
            count: 0,
        }
    }

    pub fn greet(&mut self) {
        self.count += 1;
        println!("{}", self.greeting);
    }

    pub fn greet_count(&self) -> usize {
        self.count
    }
}

type ParsedData = Vec<Ferris>;

fn parse(input: &str) -> ParsedData {
    input.lines().map(Ferris::new).collect()
}

fn part1(data: &ParsedData) -> usize {
    let mut crabs = data.clone();
    for crab in crabs {
        crab.greet();
    }
    crabs.iter().map(Ferris::greet_count).sum()
}

fn part2(_data: &ParsedData) -> usize {
    0
}

Current output

warning: variable does not need to be mutable
  --> src/main.rs:47:9
   |
47 |     let mut crabs = data.clone();
   |         ----^^^^^
   |         |
   |         help: remove this `mut`
   |
   = note: `#[warn(unused_mut)]` on by default

error[E0596]: cannot borrow `*crab` as mutable, as it is behind a `&` reference
  --> src/main.rs:49:9
   |
48 |     for crab in crabs {
   |                 ----- this iterator yields `&` references
49 |         crab.greet();
   |         ^^^^ `crab` is a `&` reference, so the data it refers to cannot be borrowed as mutable

For more information about this error, try `rustc --explain E0596`.

Other cases

Remove mut as instructed:

fn part1(data: &ParsedData) -> usize {
    let crabs = data.clone();
    for crab in crabs {
        crab.greet();
    }
    crabs.iter().map(Ferris::greet_count).sum()
}

error[E0596]: cannot borrow `*crab` as mutable, as it is behind a `&` reference
  --> src/main.rs:49:9
   |
48 |     for crab in crabs {
   |                 ----- this iterator yields `&` references
49 |         crab.greet();
   |         ^^^^ `crab` is a `&` reference, so the data it refers to cannot be borrowed as mutable

For more information about this error, try `rustc --explain E0596`.
error: could not compile `mutability` (bin "mutability") due to 1 previous error

You know this should be mutable, and the error talks about references, so let's try &mut instead:

fn part1(data: &ParsedData) -> usize {
    let mut crabs = data.clone();
    for &mut crab in crabs {
        crab.greet();
    }
    crabs.iter().map(Ferris::greet_count).sum()
}

error[E0308]: mismatched types
  --> src/main.rs:48:9
   |
48 |     for &mut crab in crabs {
   |         ^^^^^^^^^    ----- this is an iterator with items of type `&Ferris`
   |         |
   |         types differ in mutability
   |
   = note:      expected reference `&Ferris`
           found mutable reference `&mut _`

For more information about this error, try `rustc --explain E0308`.

Maybe use the &mut in a different place then?

fn part1(data: &ParsedData) -> usize {
    let mut crabs = data.clone();
    for crab in &mut crabs {
        crab.greet();
    }
    crabs.iter().map(Ferris::greet_count).sum()
}

error[E0277]: `&Vec<Ferris>` is not an iterator
  --> src/main.rs:48:17
   |
48 |     for crab in &mut crabs {
   |                 ^^^^^^^^^^ `&Vec<Ferris>` is not an iterator
   |
   = help: the trait `Iterator` is not implemented for `&Vec<Ferris>`, which is required by `&mut &Vec<Ferris>: IntoIterator`
   = note: required for `&mut &Vec<Ferris>` to implement `Iterator`
   = note: required for `&mut &Vec<Ferris>` to implement `IntoIterator`
help: consider removing the leading `&`-reference
   |
48 -     for crab in &mut crabs {
48 +     for crab in crabs {
   |

For more information about this error, try `rustc --explain E0277`.

Rust Version

rustc 1.83.0 (90b35a623 2024-11-26)
binary: rustc
commit-hash: 90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf
commit-date: 2024-11-26
host: aarch64-apple-darwin
release: 1.83.0
LLVM version: 19.1.1

Anything else?

Here's the actual fix, hidden under a spoiler if anyone wants to try and figure it out first by themselves:

Solution

#[derive(Clone)] // <- this missing Clone derive was the real cause all along!
struct Ferris {
    greeting: String,
    count: usize,
}

fn part1(data: &ParsedData) -> usize {
    let mut crabs = data.clone();
    for crab in &mut crabs {
        crab.greet();
    }
    crabs.iter().map(Ferris::greet_count).sum()
}

Additionally, a friend pointed out to me that using for crab in crabs.iter_mut() produces a helpful error message.

[cyrgani]

Note that there is a warning for this, but it is unfortunately not shown as long as the code has errors. If you remove the crab.greet(); call temporarily, the warning

warning: call to `.clone()` on a reference in this situation does nothing
  --> src/main.rs:47:25
   |
47 |     let mut crabs = data.clone();
   |                         ^^^^^^^^
   |
   = note: the type `Vec<Ferris>` does not implement `Clone`, so calling `clone` on `&Vec<Ferris>` copies the reference, which does not do anything and can be removed
   = note: `#[warn(noop_method_call)]` on by default
help: remove this redundant call
   |
47 -     let mut crabs = data.clone();
47 +     let mut crabs = data;
   |
help: if you meant to clone `Vec<Ferris>`, implement `Clone` for it
  --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:397:1
   |
397+ #[derive(Clone)]
398| pub struct Vec<T, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global> {
   |

is shown, which hints at the solution (though the location is currently wrong, which is reported in #134471).

[Walther]

Thanks for the quick triage!

there is a warning for this, but it is unfortunately not shown as long as the code has errors

Apologies in advance, dissecting this a bit further in terms of both errors as in mistakes and errors as in compiler output 🙇‍♂

Unfortunately, even the "correct code" for the part1 function shows the bizarre error about &Vec<Ferris> not being an iterator. The code in the final solution and in the last error example is exactly the same, except for the added annotation on the struct. No matter how long I would stare at the final part1 code, I would not be able to find an error as in mistake in that code. The actual mistake is the missing annotation.

I think this is the core of this issue report: the warning on useless clone needs a big bump in priority, as it can be the likely root cause of issues, including seemingly more severe errors. Perhaps it should always be shown, even when other errors are present. I'm even pondering if it should be promoted into an error (perhaps a rustc lint that can be optionally allowed if needed), or otherwise highlighted further.

Temporarily removing the mutable method call in order to remove the error (as in make it pass compilation) might make the warning visible. I don't think this is enough though, and we should improve the user experience here. Removing the call that requires mutability is extremely unlikely to be a debugging step done by the user, when using the mutable method is exactly the thing they are trying to do.

I hope this doesn't come off as hostile - I just want to point out more details from the user experience side. Providing this warning only when the code no longer has errors is likely to be too late - the user might not get there without it!

[cyrgani]

Just noticed that this problem that the lint doesn't run is already tracked in #91536.