Runtime: Proposal: Add IdnMapping Span-based APIs

[MihaZupan]

The current IdnMapping API accepts/returns strings and throws on invalid input.

I propose a set of Span-based APIs to avoid allocations and verifying the input by catching exceptions.

```c#
namespace System.Globalization
{
public sealed class IdnMapping
{
// Existing API
public string GetAscii(string unicode);
public string GetAscii(string unicode, int index);
public string GetAscii(string unicode, int index, int count);

    public string GetUnicode(string ascii);
    public string GetUnicode(string ascii, int index);
    public string GetUnicode(string ascii, int index, int count);

    // Proposed API
    public bool TryGetAscii(ReadOnlySpan<char> unicode, out string ascii);
    public bool TryGetAscii(ReadOnlySpan<char> unicode, Span<char> destination, out int charsWritten);

    public bool TryGetUnicode(ReadOnlySpan<char> ascii, out string unicode);
    public bool TryGetUnicode(ReadOnlySpan<char> ascii, Span<char> destination, out int charsWritten);
}

}
```

The TryGet* methods would return false in the case of invalid input/insufficient space in the destination span.

This new API would simplify call sites and remove allocations throughout code dealing with internationalized domain names, like Uri and Markdig.

cc: @tarekgh

[tarekgh]

In general, the proposal looks reasonable.

Why we need the APIs:

C# public string GetAscii(ReadOnlySpan<char> unicode); public string GetUnicode(ReadOnlySpan<char> ascii);

I don't think these are useful if we are going to allocate a string anyway. And the other proposed APIs can be used at that time. What do you think about that?

[MihaZupan]

Those aren't needed in my use-cases, as Try* would always be used.

I'll remove them as they could easily be exposed later if a use-case presents itself.

[stephentoub]

I don't understand these APIs:

public bool TryGetAscii(ReadOnlySpan<char> unicode, out string ascii);
public bool TryGetUnicode(ReadOnlySpan<char> ascii, out string unicode);

What does the Try mean here? If it's to avoid throwing in the case where the data is somehow invalid, that's a different meaning than the other Try overloads would have, which would be based solely on whether the destination is large enough. The Boolean returned from a Try is supposed to convey only one thing, and in such span-based Try methods, it's always used to connote whether the destination was large enough to store the transformed data. I don't think we want two overloads of the same method having a different meaning for the Try.

[MihaZupan]

The idea was for both the out string and Span destination to return false on invalid input. The span one would also return false on insufficient space.

I agree the Span overload would be confusing to use since you couldn't differentiate between invalid input/insufficient space, without ensuring you supply a worst-case sized buffer.

Do we have a pattern of Try* methods ever throwing? If so, we could have all overloads throw on invalid input, where the Try only returns false on insufficient space.
```c#
// Existing
public string GetAscii(string unicode);

// New
public string GetAscii(ReadOnlySpan unicode);
bool TryGetAscii(ReadOnlySpan unicode, Span destination, out int charsWritten);
```

Alternatively, we would need an OperationStatus-style return?

[stephentoub]

Do we have a pattern of Try* methods ever throwing?

Yes, Try methods can still throw.

Alternatively, we would need an OperationStatus-style return?

Why not just:
C# string GetAscii(ReadOnlySpan<char> unicode); bool TryGetAscii(ReadOnlySpan<char> unicode, Span<char> destination, out int charsWritten);
?

If the exception for invalid input really is unexceptional, though, with the exception happening so frequently as to be a performance problem in real situations, then yeah, OperationStatus is what you'd want.

Can you share examples where the exception is a meaningful problem?

[MihaZupan]

string GetAscii(ReadOnlySpan<char> unicode);

Yes, that is the shape we'd want (lazy copy-pasting, I've edited the comment).

A few examples of exceptional inputs from reading IdnMapping code:

• Length: Empty / over 255 chars total / over 63 chars per label
• Certain sequences
  
  
  
  • Two dots in a row
  
  
  • (- before .): foo-.bar
  
  
  • Invalid right-to-left
  
  
• etc.
• There are a few more cases in test code.

I wouldn't expect the performance overhead of exceptions to be a problem for well-behaving apps.

Edit:

where the exception is a meaningful problem?

No.

[ericstj]

Moving this to 6.0.0 as it is not yet approved and we're driving 5.0.0 issues down. Move it back if you think this is still needed in 5.0.0.