RFC - compiler-generated Clone impls for Arrays and Tuples

[arielb1]

rendered.

cc @eddyb @nikomatsakis @aturon

Edit: updated rendered link

[comex]

Perhaps add to Alternatives the option of waiting until the language features needed to make a proper generic implementation are available. For arrays, that would presumably be RFC 2000; who knows about tuples.

[cramertj]

@comex Tuples fall under #1935, but that RFC certainly won't land before the impl period.

[kennytm]

For convenience, please make these real links.

[arielb1]

Make what real links?

[kennytm]

@arielb1 Change

... ICEs such as rust-lang/rust#25733 because ...

to

... ICEs such as [rust-lang/rust#25733] because ...

[rust-lang/rust#25733]: https://github.com/rust-lang/rust/issues/25733

Similar for references to "RFC #XXXX".

[kennytm]

#2132 (comment) refers me back to here 😂: You may want to add rules for function types and function pointers (with arity > 12) as well. These types already impl Copy, and thus should also impl Clone.

T1, ..., Tn types 
R type
"ABI" abi
(may need n > 12 || "ABI" ∉ {"C", "Rust"} to avoid existing impl)
----------------------------------------------------
extern "ABI" fn(T1, ..., Tn) -> R: Clone
extern "ABI" fn(T1, ..., Tn, ...) -> R: Clone
unsafe extern "ABI" fn(T1, ..., Tn) -> R: Clone
unsafe extern "ABI" fn(T1, ..., Tn, ...) -> R: Clone


f  fn item
----------------------------------------------------
typeof(f): Clone

[kennytm]

Missing a T: Copy bound?

[arielb1]

A T: Clone rule

[arielb1]

You may want to add rules for function types and function pointers (with arity > 12) as well. These types already impl Copy, and thus should also impl Clone.

These were non-controversial, so I didn't include them in the RFC. Maybe I should.

[arielb1]

@kennytm Done.

[arielb1]

@comex

I don't think we are going to stay with the ICE forever - we need to do something about higher-ranked fn pointers, and in that case there's no good language reason not to also do Copy arrays and tuples.

[est31]

Any reason on why Clone should get special treatment? There are many other traits which this RFC doesn't fix like PartialEq, PartialOrd, Ord, Default, Debug, IntoIterator.

Also, what to do once we have RFCs 2000 and 1935 merged, should the special case in the compiler be removed in favour of an implementation in libcore, and if yes, why not?

I'd like if the proposal got extended to all traits which are currently lang items. The situation with this RFC won't be perfect (e.g. serde::ser::Serialize will still not work for arrays with more than 32 elements), but it would be a much better situation than we have now.

It might look ugly to add so much manual stuff to the compiler but if the RFC is intended as temporary solution until we have the proper tuple/array generics RFCs merged which then would allow us to move the logic into libcore, its worth it IMO.

[kennytm]

@est31 Clone gets special treatment because of Copy.

#2000 can get rid of the special case for arrays, and #1935 for tuples and simple function pointers, but the special cases must still stay for higher-ranked function pointers, function types, and closures if #2132 is merged.

[est31]

@kennytm I see.

Either way, I still think it would be a good idea to implement this for other lang items as well, just so that we don't have to wait on const generics.

[aturon]

Thanks, @arielb1, for this RFC! This nicely resolves a papercut, and doesn't seem to have any serious downsides. Let's start review for FCP.

@rfcbot fcp merge

[rfcbot]

Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams:

• @aturon
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

Concerns:

• why-only-clone resolved by RFC - compiler-generated Clone impls for Arrays and Tuples #2133 (comment)

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[scalexm]

@kennytm Clone indeed gets special treatment because of Copy, however as written in the RFC, the Clone impls for all Copy types will be added independently of this RFC, because of the ICEs (well actually the whole RFC was already implemented in #43690, somehow by mistake). So indeed I agree with @est31 that we should consider all lang items if possible.

[starkat99]

Omg yes please, I ran headfirst into this inconsistency just this week (manually newtyped around it). This is definitely something that needs to be done!

[eddyb]

@rfcbot concern why-only-clone

See #2133 (comment) and rust-lang/rust#44045 (comment) and the ensuing discussion.

[nikomatsakis]

So, as for the "Why only Clone?" question, I do think there is a concern there, but I don't think it has to hold up this RFC, because I think to some extent we are already committed to a disparity between Clone and the other traits. This RFC moves the line where the disparity shows up, but doesn't resolve the disparity. I would love it if we evolved (eventually) a story to completely solve the disparity -- we've got parts of this story, but not the whole thing. Let me explain in detail, see what you think.

How we already have a disparity: We already say that Copy: Clone. We also say that all function pointers are Copy, as are all tuples types that consist only of copy things, fixed-length array types with copyable element type, etc. Those types must all be Clone -- at present they are not, and so we get an ICE. If we fix just those cases, it will already be true that Clone works for types like [u8; 61412] where PartialEq does not, so we will already have a disparity between clone and partial-eq.

(In retrospect, maybe we should have moved all copy impls out of the compiler. Then there would be no disparity, all things would be equally bad. Ah well, water under the bridge.)

How this RFC changes the line: However, under this RFC, we would have a somewhat bigger disparity, in that Clone would work for types like [String; 61412] as well. The complete list of things where the disparity would exist is as follows, I believe:

• Fixed-length arrays of large length (e.g., [String; 61412]);
• Tuples of large arity (e.g., (String, ..., String));
• Function pointer types with bound regions (e.g., fn(&u8)).

How we could resolve this disparity: We have a number of ideas floating that can help. Const generics should enable one to write a PartialEq impl that applies to arbitrary length arrays. Variadics should resolve tuples of large arity. However, we don't have a plan that I know of around function pointer types with bound regions -- the only idea I've seen might be to have some built-in (and fundamental) trait FunctionPointer that is true for all function pointer types, so one can write a impl<T: FunctionPointer> PartialEq for T impl. This would probably require various improvements to specialization to make work.

What are our options? So it seems to me that we are stuck with a disparity between Clone and PartialEq for now. We can choose for that disparity to be limited to Copy types, or it can be extended to Clone types. The latter is strictly more convenient, obviously, and we have plans to eventually close it almost completely. I don't see a good reason to pick the former, personally.

(I feel like teaching etc would not be severely impacted by this: either way, in some cases you have to explain that Clone is a bit special, and the other traits haven't caught up yet; this is not something I would expect users to hit very early on, with the possible exception of arrays of large length, which thankfully is the one with the most solid plan for being resolved.)

[Diggsey]

It seems like what's really wanted is a blanket impl<T: Copy> Clone for T - at the risk of opening a new can of worms, could this problem be addressed via changes to the coherence rules to make that possible? Since it's in core, it could rely on an unstable feature, like #[fundamental].

[scalexm]

@Diggsey it would be a solution to fix the ICEs indeed (and one that @nikomatsakis wanted to explore at some point) although another one was preferred (for the moment), but it would not address the main topics of the RFC, that is types like [String; 2] or (String, String, String, String, String, String, String, String, String) do no implement Clone.

[nikomatsakis]

@Diggsey see my long series of blog posts on the topic ;) regardless, I do still believe we can enable that eventually through specialization, yes, though things are .. complex.

That said, I don't actually think it has all that much to do with this RFC. Or, to put it another way, that's effectively an implement detail -- @scalexm has implemented something that behaves analogously already, but it also went further, and implemented Clone for a broader set of types beyond those that are just Copy. We do have plans for how that might be achieved in a more general way for all traits, but those plans are not fully realized. Hence this RFC and ensuing discussion.

[nikomatsakis]

So I talked to @eddyb on IRC, and it seemed like we reached an agreement that he was OK with this RFC, in combination with #2132, so long as:

• We "frame it" as: the compiler supplies Clone impls for compiler-synthesized types. Or, equivalently, for structural types + closures (which are almost structural). In other words, his primary concern that kicked off a lot of this discussion was that the compiler was growing code for generating impls in MIR that it didn't really need, but if we are to accept Copy/Clone Closures #2132, that code is needed, and he wants to be sure that there is logical way to draw the boundaries for what code the compiler generates and what code requires hand-written impls.
• We are confident that the auto-generated impl will be equivalent to what we can do with const generics. I think we decided that we are (@scalexm and @arielb1 can confirm).

[eddyb]

Also I should say that with variadic generics (or even today!), I like the possibility of reusing tuple impls for closures - if (T, U, V): Hash then so should a closure with 3 captures (cc @cramertj).

[eddyb]

@rfcbot resolved why-only-clone

[nikomatsakis]

@eddyb woah -- i.e., imagining that the compiler generates a bridge impl of that kind (but for every trait)? That's a really interesting idea. I'm keen on it.

EDIT: Getting less keen as I think more about it. =) Don't want to expose ordering of the data in a closure's environment, for one thing, and also wary of having surprising traits implemented for closures. But it'd definitely be cool to allow users to implement traits for closures more easily.

[cramertj]

+1 for the bridge impl (I'm imagining in let x = 5; let y = || println!("{:?}", x);, the definition of y is something like struct SomeAnonClosure((&i32)); impl Clone for SomeAnonClosure { fn clone(&self) -> Self { self.0.clone() } }).

However, I don't want to limit the number of captures in a clone-able closure, so then the closure stuff is blocked on this change (making tuples of all sizes Clone/Copyable).

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[aturon]

This RFC has been merged! Tracking issue.

Thanks @arielb1!

[vitiral]

@arielb1 the rendered link is now broken