Unintentionally cloning references can lead to confusing borrow checker errors

[HexyWitch]

(Note from pnkfelix: I have updated the example to continue illustrating the issue even in the presence of NLL.)

This issue is a result of all references implementing Clone. The following code will not compile:

fn foo<T: Default>(list: &mut Vec<T>) {
    let mut cloned_items = Vec::new();
    for v in list.iter() {
        cloned_items.push(v.clone())
    }
    list.push(T::default());
    drop(cloned_items);
}

producing the misleading error message:

error[E0502]: cannot borrow `*list` as mutable because it is also borrowed as immutable
 --> src/main.rs:6:5
  |
3 |     for v in list.iter() {
  |              ---- immutable borrow occurs here
...
6 |     list.push(T::default());
  |     ^^^^ mutable borrow occurs here
7 |     drop(cloned_items);
  |          ------------ immutable borrow later used here
  | - immutable borrow ends here

Because Clone is not a trait bound on T, where you'd expect v.clone() to return a cloned T it's actually returning a cloned &T. The type of cloned_items is inferred to be Vec<&T>, and the lifetime of the list immutable borrow is extended to the lifetime of cloned_items. This is hard to figure out from the error message. The solution is to add Clone as a trait bound to T. The following code builds:

fn foo<T: Default + Clone>(list: &mut Vec<T>) {
    let mut cloned_items = Vec::new();
    for v in list.iter() {
        cloned_items.push(v.clone())
    }
    list.push(T::default());
}

If you give cloned_items an explicit type

let mut cloned_items: Vec<T> = Vec::new();

it gives an error that more clearly explains what you're doing wrong:

error[E0308]: mismatched types
 --> src/main.rs:4:27
  |
4 |         cloned_items.push(v.clone())
  |                           ^^^^^^^^^ expected type parameter, found &T
  |
  = note: expected type `T`
             found type `&T`

I'm not sure what my proposed fix would be. I don't know if there are any use cases for explicitly cloning a reference. Ideally you'd get a warning on cloning a reference to a type that doesn't implement Clone.

[leoyvens]

Clippy has a clone-on-copy lint, moving it into rustc would be a way to push code like this in the right direction.

[memoryruins]

Triage: the original example now compiles on 2018 edition.
2015 errors:

error[E0502]: cannot borrow `*list` as mutable because it is also borrowed as immutable
 --> src/main.rs:6:5
  |
3 |     for v in list.iter() {
  |              ---- immutable borrow occurs here
...
6 |     list.push(T::default());
  |     ^^^^ mutable borrow occurs here
7 | }
  | - immutable borrow ends here

rustc: 1.32.0

[kyren]

Just to be clear, I don't think NLL really has anything to do with it, a simple modification makes the example not compile on rust 2018 edition:

fn foo<T: Default>(list: &mut Vec<T>) {
    let mut cloned_items = Vec::new();
    for v in list.iter() {
        cloned_items.push(v.clone())
    }
    list.push(T::default());
    // Use `cloned_items` to defeat NLL
    drop(cloned_items);
}

whereas ofc this compiles:

fn foo<T: Default + Clone>(list: &mut Vec<T>) {
    let mut cloned_items = Vec::new();
    for v in list.iter() {
        cloned_items.push(v.clone())
    }
    list.push(T::default());
    // Use `cloned_items` to defeat NLL
    drop(cloned_items);
}

[memoryruins]

Thank you for pointing that out, kyren. There was another issue I ran into during triage that only triggered the error when used, and I mistagged this. Enabling nll feature on 2015 does change if an error is produced (before use), but it does not imply a fix (nor does simply compiling imply one either, which is why I made a seperate issue to be reviewed in #57964).

[memoryruins]

and for the sake of update, when using kyren's first example, is

error[E0502]: cannot borrow `*list` as mutable because it is also borrowed as immutable
 --> src/lib.rs:6:5
  |
3 |     for v in list.iter() {
  |              ---- immutable borrow occurs here
...
6 |     list.push(T::default());
  |     ^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
7 |     drop(cloned_items);
  |          ------------ immutable borrow later used here

error: aborting due to previous error

which is not much change except for the spans in diagnostics.