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

Optimise Chars::last() #37882

Merged
merged 2 commits into from Nov 20, 2016

Conversation

Projects
None yet
7 participants
@ollie27
Contributor

ollie27 commented Nov 19, 2016

The default implementation of last() goes through the entire iterator
but that's not needed here.

Optimise Chars::last()
The default implementation of last() goes through the entire iterator
but that's not needed here.
@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Nov 19, 2016

Collaborator

r? @aturon

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

Collaborator

rust-highfive commented Nov 19, 2016

r? @aturon

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

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Nov 19, 2016

Contributor

@bors r+ rollup

Thank you!

Contributor

bluss commented Nov 19, 2016

@bors r+ rollup

Thank you!

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Nov 19, 2016

Contributor

📌 Commit de2f617 has been approved by bluss

Contributor

bors commented Nov 19, 2016

📌 Commit de2f617 has been approved by bluss

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Nov 19, 2016

Contributor

By the way, this applies to char_indices as well. Another commit or another PR is welcome.

Contributor

bluss commented Nov 19, 2016

By the way, this applies to char_indices as well. Another commit or another PR is welcome.

Optimise CharIndices::last()
The default implementation of last() goes through the entire iterator
but that's not needed here.
@ollie27

This comment has been minimized.

Show comment
Hide comment
@ollie27

ollie27 Nov 20, 2016

Contributor

Good point. I've applied the same to CharIndices.

Contributor

ollie27 commented Nov 20, 2016

Good point. I've applied the same to CharIndices.

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Nov 20, 2016

Contributor

@bors r+ rollup

Contributor

bluss commented Nov 20, 2016

@bors r+ rollup

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Nov 20, 2016

Contributor

📌 Commit 9e86e18 has been approved by bluss

Contributor

bors commented Nov 20, 2016

📌 Commit 9e86e18 has been approved by bluss

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 20, 2016

Rollup merge of rust-lang#37882 - ollie27:chars_last, r=bluss
Optimise Chars::last()

The default implementation of last() goes through the entire iterator
but that's not needed here.

bors added a commit that referenced this pull request Nov 20, 2016

Auto merge of #37896 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 8 pull requests

- Successful merges: #37835, #37840, #37841, #37848, #37876, #37880, #37881, #37882
- Failed merges:

@bors bors merged commit 9e86e18 into rust-lang:master Nov 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ollie27 ollie27 deleted the ollie27:chars_last branch Nov 20, 2016

@brson brson added the relnotes label Nov 22, 2016

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka Nov 25, 2016

Contributor

Shouldn't this apply to any DoubleEndedIterator?

Contributor

durka commented Nov 25, 2016

Shouldn't this apply to any DoubleEndedIterator?

@ollie27

This comment has been minimized.

Show comment
Hide comment
@ollie27

ollie27 Nov 25, 2016

Contributor

Yeah, but I think it might be breaking change to do this for iterators that might have side effects.

Contributor

ollie27 commented Nov 25, 2016

Yeah, but I think it might be breaking change to do this for iterators that might have side effects.

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Nov 25, 2016

Contributor

Yes. Iterator::last's doc is specific about how it goes about finding the last value (traversing it all).

Contributor

bluss commented Nov 25, 2016

Yes. Iterator::last's doc is specific about how it goes about finding the last value (traversing it all).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment