Skip to content

Update BigInteger and Complex to support UTF8 parsing and formatting #117745

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tannergooding
Copy link
Member

Still need to add tests

Copy link
Contributor

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


public ValueStringBuilder(Span<TChar> initialBuffer)
{
Debug.Assert((typeof(TChar) == typeof(Utf8Char)) || (typeof(TChar) == typeof(Utf16Char)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is specific to these types that are defined just in System.Runtime.Numerics, and it's only used by System.Runtime.Numerics, we probably shouldn't have the file in Common?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not really specific to the numeric types and is usable by any other scenario using ValueStringBuilder that needs to support UTF-8 as well. So I put it in the place that people can find it if they're making similar changes to other paths in the future.

More ideally we wouldn't need the Utf8Char shim in System.Runtime.Numerics and we could just use byte/char directly. However, we currently need the direct CastFrom/CastTo helpers to ensure the right codegen happens. I think we could probably fix this by special casing CreateFromTruncating in the JIT for the primitive types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants