Unify formatting part of BigInteger of CoreLib #97889
Conversation
This reverts commit bcc70e3.
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-numerics |
Thanks for continuing the work/effort here! I'll try to get this reviewed in the next week or so, but as you know there's a few different BigInteger PRs still pending going in, so we'll see where it ends up. |
huoyaoyuan
force-pushed
the
numerics-format-2
branch
from
182ffaa
to
563dd5d
Compare
|
Splitting optimization to another PR to make this easier. I don't want this to be blocked (and blocking for others) for longer period. Edit: I'm probably unable to complete this before vacation. |
|
|
||
| internal static unsafe void NumberToString<TChar>(ref ValueListBuilder<TChar> vlb, ref NumberBuffer number, char format, int nMaxDigits, NumberFormatInfo info) where TChar : unmanaged, IUtfChar<TChar> | ||
| { | ||
| Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); |
There was a problem hiding this comment.
This assert is painful for polyfill. How about switching to sizeof?
IBinaryInteger provides similar performance with IUtfChar on CLR, but on mono it may cause regression.
There was a problem hiding this comment.
How is it painful, I'm not understanding?
There was a problem hiding this comment.
S.R.Numerics can't call these methods with byte or char directly, but a polyfill type that implements an internal definition of IUtfChar instead. I have done this in previous PR. There's no such type check in parsing logic.
|
Tests are passing locally. The adapting of this PR is unoptimized to simplify the reviewing. |
|
@huoyaoyuan, there's a conflict here. This otherwise looks good to me so I'll finish my last pass and merge if CI passes after the conflict is resolved |
|
Conflicts resolved. |
Another part of #28657, in continuation of #85978 and #95402.
It's 2.5 years from my initial attempt on this, and more than half a year for code in this PR.