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

Remove unzip() SizeHint hack #34155

Merged
merged 1 commit into from
Jun 21, 2016
Merged

Remove unzip() SizeHint hack #34155

merged 1 commit into from
Jun 21, 2016

Conversation

ollie27
Copy link
Member

@ollie27 ollie27 commented Jun 7, 2016

This was using an invalid iterator so is likely to end with buggy
behaviour.

It also doesn't even benefit many type in std including Vec so removing it
shouldn't cause any problems.

Fixes: #33468

This was using an invalid iterator so is likely to end with buggy
behaviour.

It also doesn't even benefit many type in std including Vec so removing it
shouldn't cause any problems.
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@alexcrichton
Copy link
Member

I believe this was originally used to preallocate space in a Vec, but the implementation of extend since changed to not take advantage of this.

The performance of unzip doesn't seem necessarily that important, so I'd be fine doing so. Curious what others on @rust-lang/libs think though.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 8, 2016
@shepmaster
Copy link
Member

I think @bluss will be excited to see this go!

@bluss
Copy link
Member

bluss commented Jun 9, 2016

Yes! Cool hack, but it breaks the rules of an Iterator impl.

@bluss
Copy link
Member

bluss commented Jun 9, 2016

A note from somewhere we discussed this previously: We can do a much better job with specialization anyway.

@brson
Copy link
Contributor

brson commented Jun 10, 2016

+1

@alexcrichton
Copy link
Member

The libs team discussed this PR during triage today and the conclusion was to merge. Thanks again @ollie27!

@alexcrichton
Copy link
Member

@bors: r+ 02f9be8

@bors
Copy link
Contributor

bors commented Jun 21, 2016

⌛ Testing commit 02f9be8 with merge fe96928...

bors added a commit that referenced this pull request Jun 21, 2016
Remove unzip() SizeHint hack

This was using an invalid iterator so is likely to end with buggy
behaviour.

It also doesn't even benefit many type in std including Vec so removing it
shouldn't cause any problems.

Fixes: #33468
@bors bors merged commit 02f9be8 into rust-lang:master Jun 21, 2016
@ollie27 ollie27 deleted the unzip branch June 21, 2016 16:13
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.

8 participants