Disable expl_impl_clone_on_copy when Copy is manually implemented.

[Stebalien]

Due to some limitations in the deriving infrastructure, it's sometimes necessary to explicitly implement Copy and Clone. In these cases, it would be nice if clippy detected the explicit Copy implementation and didn't warn about the explicit Clone implementation.

[ipetkov]

Just adding a clarification here: for generics, the deriving infrastructure will add bounds for all generic parameters, even when they aren't needed. In those situations a manual implementation is necessary to avoid those bounds, in which situation it would be nice for the warning to be automatically suppressed.

Example:

#![allow(unused)]

use std::marker::PhantomData;

struct Marker<A>(PhantomData<A>);

// Can copy/clone `Marker` regardless of what `A` is
impl<A> Copy for Marker<A> {}
impl<A> Clone for Marker<A> {
    fn clone(&self) -> Self {
        *self
    }
}

struct NoClone;

fn main() {
    let m: Marker<NoClone> = Marker(PhantomData);
    let m2 = m.clone();
}

Following clippy's suggestion here to auto derive either/both of Copy/Clone will prevent the example from compiling.

[oli-obk]

This seems me like an inconsistent use of PhantomData. The whole point of PhantomData is to treat the field as if it were the inner type, but without taking up space. Shouldn't you be using somehing like PhantomData<* const A> instead? If a type has a !Copy field, then it shouldn't be Copy.

Maybe I'm completely off with my reasoning here... I never fully understood all PhantomData semantics

[mcarton]

@oli-obk this example looks ok. PhantomData is mostly used to cheat around lifetimes.You want some type do not outlive some lifetime, but don't actually depend on that lifetime. See for example Iter for slices:

pub struct Iter<'a, T: 'a> {
    ptr: *const T,
    end: *const T,
    _marker: marker::PhantomData<&'a T>,
}

Without the marker, the iterator would not depend on 'a and could outlive the slice!
However the iterator itself is still Copy, whether T is or not. But native derive do not understand that, they see a T, so they want T: Copy.

[oli-obk]

That's not true (anymore), otherwise http://play.integer32.com/?gist=f7d889a3a96894cea5f855488f9a94d5&version=stable wouldn't build. There used to be some issues around these derives, but I think most have been solved.

[mcarton]

Maybe derives know about reference fields now, but they still don't know about any Foo<T> field (they can't as they operate on AST level).

[oli-obk]

They don't need to know anything about Foo<T>, because all they do is generate where Foo<T>: Copy.

#[derive(Copy, Clone)]
struct Foo<'a, T: 'a>(std::marker::PhantomData<&'a T>);

#[derive(Copy, Clone)]
struct Bar<'a, T: 'a>(Foo<'a, T>);

works just fine.

[ipetkov]

@oli-obk I'm using the phantom data to cheat around unused parameters for a builder what will convert Strings into some T. Perhaps storing a PhantomData<*const T> is more correct according to the nomicon, but the derive infrastructure still imposes the same T: Copy/T: Clone bounds I'm trying to avoid.

They don't need to know anything about Foo<T>, because all they do is generate where Foo<T>: Copy.

According to the output of rustdoc the bound is T: Copy and not Foo<T>: Copy for both Foo and Bar (wouldn't impl Copy for Foo<T> where Foo<T>: Copy be impossible to be satisfied?).

As a second data point, this example will fail to compile even though &T is always copy/clone http://play.integer32.com/?gist=d55d20c7c8bfa3f7bffa4d3edeeee844&version=stable

the following trait bounds were not satisfied:
          `NoClone : std::clone::Clone`

[oli-obk]

I found rust-lang/rust#26925, so it looks like this issue is going to stay with us.

[dennis-hamester]

In one of my projects, I have several cases like this:

pub struct Foo(Option<unsafe extern "system" fn()>);

impl Copy for Foo {}

impl Clone for Foo {
    fn clone(&self) -> Self {
        *self
    }
}

Unfortunately, fn pointers with a system ABI are neither Copy nor Clone, which is why I manually implement them.

At the moment, I simply disable expl_impl_clone_on_copy, but it would be great if clippy figured out, that deriving those traits is not an option.

[jplatte]

As long as this is not fixed, it should be listed as a known problem with the lint. Currently, known problems are "None": https://rust-lang-nursery.github.io/rust-clippy/v0.0.165/index.html#expl_impl_clone_on_copy