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

make core::str::next_code_point work on arbitrary iterator #33896

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

strake
Copy link
Contributor

@strake strake commented May 27, 2016

No description provided.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

These are in theory internal only functions that are only exported as an implementation detail between libcore/libcollections, but does this change mean that you're thinking of using them externally?

@strake
Copy link
Contributor Author

strake commented May 27, 2016

I was considering to use next_code_point in some code i'm writing, but since recognized these functions assume valid UTF-8, and think i may rather need a function to return 0xFFFD for invalid input; see this PR. The comment in next_code_point says "performance is sensitive to the exact formulation here" so i doubt you would want these modified to do so. Nonetheless, i saw these functions only need an arbitrary Iterator or DoubleEndedIterator rather than a [u8]::Iter, and i think if we have a such a decoder in libcore it would make sense to also have a version assuming valid input.

@aturon
Copy link
Member

aturon commented May 31, 2016

This seems fine to me -- thanks for the PR!

@bors: r+

@bors
Copy link
Contributor

bors commented May 31, 2016

📌 Commit db84fc1 has been approved by aturon

@bors
Copy link
Contributor

bors commented May 31, 2016

⌛ Testing commit db84fc1 with merge d1c1a4c...

@bors
Copy link
Contributor

bors commented May 31, 2016

💔 Test failed - auto-linux-64-cross-armhf

@alexcrichton
Copy link
Member

@bors: retry

On Tue, May 31, 2016 at 3:42 PM, bors notifications@github.com wrote:

💔 Test failed - auto-linux-64-cross-armhf
http://buildbot.rust-lang.org/builders/auto-linux-64-cross-armhf/builds/468


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#33896 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAD95GqenHw8Yt8KqPHZJPHPL2Amddgcks5qHLk8gaJpZM4IoHy_
.

Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 1, 2016
make core::str::next_code_point work on arbitrary iterator
bors added a commit that referenced this pull request Jun 1, 2016
Rollup of 11 pull requests

- Successful merges: #33385, #33606, #33841, #33892, #33896, #33915, #33921, #33967, #33970, #33973, #33977
- Failed merges:
@bors bors merged commit db84fc1 into rust-lang:master Jun 1, 2016
@strake strake deleted the next_code_point branch October 29, 2017 08:54
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.

5 participants