Skip to content

Revert "PR #30130 Implement Clone for more arrays" #30247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 7, 2015

Conversation

bluss
Copy link
Member

@bluss bluss commented Dec 6, 2015

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.

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.
@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@bluss
Copy link
Member Author

bluss commented Dec 6, 2015

r? @alexcrichton

I want to be conservative, no unplanned regressions.

@rust-highfive rust-highfive assigned alexcrichton and unassigned aturon Dec 6, 2015
@bluss
Copy link
Member Author

bluss commented Dec 6, 2015

cc @tbu- As the original author.

@sfackler sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 6, 2015
@tbu-
Copy link
Contributor

tbu- commented Dec 7, 2015

Yea, revert this for the time being. Theoretically these arrays ought to implement Clone as well, but that needs some compiler magic to happen.

@untitaker
Copy link
Contributor

I propose adding the relevant broken testcase to ease future refactoring efforts.

@bluss
Copy link
Member Author

bluss commented Dec 7, 2015

@untitaker good idea, added test.

@alexcrichton
Copy link
Member

@bors: r+ 0ca2a9e

I'll make sure we discuss this at the next libs meeting, however. For now seems prudent to head off the breakage.

bors added a commit that referenced this pull request Dec 7, 2015
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.
@bors
Copy link
Collaborator

bors commented Dec 7, 2015

⌛ Testing commit 0ca2a9e with merge e819d8a...

@bors bors merged commit 0ca2a9e into rust-lang:master Dec 7, 2015
@bluss bluss deleted the revert-array-clone branch December 7, 2015 05:21
@bors bors mentioned this pull request Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clone is not defined for [_; 256]
8 participants