From d69cd0a0adafbaffe33f62f3c38f65d272da1847 Mon Sep 17 00:00:00 2001 From: ubsan Date: Mon, 29 Feb 2016 17:41:42 -0800 Subject: [PATCH 1/6] First commit --- text/0000-copy-clone-semantics.md | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 text/0000-copy-clone-semantics.md diff --git a/text/0000-copy-clone-semantics.md b/text/0000-copy-clone-semantics.md new file mode 100644 index 00000000000..af9f342c0c0 --- /dev/null +++ b/text/0000-copy-clone-semantics.md @@ -0,0 +1,52 @@ +- Feature Name: N/A +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +With specialization on the way, we need to talk about the semantics of +`::clone() where T: Copy`. + +It's generally been an unspoken rule of Rust that a `clone` of a `Copy` type is +equivalent to a `memcpy` of that type; however, that fact is not documented +anywhere. This fact should be in the documentation for the `Clone` trait, just +like the fact that `T: Eq` should implement `a == b == c == a` rules. + +# Motivation +[motivation]: #motivation + +Currently, `Vec::clone()` is implemented by creating a new `Vec`, and then +cloning all of the elements from one into the other. This is slow in debug mode, +and may not always be optimized (although it often will be). Specialization +would allow us to simply `memcpy` the values from the old `Vec` to the new +`Vec`. However, if we don't actually specify this, we will not be able to do +this. + +# Detailed design +[design]: #detailed-design + +Simply add something like the following sentence to the documentation for the +`Clone` trait: + +"If `T: Copy`, `x: T`, and `y: &T`, then `let x = y.clone()` is equivalent to +`let x = *y`;" + +# Drawbacks +[drawbacks]: #drawbacks + +This is a breaking change, technically, although it breaks code that was +malformed in the first place. + +# Alternatives +[alternatives]: #alternatives + +The alternative is that, for each type and function we would like to specialize +in this way, we document this separately. This is how we started off with +`clone_from_slice`. + +# Unresolved questions +[unresolved]: #unresolved-questions + +What the exact sentence should be. From 5542c80869c290ea7d371f26564fd83ff05fd5d8 Mon Sep 17 00:00:00 2001 From: ubsan Date: Mon, 29 Feb 2016 17:48:48 -0800 Subject: [PATCH 2/6] Add some modifications suggested by @durka --- text/0000-copy-clone-semantics.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/0000-copy-clone-semantics.md b/text/0000-copy-clone-semantics.md index af9f342c0c0..4bc8e906f9b 100644 --- a/text/0000-copy-clone-semantics.md +++ b/text/0000-copy-clone-semantics.md @@ -21,8 +21,8 @@ Currently, `Vec::clone()` is implemented by creating a new `Vec`, and then cloning all of the elements from one into the other. This is slow in debug mode, and may not always be optimized (although it often will be). Specialization would allow us to simply `memcpy` the values from the old `Vec` to the new -`Vec`. However, if we don't actually specify this, we will not be able to do -this. +`Vec` in the case of `T: Copy`. However, if we don't specify this, we will not +be able to, and we will be stuck looping over every value. # Detailed design [design]: #detailed-design @@ -30,8 +30,8 @@ this. Simply add something like the following sentence to the documentation for the `Clone` trait: -"If `T: Copy`, `x: T`, and `y: &T`, then `let x = y.clone()` is equivalent to -`let x = *y`;" +"If `T: Copy`, `x: T`, and `y: &T`, then `let x = y.clone();` is equivalent to +`let x = *y;`. Manual implementations must be careful to uphold this." # Drawbacks [drawbacks]: #drawbacks From 91629f9a72fcf22e04d0eea67a1850b7258453a8 Mon Sep 17 00:00:00 2001 From: ubsan Date: Mon, 29 Feb 2016 17:56:59 -0800 Subject: [PATCH 3/6] Fix some language for @aatch --- text/0000-copy-clone-semantics.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/text/0000-copy-clone-semantics.md b/text/0000-copy-clone-semantics.md index 4bc8e906f9b..fbfca12f061 100644 --- a/text/0000-copy-clone-semantics.md +++ b/text/0000-copy-clone-semantics.md @@ -27,7 +27,10 @@ be able to, and we will be stuck looping over every value. # Detailed design [design]: #detailed-design -Simply add something like the following sentence to the documentation for the +Specify that `::clone(t)` shall be equivalent to `ptr::read(t)` +where `T: Copy, t: &T`. + +Also add something like the following sentence to the documentation for the `Clone` trait: "If `T: Copy`, `x: T`, and `y: &T`, then `let x = y.clone();` is equivalent to From 0feac231b3f82c6d954c3e2a2f7637b099c31ae1 Mon Sep 17 00:00:00 2001 From: ubsan Date: Mon, 29 Feb 2016 21:50:27 -0800 Subject: [PATCH 4/6] A poorly defined clone shan't result in UB --- text/0000-copy-clone-semantics.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0000-copy-clone-semantics.md b/text/0000-copy-clone-semantics.md index fbfca12f061..066777860ba 100644 --- a/text/0000-copy-clone-semantics.md +++ b/text/0000-copy-clone-semantics.md @@ -28,7 +28,8 @@ be able to, and we will be stuck looping over every value. [design]: #detailed-design Specify that `::clone(t)` shall be equivalent to `ptr::read(t)` -where `T: Copy, t: &T`. +where `T: Copy, t: &T`. An implementation that does not uphold this *shall not* +result in undefined behavior; `Clone` is not an `unsafe trait`. Also add something like the following sentence to the documentation for the `Clone` trait: @@ -52,4 +53,4 @@ in this way, we document this separately. This is how we started off with # Unresolved questions [unresolved]: #unresolved-questions -What the exact sentence should be. +What the exact wording should be. From df87d91bf8c2d1c38bec66e83fad3f0e57c2036d Mon Sep 17 00:00:00 2001 From: ubsan Date: Tue, 1 Mar 2016 18:51:54 -0800 Subject: [PATCH 5/6] Add more motivation --- text/0000-copy-clone-semantics.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/text/0000-copy-clone-semantics.md b/text/0000-copy-clone-semantics.md index 066777860ba..5dacfa31bb0 100644 --- a/text/0000-copy-clone-semantics.md +++ b/text/0000-copy-clone-semantics.md @@ -24,6 +24,13 @@ would allow us to simply `memcpy` the values from the old `Vec` to the new `Vec` in the case of `T: Copy`. However, if we don't specify this, we will not be able to, and we will be stuck looping over every value. +It's always been the intention that `Clone::clone == ptr::read for T: Copy`; see +[issue #23790][issue-copy]: "It really makes sense for `Clone` to be a +supertrait of `Copy` -- `Copy` is a refinement of `Clone` where `memcpy` +suffices, basically." This idea was also implicit in accepting +[rfc #0839][rfc-extend] where "[B]ecause Copy: Clone, it would be backwards +compatible to upgrade to Clone in the future if demand is high enough." + # Detailed design [design]: #detailed-design @@ -54,3 +61,6 @@ in this way, we document this separately. This is how we started off with [unresolved]: #unresolved-questions What the exact wording should be. + +[issue-copy]: https://github.com/rust-lang/rust/issues/23790 +[rfc-extend]: https://github.com/rust-lang/rfcs/blob/master/text/0839-embrace-extend-extinguish.md From b9b78547b0693af126c288adbc51a4fa2240008f Mon Sep 17 00:00:00 2001 From: ubsan Date: Tue, 1 Mar 2016 18:52:31 -0800 Subject: [PATCH 6/6] Add a date --- text/0000-copy-clone-semantics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-copy-clone-semantics.md b/text/0000-copy-clone-semantics.md index 5dacfa31bb0..0df568c4963 100644 --- a/text/0000-copy-clone-semantics.md +++ b/text/0000-copy-clone-semantics.md @@ -1,5 +1,5 @@ - Feature Name: N/A -- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Start Date: 01 March, 2016 - RFC PR: (leave this empty) - Rust Issue: (leave this empty)