Implement Clone for more arrays
#30130
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| @@ -85,6 +85,7 @@ | |||
| #![feature(unwind_attributes)] | |||
| #![cfg_attr(stage0, feature(simd))] | |||
| #![cfg_attr(not(stage0), feature(repr_simd, platform_intrinsics))] | |||
| #![feature(slice_patterns)] | |||
There was a problem hiding this comment.
We've historically had a lot of codegen issues with this in the past, so I'm somewhat wary to enable this to implement this for now. Can this be implemented without this feature?
|
Making empty array This was possible to implement without using slice patterns, but only in a weird way. |
tbu-
changed the title
Clone for [T; 0] to [T; 32] if T: CloneClone for more arrays and make empty array Copyable
| impl<T: Clone> Clone for [T; $n] { | ||
| fn clone(&self) -> [T; $n] { | ||
| let temp = [&self[$i], $(&self[$idx]),*]; | ||
| [temp[$i].clone(), $(temp[$idx].clone()),*] |
There was a problem hiding this comment.
I think you can make this a bit simpler:
fn clone(&self) -> [T; $n] {
[temp[$i-$i].clone(), $(temp[$i-$idx].clone()),*]
}
There was a problem hiding this comment.
Good idea, thank you. For some reason I couldn't figure out how to reverse the indices.
bluss
added
the
relnotes
|
It's a breaking change, but only for relatively broken code. It's simply that copyable elements have .clone() called now too, so it's a user-observable change. I think we can say that any code that breaks was already buggy (due to clone and copy not being equivalent). Example: #[derive(Copy)]
struct Foo;
impl Clone for Foo { fn clone(&self) -> Self { panic!() } }
// Will panic in new code, used to be “fine.”
[Foo].clone(); Edit / Note behaviourial changes are not actutally covered by our API evolution RFC. Anyway, this comment is mostly due diligence: We need to understand each behaviour change when we appove of it. |
|
Hm upon reflection I'm a little less cure about special casing a 0-length array, for example do we special case that for Also yeah technically this could break code, but I can't imagine anyone actually relying on that in practice. You're likely to have way more surprising results than this if your |
|
@alexcrichton Should I split that out into a different PR again? |
|
Hm yeah, sorry for the roundabout there! |
|
@alexcrichton Split it up again. |
tbu-
changed the title
Clone for more arrays and make empty array CopyableClone for more arrays
|
⌛ Testing commit 6dff9d0 with merge 62eb243... |
|
💔 Test failed - auto-mac-64-opt |
|
@bors: retry |
|
I'm not sure why, but it seems that this PR broke compilation of structures like #[derive(Copy, Clone)]
struct Array {
arr: [[u8; 256]; 4]
}that is, structures with |
|
@netvl That's a clear case that we didn't think about :( The reason is that |
This reverts commit e22a64e. This caused a regression such that types like `[[u8; 256]; 4]` no longer implemented Clone. This previously worked due to Clone for `[T; N]` (N in 0 to 32) being implemented for T: Copy. Due to fixed size arrays not implementing Clone for sizes above 32, the new implementation requiring T: Clone would not allow `[[u8; 256]; 4]` to be Clone.
Revert "PR #30130 Implement `Clone` for more arrays" This reverts commit e22a64e. This caused a regression such that types like `[[u8; 256]; 4]` no longer implemented Clone. This previously worked due to Clone for `[T; N]` (N in 0 to 32) being implemented for T: Copy. Due to fixed size arrays not implementing Clone for sizes above 32, the new implementation requiring T: Clone would not allow `[[u8; 256]; 4]` to be Clone. Fixes #30244 Due to changing back, this is technically a [breaking-change], albeit for a behavior that existed for a very short time.
bluss
removed
the
relnotes
[breaking-change]