Runtime: Add Rune.TryEncodeToUtf8Bytes

Created on 22 Feb 2019  路  7Comments  路  Source: dotnet/runtime

The API Rune.TryEncodeToUtf8Bytes (see source.dot.net) is being added to the System.Runtime ref assembly to unblock feature work, but this API was never sent through formal review. We should review it.

Here's the API shape:

namespace System.Text
{
    public readonly partial struct Rune
    {
        // EXISTING APPROVED API - writes as UTF-16
        public bool TryEncode(Span<char> destination, out int charsWritten);

        // NEW PROPOSED API - writes as UTF-8
        public bool TryEncodeToUtf8Bytes(Span<byte> destination, out int bytesWritten);
    }
}

In particular we need to figure out how to normalize the TryEncode / TryEncodeToUtf8Bytes names so that they fit a common pattern. Per our brief review today we said that we're not opposed to suffixing everything with Utf16 and Utf8, especially since we expect that consumers of this particular API will be familiar with UTF-* representation.

/cc @ahsonkhan @ericstj

api-approved area-System.Runtime

Most helpful comment

@terrajobst, @bartonjs:

There are existing public APIs named DecodeUtf16 and DecodeUtf8. For consistency where the UTF suffix is provided, should the new APIs instead be named [Try]EncodeUtf16 and [Try]EncodeUtf8? Additionally, since the side effect is that these APIs write to a destination buffer, should they be [Try]EncodeToUtf16 and [Try]EncodeToUtf8?

I don't think we need a formal review on this, but if we can get agreement on the naming here I can quickly update the other PRs.

In particular, the APIs with the closest (most symmetric) naming would be:

public int EncodeToUtf16(Span<char> utf16Destination);
public int EncodeToUtf8(Span<byte> utf8Destination);
public bool TryEncodeToUtf16(Span<char> utf16Destination, out int charsWritten);
public bool TryEncodeToUtf8(Span<byte> utf8Destination, out int bytesWritten);

All 7 comments

Would it be useful to return OperationStatus instead of bool so that the method can indicate the reason for failure to the caller? If you're trying to use it with pipelines and you just need to get a bigger buffer it'd be better to retry than fail the entire operations wouldn't it?

The only failure reason would be that the buffer is too small. We could return OperationStatus, but it wouldn't buy us much.

If that's the case and there's some way to determine/calculate how much space will be needed that's fine.
Can Utf8 encoding of arbitrary bytes not fail?

Rune instances have Utf16SequenceLength and Utf8SequenceLength properties which will tell you the required buffer size.

UTF-8 encoding of arbitrary data can fail, but the Rune type does not represent arbitrary data. It represents a well-formed Unicode scalar value. This is validated by Rune's ctors. A well-formed Unicode scalar value will never fail encoding as long as the destination buffer is large enough.

Great, thanks for the clarification.

Video

Looks good, minor suggestion from review:

C# bool TryEncodeAsUtf8(...);

@GrabYourPitchforks should we also add a non-Try version that throws?

@terrajobst, @bartonjs:

There are existing public APIs named DecodeUtf16 and DecodeUtf8. For consistency where the UTF suffix is provided, should the new APIs instead be named [Try]EncodeUtf16 and [Try]EncodeUtf8? Additionally, since the side effect is that these APIs write to a destination buffer, should they be [Try]EncodeToUtf16 and [Try]EncodeToUtf8?

I don't think we need a formal review on this, but if we can get agreement on the naming here I can quickly update the other PRs.

In particular, the APIs with the closest (most symmetric) naming would be:

public int EncodeToUtf16(Span<char> utf16Destination);
public int EncodeToUtf8(Span<byte> utf8Destination);
public bool TryEncodeToUtf16(Span<char> utf16Destination, out int charsWritten);
public bool TryEncodeToUtf8(Span<byte> utf8Destination, out int bytesWritten);
Was this page helpful?
0 / 5 - 0 ratings

Related issues

Timovzl picture Timovzl  路  3Comments

noahfalk picture noahfalk  路  3Comments

yahorsi picture yahorsi  路  3Comments

v0l picture v0l  路  3Comments

jzabroski picture jzabroski  路  3Comments