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.

Sign up for GitHub

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

Jump to bottom

Implement str::split_at (RFC 1123) #25839

Merged
merged 2 commits into from
Jun 11, 2015
Merged

Implement str::split_at (RFC 1123) #25839

merged 2 commits into from
Jun 11, 2015

Conversation

bluss
Copy link
Member

@bluss bluss commented May 27, 2015

Implement RFC rust-lang/rfcs#1123

Add str method str::split_at(mid: usize) -> (&str, &str).

Also a minor cleanup in the collections::str module. Remove redundant slicing of self.

@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. 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 CONTRIBUTING.md for more information.

@bluss
Copy link
Member Author

bluss commented May 27, 2015

Please see the RFC: rust-lang/rfcs#1123. It is not yet accepted.

Tacked on two one change that I found when I visited the file. Removing redundant slicing saves us nothing (I hope) but codegen and optimization time.

Inlining as_bytes() is just something I saw in by coincdence. Inling it should help optimizations where they're as most needed. A lot more str methods are marked inline in libcore, but I went for those that should be uncontroversial and important for optimizations. As_bytes() was handled by @Aatch in the meantime. Inlining further str methods should be a separate PR.

@alexcrichton
Copy link
Member

Looks good to me, thanks @bluss!

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 2, 2015
@bors
Copy link
Contributor

bors commented Jun 9, 2015

☔ The latest upstream changes (presumably #26065) made this pull request unmergeable. Please resolve the merge conflicts.

@bluss
Copy link
Member Author

bluss commented Jun 9, 2015

Rebased, tossed the inlining commit.

@alexcrichton alexcrichton added the B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. label Jun 9, 2015
@alexcrichton
Copy link
Member

Turning this into the tracking issue for rust-lang/rfcs#1123. Thanks again @bluss!

@bors: r+ 37bbcc3

@bluss
Copy link
Member Author

bluss commented Jun 10, 2015

☀️

@bors
Copy link
Contributor

bors commented Jun 10, 2015

⌛ Testing commit 37bbcc3 with merge 5294ccc...

@bors
Copy link
Contributor

bors commented Jun 10, 2015

💔 Test failed - auto-mac-64-opt

Ulrik Sverdrup added 2 commits June 10, 2015 09:15
Add str::split_at
d43bf53 
Implement RFC rust-lang/rfcs#1123

Add str method str::split_at(mid: usize) -> (&str, &str).
collections: Remove redundant slicing for str
1112a05 
This is a remnant from a previous implementation of the str methods.
Using `self` is fine now.
@bluss
Copy link
Member Author

bluss commented Jun 10, 2015

Fixed the doc example, sorry about that.

@alexcrichton
Copy link
Member

@bors: r+ 1112a05

@bluss
Copy link
Member Author

bluss commented Jun 10, 2015

@bors: r=alexcrichton

Let's give this another go

@bors
Copy link
Contributor

bors commented Jun 10, 2015

📌 Commit 1112a05 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jun 11, 2015

⌛ Testing commit 1112a05 with merge fbb1354...

bors added a commit that referenced this pull request Jun 11, 2015
@bors
Auto merge of #25839 - bluss:str-split-at-impl, r=alexcrichton
fbb1354 
Implement RFC rust-lang/rfcs#1123

Add str method str::split_at(mid: usize) -> (&str, &str).

Also a minor cleanup in the collections::str module. Remove redundant slicing of self.
@bors
Copy link
Contributor

bors commented Jun 11, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt

@bors bors merged commit 1112a05 into rust-lang:master Jun 11, 2015
@bluss bluss deleted the str-split-at-impl branch June 11, 2015 08:51
@huonw huonw mentioned this pull request Jan 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aturon aturon

Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bluss @rust-highfive @alexcrichton @bors @aturon