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

Reduce str transmutes, add mut versions of methods. #41096

Merged
merged 1 commit into from
Apr 11, 2017

Conversation

clarfonthey
Copy link
Contributor

When I was working on the various parts involved in #40380 one of the comments I got was the excess of transmutes necessary to make the changes work. This is part of a set of multiple changes I'd like to offer to fix this problem.

I think that having these methods is reasonable because they're already possible via transmutes, and it makes the code that uses them safer. I can also add pub(crate) to these methods for now if the libs team would rather not expose them to the public without an RFC.

/// Converts a mutable string slice to a mutable byte slice.
#[unstable(feature = "str_mut_extras", issue = "0")]
#[inline(always)]
pub fn as_bytes_mut(&mut self) -> &mut [u8] {
Copy link
Member

Choose a reason for hiding this comment

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

This would have to be unsafe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I thought I annotated this as unsafe but I didn't.

@alexcrichton
Copy link
Member

These seem like reasonable additions to me!

(clear mirrors of the slice equivalents)

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 5, 2017
@alexcrichton alexcrichton self-assigned this Apr 5, 2017
@alexcrichton
Copy link
Member

Looks like travis also has failures?

@clarfonthey
Copy link
Contributor Author

Travis failures are due to a requirement for docs of the new features; I will fix those in a bit.

@clarfonthey
Copy link
Contributor Author

Also I just added a from_utf8_mut as well.

@alexcrichton
Copy link
Member

Looks like the build may still be failing?

@clarfonthey clarfonthey force-pushed the as_bytes_mut branch 4 times, most recently from d20e65e to aded772 Compare April 9, 2017 22:33
@clarfonthey
Copy link
Contributor Author

Seems to be passing now!

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 10, 2017

📌 Commit a2b28be has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge dc93ce2...

@bors
Copy link
Contributor

bors commented Apr 11, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 11, 2017

@bors
Copy link
Contributor

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge 478d3b2...

@bors
Copy link
Contributor

bors commented Apr 11, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 11, 2017

@bors
Copy link
Contributor

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge 2137a14...

@bors
Copy link
Contributor

bors commented Apr 11, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 11, 2017

@bors
Copy link
Contributor

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge c58c928...

bors added a commit that referenced this pull request Apr 11, 2017
Reduce str transmutes, add mut versions of methods.

When I was working on the various parts involved in #40380 one of the comments I got was the excess of transmutes necessary to make the changes work. This is part of a set of multiple changes I'd like to offer to fix this problem.

I think that having these methods is reasonable because they're already possible via transmutes, and it makes the code that uses them safer. I can also add `pub(crate)` to these methods for now if the libs team would rather not expose them to the public without an RFC.
@bors
Copy link
Contributor

bors commented Apr 11, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing c58c928 to master...

@bors bors merged commit a2b28be into rust-lang:master Apr 11, 2017
@clarfonthey clarfonthey deleted the as_bytes_mut branch April 11, 2017 19:40
bors added a commit that referenced this pull request Apr 26, 2017
More methods for str boxes. (reduce Box<[u8]> ↔ Box<str> transmutes)

This is a follow-up to #41096 that adds safer methods for converting between `Box<str>` and `Box<[u8]>`. They're gated under a different feature from the `&mut str` methods because they may be too niche to include in public APIs, although having them internally helps reduce the number of transmutes the standard library uses.

What's added:

* `From<Box<str>> for Box<[u8]>`
* `<Box<str>>::into_boxed_bytes` (just calls `Into::into`)
* `alloc::str` (new module)
* `from_boxed_utf8` and `from_boxed_utf8_unchecked`, defined in `alloc:str`, exported in `collections::str`
* exports `from_utf8_mut` in `collections::str` (missed from previous PR)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants