Skip to content
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

Implement Clone for IntoIter where A: Clone #191

Closed
wants to merge 3 commits into from

Conversation

L0uisc
Copy link
Contributor

@L0uisc L0uisc commented Dec 12, 2019

Implements Clone for IntoIter<A> where A: Array + Clone and A::Item: Copy (necessary to be able to use from_slice()) as per #178.

I also left a commented implementation which requires A::Item to be the more relaxed Clone, which also works. However, it will be less efficient than the other impl when the types are Copy because it can't use from_slice(). I believe the feature I'm looking for is specialization?

@L0uisc L0uisc closed this Dec 12, 2019
@L0uisc L0uisc deleted the clone-into-iter branch December 12, 2019 20:55
@mbrubeck
Copy link
Collaborator

Ah, I forgot that from_slice requires Copy.

I think we should use the more-general, less-performant version for now, and when specialization is available we can use it to optimize the Copy case.

If you use SmallVec::from(slice) instead of SmallVec::from_slice(slice) then this should work for Clone types, and when the "specialization" Cargo feature is enabled it will use from_slice when possible.

@L0uisc L0uisc restored the clone-into-iter branch December 12, 2019 21:00
@L0uisc
Copy link
Contributor Author

L0uisc commented Dec 12, 2019

Sorry, I'm trying out Visual Studio's git plugin, and it seems I'm doing things wrong...

@L0uisc L0uisc deleted the clone-into-iter branch December 12, 2019 21:12
@L0uisc
Copy link
Contributor Author

L0uisc commented Dec 12, 2019

#192 should be correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants