Add a lint for unnecessary by-ref pattern

[huonw]

There are several (many?) instances in rust/src where one has, for example,

match a_function_returning_option() {
    Some(ref a) => { move_a(a.clone()) }
    None => { do_whatever() }
}

The ref a is entirely unnecessary; a could be moved out in the pattern, saving a clone.

(This would have to be careful to not lint when e.g. a is used as a reference in the body, and probably other situations.)

[emberian]

@huonw how would you do this analysis? ie, what if instead if were move_b(a.get_b())?

[emberian]

I mean, clone takes &self, so it is used as a reference. I don't see a way to do this.

[huonw]

The example above is matching on a temporary, so there is never a need for ref. (Similarly if the match is the last use of a variable, e.g.

let foo = returns_option();
do_things(&foo);
match foo {
    Some(ref a) => { /* use a */ }
    None => {}
}

would warn but

let foo = returns_option();
match foo {
    Some(ref a) => { /* use a */ }
    None => {}
}
do_things(&foo);

would not.)

It could be "upgraded" to understand when things do/do not move, and would need to understand guards (i.e. not warn for Some(ref a) if /* uses a */ => ...).

[thestinger]

There are performance reasons to use by-reference for large values even if they don't have a destructor.

[steveklabnik]

Triage: don't think much has changed here.

[steveklabnik]

Triage: still no change. Runnable:

fn returns_option() -> Option<i32> {
    Some(5)
}

fn do_things(_x: &Option<i32>) {
}

fn main() {
    let foo = returns_option();
    do_things(&foo);
    match foo {
            Some(ref a) => { /* use a */ }
                None => {}
    }
}

Given that there are good reasons to use this, and nobody has been clamoring for this lint, I think that we can give this one a close.