Assuming `Clone::clone()` is equivalent to `Copy` semantics for all instances of `Copy`

[briansmith]

I implement Clone by hand in winapi even though everything is also Copy since it lets me make the Clone impl invoke the Copy impl. The derived Clone is a pile of bloated codegen that takes a significant amount of time to compile.

I think that will be fixed in https://github.com/rust-lang/rust/issues/31085.

[briansmith]

This isn’t true in a mechanical sense: I’ve implemented Clone by hand on Copy types in the past just fine (you have to sometimes if you have phantom’ed type parameters which derive will merrily whack constraints on even when it doesn’t make sense to do so).

Sorry, I don’t understand this. Could you post (a link to) an example?

[DanielKeep]

Well, there’re these manual impls of Copy and Clone.

[huon]

It has been discussed. One route planned is/was to have a impl<T: Copy> Clone for T {}, but this needs specialisation, and, apparently, even the current specialisation plan doesn’t quite solve it.

If a type implements Copy, it must derive Clone, not provide a custom implementation.

This is non-trivial: a type may only be Copy in a (strict) subset of the times it is Clone, e.g. Option<T>: Copy when T: Copy, but Option<T>: Clone when T: Clone.

[nikomatsakis]

My feeling is that if a type is Copy, then it is valid to not invoke Clone and instead copy the bytes manually.

[nikomatsakis]

In other words – Copy refines the contract on Clone to be more precise, and if your Clone impl diverges from the Copy semantics then you are asking for trouble.

[glaebhoerl]

@nikomatsakis But isn’t giving the optimizer the right to assume that the contract is obeyed the kind of thing that’s only OK with unsafe traits?

[rkruppe]

I don’t know about the optimizer (different behavior depending on whether a compiler optimization runs is really really bad), but certainly library code (including essential code in libcore, like clone_from_slice) can assume a contract holds even without unsafe trait. The latter is only needed when violating the contract can lead to memory unsafety. Functionally wrong behavior is not in the purview of unsafe.

[glaebhoerl]

certainly library code (including essential code in libcore, like clone_from_slice) can assume a contract holds even without unsafe trait. The latter is only needed when violating the contract can lead to memory unsafety

Yes I agree.

I don’t know about the optimizer

I think it’s different.

Suppose you wrote unsafe code which depends on your custom implementation of clone() being called. The unsafe code in this instance is not relying on any implicit contracts: it is relying on the fact that when it calls clone(), clone() will be called. If the compiler rewrites it to skip the call and Copy instead, the unsafe code violates memory safety. Who is at fault? It seems pretty clear-cut to me that it’s the compiler. What gives it the right to rewrite expressions according to the informal invariant around Clone and Copy, but not other any other traits, when there isn’t any in-language indication of Clone or Copy being special in this respect? For this to be OK I believe that we would need unsafe trait Copy, analogously to ExactSizeIterator.

Note that I’m not asking how probable, wise, or inlandish this scenario is. It’s probably not any of those things. But when thinking about language guarantees and unsafe we deal in absolutes. Overflowing the reference count of an Rc or Arc is not particularly likely either, yet we considered it a bug in need of fixing. This optimization would likewise be wrong on principle.

[bluss]

This seems to be precisely the parametricity debate that is going on in the impl specialization RFC thread too. We discussed different kinds of parametricity violations, and that traits that are sufficiently similar, such as BufRead being an extension of Read, could be expected to be used.

Following that, Copy is a very close extension to Clone, and if it seems to be the epitome of a closely related trait and is strictly better to use when it is possible.

[bluss]

I agree that the optimizer shouldn’t outright replace a call to .clone() with something else.

[nikomatsakis]

Aren’t we talking about library code here? I was not saying that the optimizer would go and remove calls to clone.

[arielb1]

At least, if we do it, make Clone a lang-item. If we have the rule well-specified enough this should not pose a conceptual problem (in the same way that integers being able to overflow is not a conceptual problem, just a semantics problem).

[briansmith]

Aren’t we talking about library code here? I was not saying that the optimizer would go and remove calls to clone.

The optimizer sure can remove calls to clone and I hope it already does! In particular, the compiler should be able to determine when a clone() implementation is equivalent to a memcpy and then optimize it away to memcpy. That’s optimizer 101 in 2016 and any case the compiler doesn’t already do that should be filed as a bug against the compiler. The only issue is whether the optimizer can cheat and consider a Copy annotation to be sufficient evidence to avoid analyzing the implementation of the clone() method. Actually, having thought about it now, I don’t think it is necessary for it to do that, because it should easily be able to infer that from the implementation of clone().

[nikomatsakis]

@briansmith that is a bit different – if it can reason about the actual code being called, it can do the usual optimizations, but the author can’t tell the difference. The question here is could the compiler opt to change

x.clone()

so that it in fact just does a mem copy even if clone contains (or may contain, because we don’t know what it does) a println! (and hence the println would never fire).

Probably this is a bridge too far, though it’s certainly not that different from libstd APIs selecting a specialized impl. You can use this to argue many possible conclusions, I think.

[briansmith]

My point is that, given a reasonable optimizer, then there will be no significant advantage to recognizing that the type implements Copy so the issue is moot, at least as far as Clone/Copy goes.

There might be a more general issue for specialization, but it shouldn’t be an issue for Clone/Copy.

At least, it would be helpful to see a realistic example of an implementation of clone() that the compiler can’t reasonably be expected to optimize to memcpy() even without knowing anything special about Copy.

[bluss]

Sure here’s such an example (playground link)..

The function copy_composite is compiled to memcpy, the clone_composite function is not. Use Release mode and look at ASM to verify.

The reason seems to be that in clone_composite, it’s using the derived Clone impl that assigns fields separately, and the other struct uses a Copy-based Clone impl. It’s a bit disappointing that such a simple example (no struct padding) fails. The simpler example of a single-field struct is of course successful.

[briansmith]

I think that is a bug in the optimizer. I commented such in https://github.com/rust-lang/rust/pull/31414. If rustc doesn’t optimize that kind of code then there are a lot of (functional) coding patterns that are going to be a disaster in terms of performance.

[glaebhoerl]

Aren’t we talking about library code here? I was not saying that the optimizer would go and remove calls to clone.

Oh. For some reason I was interpreting the OP as the compiler magically doing it somehow. Doing it in library code seems less problematic.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.