Skip to content

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.

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
Collaborator

bors commented Apr 10, 2017

📌 Commit a2b28be has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge dc93ce2...

@bors
Copy link
Collaborator

bors commented Apr 11, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 11, 2017

@bors
Copy link
Collaborator

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge 478d3b2...

@bors
Copy link
Collaborator

bors commented Apr 11, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 11, 2017

@bors
Copy link
Collaborator

bors commented Apr 11, 2017

⌛ Testing commit a2b28be with merge 2137a14...

@bors
Copy link
Collaborator

bors commented Apr 11, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 11, 2017

@bors
Copy link
Collaborator

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
Collaborator

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