Clippy clone double ref advice doesn't work

1

Here is an extract from some sequence matcher code:

fn chain_b_seq(&mut self) {
    for i in 0..self.b.len() {
        let indexes = self.b2j.entry(
            self.b.at_index(i).unwrap()).or_insert_with(Vec::new);
        indexes.push(i);
    }
    let len = self.b.len(); // changed by above
    if len >= 200 {
        let mut b_popular = FnvHashSet::default();
        let test_len = (len as f64 / 100.0).floor() as usize + 1;
        for (element, indexes) in self.b2j.iter() {
            if indexes.len() > test_len {
                b_popular.insert(element.clone());
            }
        }
        for element in &b_popular {
            self.b2j.remove(*element);
        }
    }
}

I'm using 1.31.1 2018 on Win-7-64; clippy says:

error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type --> sequencematcher\src\lib.rs:92:23
   |
92 |                     b_popular.insert(element.clone());
   |                                      ^^^^^^^^^^^^^^^
   |
   = note: #[deny(clippy::clone_double_ref)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/master/index.html#clone_double_ref
help: try dereferencing it
   |
92 |                     b_popular.insert(&(*element).clone());
   |                                      ^^^^^^^^^^^^^^^^^^^
help: or try being explicit about what type to clone
   |
92 |                     b_popular.insert(&str::clone(element));
   |                                      ^^^^^^^^^^^^^^^^^^^^

When I try the first solution I get this error:

error[E0716]: temporary value dropped while borrowed
  --> sequencematcher\src\lib.rs:92:24
   |
92 |                     b_popular.insert(&(*element).clone());
   |                                       ^^^^^^^^^^^^^^^^^^ - temporary value is freed at the end of this statement
   |                                       |
   |                                       creates a temporary which is freed while still in use
...
95 |             for element in &b_popular {
   |                            ---------- borrow used here, in later iteration of loop
   |

And when I try the second solution I get this error (even with use std::clone::Clone):

error[E0277]: the trait bound `str: std::clone::Clone` is not satisfied
  --> sequencematcher\src\lib.rs:93:24
   |
93 |                     b_popular.insert(&str::clone(element));
   |                                       ^^^^^^^^^^ the trait `std::clone::Clone` is not implemented for `str`
   |
   = note: required by `std::clone::Clone::clone`

None of this is a problem for me; I'm happy with the code as it is. It is just that I want to take clippy's advice where possible and thought I'd mention this in case the advice is wrong in this case.

2

So this boils down to:

fn main() {
    let a: &str = "element";
    let b = a.clone();
}

The problem is, that str cannot be cloned and therefore none of the solutions will work.

Can you please tell us, what type element is? Is it really a &str? If yes, clippy is right and you are not copying the String at all and it is propably a bug in your code.

3

The type of b2j is FnvHashMap<&'a str, Vec<usize>>, so element really is a &str

4

In this case you just don't need clone, you only need a copy of the reference in your b_popular hashset so either b_popular.insert(element) or b_popular.insert(*element) should work (because immutable references implement Copy, the second variant is if element is actually &&str which I think it will be in this case).

I think there is a bug in clippy's suggestion though, the second example should show b_popular.insert(<&str>::clone(element)), which should also work but is unnecessary since references are Copy.

3 Likes
5

I tried .insert(element) but that gave E0502. However, .insert(*element) compiled and seems to work. Thanks!