Define more conversions to UTF8String

[malmaud]

cf #13582 (comment)

[stevengj]

utf16(a::Vector{UInt16}) does not assume that a is NUL-terminated (if a ends with a NUL codeunit, then the resulting string contains a NUL). So, it would be more consistent to use len=length(a) here, even if that means you need to do UTF8String(buf, length(buf)-1) at most of the call sites.

[tkelman]

It would be a bigger change, but would now be the right time to revisit the nul termination of utf16 and utf32 strings?

[stevengj]

Needs a test case, and probably a doc update since this is a user-visible function.

[tkelman]

Last time around we argued pretty strongly against conversions between string types and uint vectors with different encodings. I don't see what's changed.

[stevengj]

@tkelman, can you cite the relevant issue where this discussion took place?

We noticed a lot of cases where we were doing utf8(UTF16String(data)) and it would be better to avoid the intermediate allocation of a UTF16String.

[tkelman]

It was during one or more of the big messy unicode overhaul PR's IIRC. UTF16String(data::Vector{UInt16}) should be a cheap non-copying wrapper, shouldn't it?

[ScottPJones]

The problem (as I discussed back in the reviews of my PRs) with UTF16String, is that it requires a terminating 0x0000 word, forcing a copy just to add it in many cases.
The reason I had encode_to_utf8 and encode_to_utf16 functions was to handle the inconsistencies present in the Julia string support (visible \0 or not, UInt* or Char - at least the Char one has been fixed as I had suggested).
I don't recall who was opposed to having the methods that took a Vector{UInt16} or Vector{UInt32} (i.e without a trailing NUL word) besides @tkelman.
I still think, as this use case shows, that it is very important functionality, and should be in base
(we also are using it in our code, having to call the functions I added in Base, i.e. Base.encode_to_utf16 directly, since Tony removed the convert methods I had in my PR (which this PR seems to just re-add).

[tkelman]

If the underlying issue is trailing nuls, let's address the underlying issue now that we're in a dev period. A vector of unsigned integers is not the same thing as a string (the latter is usually represented by the former, but that is an implementation detail, not a fundamental invariant) and it's not a good idea IMO to define conversions that conflate encoding and storage by pretending otherwise. I seem to recall Jameson and a few others being of a similar opinion at the time.

[StefanKarpinski]

IMO, ensuring that trailing NUL bytes are present should be done on-demand when strings are passed to C. We now have the machinery in place to make that happen at the ccall entry point.

[ScottPJones]

@StefanKarpinski Yes, that was a very good addition. The problem now is to go through a lot of code in Base and packages, and ensure that any ccall's that really need a nul-terminated C string use Cstring or Cwstring instead of Ptr{UInt8}, Ptr{UInt16}.

[stevengj]

Actually, I think in all the cases where we have utf8(UTF16String(data)), the data is NUL-terminated, so UTF16String won't make a copy, and hence you're right that this PR is probably superfluous.

[stevengj]

@StefanKarpinski, we certainly have the machinery to add trailing NUL words when a string is passed as Cwstring. But this would entail making a copy of the string at every call site. It doesn't seem worth it.

[malmaud]

Alright, I'm just going to close this. If we want to have a separate PR that revisits null-terminated UInt16 considerations, that would make sense but it's probably not something I can take point on.

[StefanKarpinski]

@stevengj: my hypothesis is that every C API that takes NUL-terminated data only deals with small strings so the copying would have negligible impact. It would also be avoidable if the NUL already exists, so you could avoid it by just pre-converting to NUL-terminated form.