Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Utf8Formatter/Utf8Parser should have UInt128 and Int128 overloads #73842

Closed
Tracked by #79006
krwq opened this issue Aug 12, 2022 · 5 comments
Closed
Tracked by #79006
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Buffers
Milestone

Comments

@krwq
Copy link
Member

krwq commented Aug 12, 2022

Background and motivation

As I started looking into #73500 I realized Utf8Formatter/Utf8Parser are missing UInt128/Int128 overloads and therefore we need to unnecessarily do UTF16 <--> UTF8 conversion.

cc: @tannergooding

API Proposal

namespace System.Buffers.Text;

public static partial class Utf8Formatter
{
    public static bool TryFormat(UInt128 value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format = default(System.Buffers.StandardFormat)) { throw null; }
    public static bool TryFormat(Int128 value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format = default(System.Buffers.StandardFormat)) { throw null; }
}

public static partial class Utf8Parser
{
    public static bool TryParse(System.ReadOnlySpan<byte> source, out UInt128 value, out int bytesConsumed, char standardFormat = '\0') { throw null; }
    public static bool TryParse(System.ReadOnlySpan<byte> source, out Int128 value, out int bytesConsumed, char standardFormat = '\0') { throw null; }
}

API Usage

if (Utf8Parser.TryParse(someUtf8Span, out UInt128 value, out int bytesConsumed) && someUtf8Span.Length == bytesConsumed)
{
    Console.WriteLine($"Successfully parsed UInt128 value: {value}");
}

UInt128 value = ...;
bool result = Utf8Formatter.TryFormat(value, utf8Output, out int bytesWritten);

// and similar for Int128

Alternative Designs

No response

Risks

No response

@krwq krwq added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Buffers labels Aug 12, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 12, 2022
@ghost
Copy link

ghost commented Aug 12, 2022

Tagging subscribers to this area: @dotnet/area-system-buffers
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

As I started looking into #73500 I realized Utf8Formatter/Utf8Parser are missing UInt128/Int128 overloads and therefore we need to unnecessarily do UTF16 <--> UTF8 conversion.

cc: @tannergooding

API Proposal

namespace System.Buffers.Text;

public static partial class Utf8Formatter
{
    public static bool TryFormat(UInt128 value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format = default(System.Buffers.StandardFormat)) { throw null; }
    public static bool TryFormat(Int128 value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format = default(System.Buffers.StandardFormat)) { throw null; }
}

public static partial class Utf8Parser
{
    public static bool TryParse(System.ReadOnlySpan<byte> source, out UInt128 value, out int bytesConsumed, char standardFormat = '\0') { throw null; }
    public static bool TryParse(System.ReadOnlySpan<byte> source, out Int128 value, out int bytesConsumed, char standardFormat = '\0') { throw null; }
}

API Usage

if (Utf8Parser.TryParse(someUtf8Span, out UInt128 value, out int bytesConsumed) && someUtf8Span.Length == bytesConsumed)
{
    Console.WriteLine($"Successfully parsed UInt128 value: {value}");
}

UInt128 value = ...;
bool result = Utf8Formatter.TryFormat(value, utf8Output, out int bytesWritten);

// and similar for Int128

Alternative Designs

No response

Risks

No response

Author: krwq
Assignees: -
Labels:

api-suggestion, area-System.Buffers

Milestone: -

@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Aug 12, 2022
@tannergooding tannergooding added this to the 8.0.0 milestone Aug 12, 2022
@bartonjs
Copy link
Member

bartonjs commented Sep 6, 2022

Video

Looks good as proposed

namespace System.Buffers.Text;

public static partial class Utf8Formatter
{
    public static bool TryFormat(UInt128 value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format = default(System.Buffers.StandardFormat)) { throw null; }
    public static bool TryFormat(Int128 value, System.Span<byte> destination, out int bytesWritten, System.Buffers.StandardFormat format = default(System.Buffers.StandardFormat)) { throw null; }
}

public static partial class Utf8Parser
{
    public static bool TryParse(System.ReadOnlySpan<byte> source, out UInt128 value, out int bytesConsumed, char standardFormat = '\0') { throw null; }
    public static bool TryParse(System.ReadOnlySpan<byte> source, out Int128 value, out int bytesConsumed, char standardFormat = '\0') { throw null; }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Sep 6, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 16, 2023
@hrrrrustic
Copy link
Contributor

hrrrrustic commented May 17, 2023

@krwq @tannergooding
I'm trying to implement this API, but there are a lot of usages Convert.ChangeType and Convert.ToIntXX in the tests. Unfortunately, these methods are not support {U}Int128, is it by design or just nobody asked for this? Does this methods kind of obsolete and should be workarounded or I should create an issue with API proposal?

For Convert.ChangeType U{Int128} should implement IConvertible
For Convert.ToIntXX just add new methods

Btw, why U{Int128} returns a new instance every time in properties like {Max/Min}Value or NegativeOne? Other number types returns private consts or static readonly fields

public static Int128 MinValue => new Int128(0x8000_0000_0000_0000, 0);

@hrrrrustic
Copy link
Contributor

@dotnet/area-system-buffers (does this tags work? 🤔)
This is kind of blocking linked PR, need some help for comment above

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 6, 2023
@tannergooding
Copy link
Member

Much like with #53768 (comment), the plan is for APIs to simply implement IUtf8SpanFormattable and IUtf8SpanParsable moving forward.

For number types in particular, parsing where the first invalid character is treated as "end of string" rather than "invalid input" will be supported via #87171

@tannergooding tannergooding added api-suggestion Early API idea and discussion, it is NOT ready for implementation and removed api-approved API was approved in API review, it can be implemented labels Jul 24, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Buffers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants