Skip to content

Add String::into_boxed_slice and Box<str>::into_string #26931

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
Merged
merged 1 commit into from
Jul 12, 2015
Merged

Add String::into_boxed_slice and Box<str>::into_string #26931

merged 1 commit into from
Jul 12, 2015

Conversation

@reem
Copy link
Contributor

@reem reem commented Jul 10, 2015

Implements merged RFC 1152.

Closes #26697.

@rust-highfive
Copy link
Contributor

rust-highfive commented Jul 10, 2015

r? @alexcrichton

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

mitaa
mitaa reviewed Jul 10, 2015
View reviewed changes
src/libcollectionstest/str.rs Outdated
Copy link
Contributor

@mitaa mitaa Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acquite acquire

@reem reem force-pushed the string-conversions branch 2 times, most recently from f8b9f64 to 17c914e Compare July 10, 2015 08:56
@reem
Copy link
Contributor Author

reem commented Jul 10, 2015

Responded to feedback in the newest commit. Updated a typo, and use Box::into_raw.

bluss
bluss reviewed Jul 10, 2015
View reviewed changes
src/libcollections/string.rs Outdated
Copy link
Contributor

@bluss bluss Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: backticks

@reem reem force-pushed the string-conversions branch 2 times, most recently from 1520bc0 to 3509b13 Compare July 10, 2015 09:18
@bluss
Copy link
Contributor

bluss commented Jul 10, 2015

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 10, 2015

📌 Commit 3509b13 has been approved by bluss

@bluss bluss assigned bluss and unassigned alexcrichton Jul 10, 2015
alexcrichton
alexcrichton reviewed Jul 10, 2015
View reviewed changes
src/libcollections/str.rs Outdated
Copy link
Member

@alexcrichton alexcrichton Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be best implemented compositionally with a transmute, <[_]>::into_vec plus String::from_utf8_unchecked, that way there's no need to worry about the raw parts being exactly what we want, and there's exactly one point of Box<[T]> to Vec<T> (into_vec)

@alexcrichton
Copy link
Member

alexcrichton commented Jul 10, 2015

@bors: r- (just some minor nits about the implementation, but follow ups are hard...)

@reem reem force-pushed the string-conversions branch from 3509b13 to 69521af Compare July 12, 2015 04:32
@reem
Copy link
Contributor Author

reem commented Jul 12, 2015

@alexcrichton updated with suggested refactor - I actually thought of doing it that way the first time, but decided on the more manual route to avoid transmuting Box<[u8]> to Box<str>.

@alexcrichton
Copy link
Member

alexcrichton commented Jul 12, 2015

@bors: r+ 69521af

Thanks!

@bors
Copy link
Collaborator

bors commented Jul 12, 2015

⌛ Testing commit 69521af with merge 88c1105...

bors added a commit that referenced this pull request Jul 12, 2015
@bors
Auto merge of #26931 - reem:string-conversions, r=alexcrichton
88c1105 
Implements merged RFC 1152.

Closes #26697.
@bors
Copy link
Collaborator

bors commented Jul 12, 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 69521af into rust-lang:master Jul 12, 2015
@bluss
Copy link
Contributor

bluss commented Aug 12, 2015

I want to rename this method, see Rename String::into_boxed_slice -> into_boxed_str #27696

@reem reem deleted the string-conversions branch August 13, 2015 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@bluss bluss

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@reem @rust-highfive @bluss @bors @alexcrichton @mitaa