Diagnostics for cloning references are terrible

[Manishearth]

struct Foo;
fn main() {
    let x = Some(Foo);
    if let Some(ref y) = x {
        foo(y.clone());
    }
}

fn foo(x: Foo) {}

https://is.gd/f2AX0B

This gives the very unintuitive error:

error: mismatched types [--explain E0308]
 --> <anon>:5:13
5 |>         foo(y.clone());
  |>             ^^^^^^^^^ expected struct `Foo`, found &-ptr
note: expected type `Foo`
note:    found type `&Foo`

You expect y.clone() to return (*y).clone(), i.e. a Foo. It mysteriously returns an &Foo instead.

Because &T has a clone impl, it too, can be cloned (really, copied), returning a reference of the same lifetime. This is an operation you rarely want to do explicitly, except when dealing with generics.

This error can crop up when you've forgotten to implement Clone on Foo, and have a reference (which could have been silently inserted using ref, as in the code above). However, it's not very obvious what's happening here. It can also crop up if you forget a T: Clone bound

We should hint that Foo itself isn't cloneable and thus we fell back to cloning &Foo here.

This could be a lint on hitting the clone impl for references, but lints run too late.

I'm not sure how this can be implemented, since we need to see the source of a value and that's a bit tricky.

[Manishearth]

cc @nikomatsakis @jonathandturner

[nikomatsakis]

Interesting. This does seem like it would require some pretty special-case code to achieve, but I agree it'd be a nice case to capture.

[Manishearth]

A relatively easy way to catch most instances of this is to

• Pick up the expression which has a type error like this
• If it is a method call, check if that method call is a clone() call resolving to <&T as Clone>::clone
• If it is a variable, find the variable's def
• If the def is a local def, look at the init expr, check if it's a bad clone

This will miss things, but I think the majority of such bugs will be due to this. As it stands this feels like an incredibly easy case to hit (we've all forgotten to decorate structs).

[antoyo]

cc @GuillaumeGomez

[estebank]

CC #34629

[estebank]

As reported in #60875:

use std::collections::HashMap;

#[derive(Clone, Debug)]
struct Foo;

#[derive(Debug)]
struct Bar;

type H = HashMap<String, Vec<(Foo, Bar)>>;

fn wat(h: &H) -> H {
    let h: H = h.clone();
    h
}

fn main() {
    let h = H::new();
    println!("{:?}", wat(&h));
}

This reports:

  --> src/main.rs:12:16
   |
12 |     let h: H = h.clone();
   |                ^^^^^^^^^ expected struct `std::collections::HashMap`, found reference
   |
   = note: expected type `std::collections::HashMap<std::string::String, std::vec::Vec<(Foo, Bar)>>`
              found type `&std::collections::HashMap<std::string::String, std::vec::Vec<(Foo, Bar)>>`

error: aborting due to previous error