Some work on std::ascii #9059
Some work on std::ascii #9059
Conversation
Kimundi
commented
Sep 8, 2013
Kimundi
commented
- Marked a unsafe helper function as unsafe, added a second helper function
- Added moving implementations
| unsafe fn str_map_bytes(mut string: ~str, map: &'static [u8]) -> ~str { | ||
| // FIXME: #9058 Assuming this is faster than composed Iterators. | ||
| // If not, should probably rewrite. | ||
| do string.as_mut_buf |mut buf, len| { |
There was a problem hiding this comment.
This might be cleaner if one converted the string to ~[u8] and just used .mut_iter(); rather than pointer dances.
There was a problem hiding this comment.
It would be indeed, but the function I updated there used pointer arithmetic for what I assumed to be speed reasons. I don't really know how to test such things, so I left the basic structure of the function as is.
There was a problem hiding this comment.
The iterators are designed to optimize very well (they use pointers arithmetic internally); so that gives you the benefit of no unsafe code and equal (or faster) speed.
There was a problem hiding this comment.
I could just replace both str_map_bytes and str_copy_map_bytes with iterator chains if you think that would be better.
str_copy_map_bytes originally got introduced in #8231 under the name map_bytes. @SimonSapin: Any reason why you choose pointer arithmetic?
There was a problem hiding this comment.
I wasn’t aware of vec.mut_iter(). How do you get a mutable [u8] from a string?
There was a problem hiding this comment.
In this case, I think string.into_bytes() to get a ~[u8] which can be placed into a mut slot. (And then str::from_bytes_owned() at the end.)
(And yes; I think keeping the amount of unsafe code as small as possible is a good goal: less code to audit, and if there is a bug found then it'll only need to be changed in one spot.)
There was a problem hiding this comment.
I think you’d want str::raw::from_bytes_owned to avoid the UTF-8 check. (This is safe when the map only changes bytes in the ASCII range.)
There was a problem hiding this comment.
Okay, gonna rewrite the functions to only use unsafe to circumvent the utf8 checks.
- Marked a unsafe helper function as unsafe, added a second helper function - Added moving implementations
…lexendoo [`manual_retain`]: add lint case for `binary_heap` Closes rust-lang#9059 This PR adds changes to perform linting on the `binary_heap` as well, under the [manual_retain rule](https://rust-lang.github.io/rust-clippy/master/index.html#/manual_retain). changelog: [manual_retain]: update for lint `binary_heap`
