functions implement Copy but not Clone

[fuchsnj]

This compiles

fn func(){}

fn main(){
    run(func);
    run(func);
}

fn run<T>(f: T)
where T: Fn() + Copy{
    f();
}

but this does not (playground)

fn func(){}

fn main(){
    run(func);
    run(func);
}

fn run<T>(f: T)
where T: Fn() + Clone{
    f();
}

The only difference is the bounds on T. The Copy trait requires that the Clone trait is also implemented, so I believe both of these should compile.

[jonas-schievink]

Even worse, because Copy usually implies Clone, this ICE's:

fn c<T: Copy>(t: T) -> (T, T) { (t.clone(), t) }

fn f() {}

fn main() {
    c(0);
    c(f);
}

<anon>:1:34: 1:43 error: internal compiler error: Encountered error `Unimplemented` selecting `Binder(<fn() {f} as core::clone::Clone>)` during trans
<anon>:1 fn c<T: Copy>(t: T) -> (T, T) { (t.clone(), t) }
                                          ^~~~~~~~~

This (including the ICE) was already reported in #24000, so this looks like a duplicate

[alexcrichton]

Nominating as this seems relatively serious, but the failure mode is just an ICE then it doesn't seem super super pressing.

[arielb1]

@nikomatsakis: should we make Clone a lang-item? I think it being a supertrait of Copy forces our hand.

[eefriedman]

We could just add something like the following to the libcore, I think:

impl<R> Clone for fn() -> R {
    fn clone(&self) -> Self { self }
}
impl<R, A1> Clone for fn(A1) -> R {
    fn clone(&self) -> Self { self }
}
impl<R, A1, A2> Clone for fn(A1, A2) -> R {
    fn clone(&self) -> Self { self }
}
impl<R, A1, A2, A3> Clone for fn(A1, A2, A3) -> R {
    fn clone(&self) -> Self { self }
}

We do exactly the same thing for arrays.

[eefriedman]

Err, nevermind, that doesn't really do the right thing: that only works for function pointers, not function definitions.

[nagisa]

impl<T: Copy> Clone for T?

I’m pretty sure this cannot be done backwards compatibly because such impl would conflict with all the explicit impl Clones, but still had to mention it 😊

[huonw]

@nagisa it should be fine with specialisation.

[arielb1]

@huonw

It would be fine with lattice impls. Which is a different feature from specialization. Actually, even that wouldn't be backwards-compatible, given that you would need to provide the meet impls (e.g. impl<T: Copy> Clone for Option<T>) or to have some sort of priority mechanism.

[huonw]

I don't understand what you mean, allowing more specialised impls (i.e. this) is exactly what specialisation is. Anyway nitpicking the exact terminology probably doesn't matter so much here...

[arielb1]

@huonw

@aturon's specialization RFC - rust-lang/rfcs#1210 - does not allow for overlapping impls in any case. There are also proposals for "lattice impls", which allow overlapping impls as long as there is a meet between every 2 impls. This is not part of the RFC.

[huonw]

Ah, I was speaking more abstractly, but in fact, @aturon has realised a modification to the RFC that allows cases like this, just hasn't had time to publish.

[nikomatsakis]

On Fri, Sep 04, 2015 at 01:33:13PM -0700, arielb1 wrote:

@nikomatsakis: should we make Clone a lang-item? I think it being a supertrait of Copy forces our hand.

Possibly. My preferred strategy here would be to leverage
specialization, such that we have a blanket impl of Clone for all T: Copy. I know that some variants of specialization cannot accommodate
this, though.

[arielb1]

@nikomatsakis

Variants which include all these proposed in the RFC.

[nikomatsakis]

triage: P-medium

We're still working out the preferred way to solve this problem. I tagged T-lang because of the interaction w/ specialization.

[brson]

Triage: P-low. ICE is problematic though would be great to fix it. Can anybody mentor?

[nikomatsakis]

@brson fixing this is no small thing, sadly. I had hoped we could address it through specialization but that does not seem to be the case. @arielb1 has been advocating for building the clone impls into the compiler, that might be the most prudent short-term path at least, particularly given that Clone is a lang-item anyway. I could mentor this.

[brson]

@nikomatsakis oh, that would be great. I was only hoping to stop the ICE!

[retep998]

If Copy is implemented by the compiler automatically for certain types, then I don't see why the compiler wouldn't also implement Clone, and seems like the obvious solution to me. Using blanket impls and specialization just feels like the standard library covering up a deficiency in the compiler.

[cseale]

@nikomatsakis If this is still an issue and you have the time, I would like to look at this. I would be pretty new to Rust and the Compiler though

[nikomatsakis]

@cseale Great! Since writing my last comment, I now have greater hope for being able to fix this through specialization (and hence without modifying the compiler), but those plans remain a bit in flux, so I think it's probably best to close this hole in the compiler for the time being. I will try to write up some instructions tomorrow!

[cseale]

Hi @nikomatsakis, any chance we could take a look at this? 😃

[nikomatsakis]

@cseale argh, sorry, added to my to-do list for tomorrow =)

I think part of my hesitation is that it's going to be potentially a bit messy to hack this in, and if specialization can solve it that will be much nicer (and of course it's been this long...). But let me glance over code tomorrow to try and decide just how invasive it will be.

[cseale]

@nikomatsakis no problem! if there is another outstanding E-mentor issue involving that compiler maybe I would be better spending my time there? I am just trying to get involved in that part of the codebase

[scalexm]

I've written a fix for this locally, however I wonder how it would be integrated in the compiler (if it is).
Basically I had to do two steps:

• Add a lang item for Clone and let the compiler generate implementations for relevant types (i.e. every type which has builtin support for Copy, + tuples and arrays of types which implement Clone). Then build the compiler.
• Change libcore so that the Clone trait is marked as the corresponding lang item and remove manual implementations of Clone for the types described above. Then build the compiler using the previously built compiler.

I cannot merge the two steps since the compiler would not understand the attribute #[lang = "clone"] and would anyway not be able to compile with the Clone implementations being removed.

[kennytm]

@scalexm Use #[cfg_attr(not(stage0), lang = "clone")] so only the new compiler will recognize the lang item.

Basically you slap a #[cfg(not(stage0))] on anything that only the new compiler understands, and put #[cfg(stage0)] on anything only required by the old compiler.