Out of curiosity, what does the perf look like with an iterator vs a hard-coded to_utf16 loop?

Is there any special reason why from_utf8 returns Option<&'a str> while from_utf16 returns Option<String>? This tripped me up a bit when working on the tests. Some of the {to, from}_utf{8,16} works on str and some on String, perhaps this could be unified somehow?

str and String both use UTF-8 internally, so from_utf8 can just check if the &[u8] buffer is valid UTF-8, and then cast it to a &str: the conversion can be done by just reinterpreting the same chunk of memory as a different type.

On the other hand, a UTF-16 vector does not have the same memory layout as a string, the u16s have to be decoded and built up into a dynamic String. (Theoretically from_utf16 can/should return an Iterator<char>, rather than actually building the String internally.)

Fixed tests, squashed and rebased. ping @huonw

It feels unnecessary to have both to_utf16 and utf16_iter, since the iterator is strictly more flexible. How about removing to_utf16, renaming utf16_iter to to_utf16, and also make from_utf16 into an iterator so the semantics match. I can refactor the call sites as needed.

How about removing to_utf16,

This PR is doing this already: deprecation is how functions are removed (deprecated things are properly deleted a month or so after they were deprecated).

renaming utf16_iter to to_utf16

Using .to_utf16 defies the naming conventions (it's not an expensive cast).

also make from_utf16 into an iterator so the semantics match

This is a good idea. Theoretically both of these can just be iterator adaptors, that is

fn from_utf16<I: Iterator<u16>>(it: I) -> FromUtf16<I> { ... }
fn to_utf16<I: Iterator<char>>(it: I) -> ToUtf16<I> { ... }

impl<I: Iterator<u16>> Iterator<char> for FromUtf16<I> { ... }
impl<I: Iterator<char>> Iterator<u16> for ToUtf16<I> { ... }

and this would then be called like to_utf16(string.chars()). I don't think this is necessary to do in this PR.

(BTW, I edited a [breaking-change] description into the PR text, since this "removes" a method from strings.)

@huonw r?

This is tricky, since it touches a bunch of Windows-related things in libnative that I can't check. So it builds fine locally but fails when bors runs the tests. I spent a couple of hours trying to build Rust on Windows but there is no way I'll figure that out. I've changed all occurrences of to_utf16 to utf16_iter and I hope I got it right...

@huonw r? Sorry for spamming about this, didn't anticipate the libnative errors...

Shouldn't this be called utf16() or something like utf16_units() without _iter in the name?

I asked on IRC. r=me if the name changes to utf16_units. (git ls-files -z | xargs -0 sed -i s/utf16_iter/utf16_units/g should do the renaming throughout the whole codebase.)

@huonw Renamed. I have tried building this locally, but as reported in #15250 and #15279, I have not been able to make it through the test suite even with master! So no guarantees, but I really hope this works now. r?

Yay, finally!