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

Specialize methods on iter::Cloned<I> where I::Item: Copy. #39022

Closed
wants to merge 1 commit into from

Conversation

Stebalien
Copy link
Contributor

@Stebalien Stebalien commented Jan 13, 2017

Instead of cloning a bunch of copyable types only to drop them (in nth, last, and count), take advantage of RFC 1521 (Copy clone semantics) and don't bother cloning them in the first place (directly call nth, last, and count on the wrapped iterator). If the wrapped iterator optimizes these methods, Cloned now inherits this optimization.

This is a retry of #36791. It was blocked waiting for fixes for #36053 and #36848 to make it into a beta snapshot.

Instead of cloning a bunch of copyable types only to drop them (in `nth`,
`last`, and `count`), take advantage of rust-lang#1521 (Copy clone semantics) and don't
bother cloning them in the first place (directly call `nth`, `last`, and `count`
on the wrapped iterator). If the wrapped iterator optimizes these methods,
`Cloned` now inherits this optimization.
@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

@Stebalien
Copy link
Contributor Author

Stebalien commented Jan 13, 2017

As per the last attempt, cc @rust-lang/libs, @bluss. Also, please wait for travis.

(*edit* apparently I can't call teams :( )

@Mark-Simulacrum
Copy link
Member

#1521 seems to be the wrong issue?

@nagisa
Copy link
Member

nagisa commented Jan 13, 2017

See #28125. I::Item: Copy does not imply Clone has no side effects.

Would be a shame to block such lib-opts on that though, but it has already happened.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Jan 13, 2017

Since #28125 was closed, RFC 1521 (most likely what @Stebalien meant by "#1521") has been written and accepted, which does allow library code to assume Clone has no side effects for Copy types.

@nagisa
Copy link
Member

nagisa commented Jan 13, 2017

Aw, right; totally forgot about that.

@Stebalien
Copy link
Contributor Author

@rkruppe Yes. Sorry about that @Mark-Simulacrum.

@alexcrichton
Copy link
Member

The error on Travis seems worrisome?

@Stebalien
Copy link
Contributor Author

If nothing else, this PR is a good way to find bugs... Closed waiting on #39025.

@Stebalien Stebalien closed this Jan 13, 2017
@Stebalien
Copy link
Contributor Author

Actually, that's just a dup of #36804.

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.

6 participants