Add explicit Iterator::count impl for str::EncodeUtf16#159467
Conversation
|
r? @Darksonn rustbot has assigned @Darksonn. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| #[inline] | ||
| fn count(self) -> usize { |
There was a problem hiding this comment.
I don’t know if this should be #[inline], for now it follows the example of other methods in this impl block
In the early days of Unicode, many systems adopted it by storing in-memory strings in (roughly) `Vec<u16>`, encoded as UCS-2 / UTF-16 / WTF-16. The “natural” way to index such strings or measure their length is counting UTF-16 code units. In some cases, such lengths or offsets found their way in protocols or APIs, such that even a Rust implementation that uses `&str` internally needs to count 16-bit code units for compatibility. For example this comes up in Servo’s implementation of some DOM APIs. One way to convert from an `&str` index (counted 8-bit code units) is: ```rust let index_16bit = string[..i].encode_utf16().count(); ``` Before this PR, the default `Iterator::count` relies on `Iterator::next` which decodes UTF-8 and encodes to UTF-16, doing more work than needed. Instead, this PR matches on bit patterns in the underlying UTF-8 string. For comparison, `str::Chars` already has a sophisticated impl for `Iterator::count` in `library/core/src/str/count.rs`.
981fca9 to
630793c
Compare
|
Fantastic, thank you! |
| self.chars.as_str().as_bytes().iter().fold(extra, |acc, &byte| { | ||
| acc.wrapping_add({ |
There was a problem hiding this comment.
This fold() looks like a manual reimplementation of sum().
There was a problem hiding this comment.
It is indeed. I accidentally removed it at some point but I had a comment saying this manually inlines <usize as Sum>::sum in order to use extra as the initial accumulator value, instead of using zero then adding extra
I also changed + with #[rustc_inherit_overflow_checks] to wrapping_add since this can never overflow
If you think that's better I can change it back to .map(...).sum() + extra
| } | ||
|
|
||
| #[test] | ||
| fn test_utf16_count() { |
There was a problem hiding this comment.
How about iterating through all unicode codepoints and verifying that count() gives the same answer as manually counting with a for loop? There are only one million codepoints, so it shouldn't take long.
There was a problem hiding this comment.
That feels a bit excessive? There's only four kinds of UTF-8 byte sequences / relevant ranges of code points, and the tests cover each of them
|
⌛ Testing commit 630793c with merge ecb69f1... Workflow: https://github.com/rust-lang/rust/actions/runs/29641888078 |
Add explicit `Iterator::count` impl for `str::EncodeUtf16` In the early days of Unicode, many systems adopted it by storing in-memory strings in (roughly) `Vec<u16>`, encoded as UCS-2 / UTF-16 / WTF-16. The “natural” way to index such strings or measure their length is counting UTF-16 code units. In some cases, such lengths or offsets found their way in protocols or APIs, such that even a Rust implementation that uses `&str` internally needs to count 16-bit code units for compatibility. For example this comes up in Servo’s implementation of some DOM APIs. One way to convert from an `&str` index (counted 8-bit code units) is: ```rust let index_16bit = string[..i].encode_utf16().count(); ``` Before this PR, the default `Iterator::count` relies on `Iterator::next` which decodes UTF-8 and encodes to UTF-16, doing more work than needed. Instead, this PR matches on bit patterns in the underlying UTF-8 string. For comparison, `str::Chars` already has a sophisticated impl for `Iterator::count` in `library/core/src/str/count.rs`.
…oboet Add explicit `Iterator::count` impl for `str::EncodeUtf16` In the early days of Unicode, many systems adopted it by storing in-memory strings in (roughly) `Vec<u16>`, encoded as UCS-2 / UTF-16 / WTF-16. The “natural” way to index such strings or measure their length is counting UTF-16 code units. In some cases, such lengths or offsets found their way in protocols or APIs, such that even a Rust implementation that uses `&str` internally needs to count 16-bit code units for compatibility. For example this comes up in Servo’s implementation of some DOM APIs. One way to convert from an `&str` index (counted 8-bit code units) is: ```rust let index_16bit = string[..i].encode_utf16().count(); ``` Before this PR, the default `Iterator::count` relies on `Iterator::next` which decodes UTF-8 and encodes to UTF-16, doing more work than needed. Instead, this PR matches on bit patterns in the underlying UTF-8 string. For comparison, `str::Chars` already has a sophisticated impl for `Iterator::count` in `library/core/src/str/count.rs`.
|
@bors yield |
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #159510. |
…uwer Rollup of 5 pull requests Successful merges: - #159467 (Add explicit `Iterator::count` impl for `str::EncodeUtf16`) - #158545 (Move `std::io::read_to_string` to `alloc::io`) - #159328 (tests/assembly-llvm: pin frame pointer in issue-141649 aarch64 test) - #159470 (Fix string indexing in diagnostic format strings) - #159500 (Move compiletest CLI parsing to `cli.rs`)
…uwer Rollup of 8 pull requests Successful merges: - #159425 (Windows: add context when opening NUL for child stdio fails) - #159467 (Add explicit `Iterator::count` impl for `str::EncodeUtf16`) - #157860 (Move rustdoc run-make tests into new `tests/run-make/rustdoc/`) - #158545 (Move `std::io::read_to_string` to `alloc::io`) - #159328 (tests/assembly-llvm: pin frame pointer in issue-141649 aarch64 test) - #159459 (Detect when trait bound requires closure to return itself) - #159470 (Fix string indexing in diagnostic format strings) - #159500 (Move compiletest CLI parsing to `cli.rs`)
Rollup merge of #159467 - SimonSapin:encode_utf16-count, r=joboet Add explicit `Iterator::count` impl for `str::EncodeUtf16` In the early days of Unicode, many systems adopted it by storing in-memory strings in (roughly) `Vec<u16>`, encoded as UCS-2 / UTF-16 / WTF-16. The “natural” way to index such strings or measure their length is counting UTF-16 code units. In some cases, such lengths or offsets found their way in protocols or APIs, such that even a Rust implementation that uses `&str` internally needs to count 16-bit code units for compatibility. For example this comes up in Servo’s implementation of some DOM APIs. One way to convert from an `&str` index (counted 8-bit code units) is: ```rust let index_16bit = string[..i].encode_utf16().count(); ``` Before this PR, the default `Iterator::count` relies on `Iterator::next` which decodes UTF-8 and encodes to UTF-16, doing more work than needed. Instead, this PR matches on bit patterns in the underlying UTF-8 string. For comparison, `str::Chars` already has a sophisticated impl for `Iterator::count` in `library/core/src/str/count.rs`.
In the early days of Unicode, many systems adopted it by storing in-memory strings in (roughly)
Vec<u16>, encoded as UCS-2 / UTF-16 / WTF-16. The “natural” way to index such strings or measure their length is counting UTF-16 code units. In some cases, such lengths or offsets found their way in protocols or APIs, such that even a Rust implementation that uses&strinternally needs to count 16-bit code units for compatibility. For example this comes up in Servo’s implementation of some DOM APIs.One way to convert from an
&strindex (counted 8-bit code units) is:Before this PR, the default
Iterator::countrelies onIterator::nextwhich decodes UTF-8 and encodes to UTF-16, doing more work than needed. Instead, this PR matches on bit patterns in the underlying UTF-8 string.For comparison,
str::Charsalready has a sophisticated impl forIterator::countinlibrary/core/src/str/count.rs.