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.

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

Merged
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

r? @alexcrichton

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

@@ -1746,6 +1746,14 @@ fn to_uppercase() {
assert_eq!("aéDžßfiᾀ".to_uppercase(), "AÉDŽSSFIἈΙ");
}

#[test]
fn test_into_string() {
// The only way to acquite a Box<str> in the first place is through a String, so just
Copy link
Contributor

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.

@@ -741,6 +742,24 @@ impl String {
string: self_ptr,
}
}

/// Converts the string into Box<str>.
Copy link
Member

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
Member

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

unsafe {
String::from_raw_parts(Box::into_raw(self) as *mut u8, len, len)
}
Copy link
Member

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

@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

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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants