Avoid re-validating UTF-8 in FromUtf8Error::into_utf8_lossy
#130408
Conversation
|
r? @Noratrieb rustbot has assigned @Noratrieb. Use |
rustbot
added
S-waiting-on-review
| let iter = self.bytes[self.error.valid_up_to()..].utf8_chunks(); | ||
|
|
||
| for chunk in iter { | ||
| res.push_str(chunk.valid()); | ||
| if !chunk.invalid().is_empty() { | ||
| res.push_str(REPLACEMENT); | ||
| } | ||
| } |
There was a problem hiding this comment.
Following the logic from String::from_utf8_lossy
library/alloc/src/string.rs
Outdated
|
|
||
| // `Utf8Error::valid_up_to` returns the maximum index of validated | ||
| // UTF-8 bytes. Copy the valid bytes into the output buffer. | ||
| v.extend(self.bytes[..self.error.valid_up_to()].iter().copied()); |
There was a problem hiding this comment.
can you use extend_from_slice here? that's both simpler and possible more efficient (though the iterator probably produces good code as well? but either way, extend_from_slice is shorter)
There was a problem hiding this comment.
Sure, I normally use extend_from_slice and second-guessed myself since I couldn't remember if there was a difference.
I checked the docs and went with this because of the last sentence here but that's maybe a bit hopeful still.
Note that this function is same as
extendexcept that it is specialized to work with slices instead. If and when Rust gets specialization this function will likely be deprecated (but still available).
https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.extend_from_slice
library/alloc/tests/string.rs
Outdated
| assert_eq!(func(xs), ys); | ||
|
|
||
| let xs = b"Hello\xC2 There\xFF Goodbye"; | ||
| assert_eq!(func(xs), String::from("Hello\u{FFFD} There\u{FFFD} Goodbye")); |
There was a problem hiding this comment.
nit (you don't have to change this but can if you're touching it anyways and want to): you've used .to_owned() above but use String::from here, i prefer .to_owned() myself so they could be made consistent
There was a problem hiding this comment.
Addressed.
I just copied the test above but I prefer that too.
Refactor `into_utf8_lossy` to copy valid UTF-8 bytes into the buffer, avoiding double validation of bytes. Add tests that mirror the `String::from_utf8_lossy` tests
okaneco
force-pushed
the
into_lossy_refactor
branch
from
a9b04b3
to
b94c5a1
Compare
|
Force pushed |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Part of the unstable feature
string_from_utf8_lossy_owned- #129436Refactor
FromUtf8Error::into_utf8_lossyto copy valid UTF-8 bytes into the buffer, avoiding double validation of bytes.Add tests that mirror the
String::from_utf8_lossytests.