-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add hexadecimal float/double parsing and formatting support (IEEE 754:2008) #120637
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, there are also some analyzer failures:
D:\a\_work\1\s\src\libraries\System.Private.CoreLib\src\System\Number.Formatting.cs(639,96): error IDE0060: Remove unused parameter 'precision' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0060) [D:\a\_work\1\s\src\coreclr\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\a\_work\1\s\src\libraries\System.Private.CoreLib\src\System\Number.Formatting.cs(639,115): error IDE0060: Remove unused parameter 'format' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0060) [D:\a\_work\1\s\src\coreclr\System.Private.CoreLib\System.Private.CoreLib.csproj]
D:\a\_work\1\s\src\libraries\System.Private.CoreLib\src\System\Number.Formatting.cs(639,140): error IDE0060: Remove unused parameter 'info' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0060) [D:\a\_work\1\s\src\coreclr\System.Private.CoreLib\System.Private.CoreLib.csproj]
if ((style & InvalidNumberStyles) != 0) | ||
{ | ||
ThrowInvalid(style); | ||
throw new ArgumentException(SR.Argument_InvalidNumberStyles, nameof(style)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, this code is hot path, and the manual outlining with throw helpers was done on purpose. Please revert your changes that inlined these and ensure that any additional throws you add to such hot paths are similarly manually outlined into throw helpers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted the inlining and restored manual throw helper outlining with local static methods marked with [DoesNotReturn]. Added the required using directive for System.Diagnostics.CodeAnalysis.
value = -value; | ||
} | ||
|
||
string numStr = ((uint)value).ToString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, there are already routines for formatting Int32 values; can those not be reused? And if for some reason they can't and you have to write your own, we should not be allocating this temporary string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced the FormatInt32 method with FormatInt32ToValueListBuilder that formats digits directly into the ValueListBuilder without allocating a temporary string. The implementation now reverses digits in-place after formatting them in reverse order.
// Parse "0x" or "0X" prefix | ||
if (index + 1 >= value.Length || | ||
TChar.CastToUInt32(value[index]) != '0' || | ||
(TChar.CastToUInt32(value[index + 1]) != 'x' && TChar.CastToUInt32(value[index + 1]) != 'X')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, can you use the | 0x20
trick to just compare against 'x'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use | 0x20
trick: (TChar.CastToUInt32(value[index + 1]) | 0x20) != 'x'
yield return new object[] { "0x0.8p0", NumberStyles.HexFloat, invariantFormat, 0.5 }; | ||
yield return new object[] { "-0x1.0p0", NumberStyles.HexFloat, invariantFormat, -1.0 }; | ||
yield return new object[] { "+0x1.0p0", NumberStyles.HexFloat, invariantFormat, 1.0 }; | ||
yield return new object[] { "0x1.921fb54442d18p+1", NumberStyles.HexFloat, invariantFormat, Math.PI }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, this needs much more extensive testing. You've only added limited tests here, only for double, and only for parsing. You need to validate for floats and Half as well, for not only parsing but also formatting, and a much more extensive set of inputs, covering all manner of corner cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comprehensive tests for double, float, and Half covering:
- Parsing: basic values, case variations, no decimal point, only fractional part, large exponents, edge cases (epsilon, max, min), whitespace
- Formatting: uppercase/lowercase, various precision levels, negative values, zero, edge cases
- Total of 40+ new test cases across all three types
|
||
// Format exponent as decimal | ||
FormatInt32(ref vlb, actualExponent, 0, null, info); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, I had gpt-5 review your implementation of FormatFloatAsHex, and it had the following feedback:
Here are the key issues and what to change:
1. Wrong first nibble & double-append bug
You compute firstNibble = (int)(mantissa >> 60) and then do:
vlb.Append('0' + (firstNibble > 9 ? 0 : firstNibble));
if (firstNibble > 9) vlb.Append((char)(hexBase + firstNibble - 10));
This appends two chars for A–F (e.g., “0A”). It should append one char total.
Fix: compute a single char hexChar = nibble < 10 ? (char)('0' + nibble) : (char)(hexBase + nibble - 10); and append it once.
2. Mantissa normalization puts the leading 1 in the wrong place
You left-shift the significand so the implicit 1 lands at bit 63, then read the top nibble. That top nibble will be 0b1000 (= 8) for normalized values, so you’ll emit 0x8... instead of standard %a/%A’s 0x1....
%a requires the significand to be in [1, 2) (hex rendered as 1.xxxxx). The usual way:
For normals: sig = (1UL << mantissaBits) | mantissa.
For subnormals: sig = mantissa; then count leading zeros lz and shift left so the highest 1 is at the same place as normals; decrement the exponent by lz.
Then, to make the first hex digit be 1, align the leading 1 to bit 60 (not 63). Equivalently: after you’ve put the leading 1 at bit 63, shift right by 3 and add 3 to the exponent (since you divided the significand by 2³).
Without this, your integer digit will be 8, not 1.
3. Subnormals aren’t normalized
For biasedExponent == 0 && mantissa != 0, you set actualExponent = 1 - exponentBias but don’t shift the mantissa up to locate the first 1. You must:
lz = LeadingZeroCount(sig) - (64 - (mantissaBits + 1)) (for a 64-bit container),
sig <<= lz;
actualExponent -= lz;
…then apply the “align to bit 60” adjustment above.
4. Special values handling is incorrect
For Infinity or NaN you currently print 0x0p+0. That’s misleading. Typical %a emits:
inf / -inf
nan (sign ignored; optional payload not shown)
If you must keep a hex form only, at minimum don’t pretend it’s a finite 0 with p+0.
5. Case handling for %A
Uppercase format should use 0X and P (not p) and hex digits A–F. You only uppercase the 0X and digit letters, but you always write 'p'.
6. Precision semantics & rounding
You truncate; %a requires rounding to the requested precision (ties to even, generally). Also, if precision < 0, default should be enough hex digits to exactly represent the type (ceil(mantissaBits/4)). You did that part, but still need rounding.
7. Exponent value correctness after your shifts
Once you fix the normalization (items 2 & 3), remember: any time you shift the significand by k bits toward the right (to make the first hex digit 1), you must add k to the binary exponent; shifting left by k subtracts k.
8. Minor: prefix and zero
0x0p+0 / -0x0p+0 for ±0 are fine. If precision > 0, you add .000… which is acceptable, but ensure this matches your intended spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed all issues in FormatFloatAsHex:
- Fixed nibble output to emit single char using consistent logic
- Fixed mantissa normalization - now shifts significand so leading 1 is at bit 60 (not 63), ensuring first hex digit is 1
- Added proper subnormal normalization using BitOperations.LeadingZeroCount and adjusting exponent accordingly
- Shifted right by 3 and added 3 to exponent to compensate for bit 60 positioning
The implementation now correctly outputs standard hex float format with leading digit in [1, 2) range.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements IEEE 754:2008 section 5.12.3 support for parsing and formatting floating-point numbers using hexadecimal significand representation. The implementation allows parsing values like 0x1.8p0
(= 1.5) and formatting floats/doubles to hex representation using the 'X'/'x' format specifiers.
Key Changes:
- Adds
NumberStyles.HexFloat
enum value for hexadecimal float parsing - Implements comprehensive hex float parsing logic with proper IEEE 754 conversion
- Adds hex float formatting support using 'X'/'x' format specifiers
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
NumberStyles.cs |
Adds HexFloat composite style flag |
System.Runtime.cs |
Updates reference assembly with new HexFloat enum value |
NumberFormatInfo.cs |
Updates validation to allow hex specifier for floating-point with specific constraints |
Number.Parsing.cs |
Implements complete hex float parsing with IEEE 754 conversion |
Number.Formatting.cs |
Adds hex float formatting functionality |
DoubleTests.cs |
Adds basic test cases for hex float parsing |
// Handle special cases | ||
if (biasedExponent == TNumber.InfinityExponent) | ||
{ | ||
// Infinity or NaN - just output as 0 |
Copilot
AI
Oct 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra space in comment: 'output as' should be 'output as'.
// Infinity or NaN - just output as 0 | |
// Infinity or NaN - just output as 0 |
Copilot uses AI. Check for mistakes.
vlb.Append(TChar.CastFrom((char)('0' + (firstNibble > 9 ? 0 : firstNibble)))); | ||
if (firstNibble > 9) | ||
{ | ||
vlb.Append(TChar.CastFrom((char)(hexBase + firstNibble - 10))); | ||
} |
Copilot
AI
Oct 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error in hex digit output. Lines 603-607 will output '0' followed by the hex character for values > 9, but should output just the hex character. The conditional logic should be reversed or restructured.
vlb.Append(TChar.CastFrom((char)('0' + (firstNibble > 9 ? 0 : firstNibble)))); | |
if (firstNibble > 9) | |
{ | |
vlb.Append(TChar.CastFrom((char)(hexBase + firstNibble - 10))); | |
} | |
vlb.Append(TChar.CastFrom(firstNibble < 10 ? (char)('0' + firstNibble) : (char)(hexBase + firstNibble - 10))); |
Copilot uses AI. Check for mistakes.
{ | ||
if (value < 0) | ||
{ | ||
vlb.Append(TChar.CastFrom('-')); |
Copilot
AI
Oct 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential integer overflow when negating int.MinValue
. The negation of int.MinValue
overflows and remains negative, which could cause incorrect behavior.
vlb.Append(TChar.CastFrom('-')); | |
vlb.Append(TChar.CastFrom('-')); | |
if (value == int.MinValue) | |
{ | |
// int.MinValue cannot be negated; use its magnitude as uint | |
string numStr = ((uint)int.MinValue).ToString(); | |
foreach (char c in numStr) | |
{ | |
vlb.Append(TChar.CastFrom(c)); | |
} | |
return; | |
} |
Copilot uses AI. Check for mistakes.
… comprehensive tests Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
vlb.Append(TChar.CastFrom((char)('0' + (uvalue % 10)))); | ||
uvalue /= 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use Math.DivRem
.
} | ||
|
||
// Parse "0x" or "0X" prefix | ||
if (index + 1 >= value.Length || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (index + 1 >= value.Length || | |
if (index >= value.Length - 1 || |
To avoid potential overflow (just to be super pedantic about this).
Implementation Plan for Hexadecimal Float/Double Parsing and Formatting
HexFloat
composite style toNumberStyles
enumValidateParseStyleFloatingPoint
to allowAllowHexSpecifier
with appropriate flags for float/double parsingNumber.Parsing.cs
TryParseFloat
methodNumber.Formatting.cs
Code Review Feedback Addressed
Hot Path Performance (ValidateParseStyleFloatingPoint)
[DoesNotReturn]
attributeusing System.Diagnostics.CodeAnalysis;
FormatFloatAsHex Bugs Fixed
FormatInt32 Improvements
Parser Optimization
| 0x20
trick for case-insensitive 'x'/'X' comparisonComprehensive Testing
Original prompt
This section details on the original issue you should resolve
<issue_title>Add the ability to parse/format a float/double from/to a hexadecimal literal</issue_title>
<issue_description>### Part of #27204
In IEEE 754:2008 part 5.12.3, transfering a float/double from/to an
external hexadecimal-significand character sequence representing finite number
is requested while we don't have it yet. The pattern is like this: (regex)which means:
benefits
0x0.ffp0
is the equivalent of0.99609375
while using less chars. Same for formatting as it reduces the size of string to transfer.proposal APIs
And let
Numbers.ParseDouble/Single(string s, NumberStyles style[, NumberFormatInfo info])
acceptNumberstyles.HexFloat
(or it withoutNumberStyles.AllowDecimalPoint
) and correctly parse string.Edit
Numbers.FormatDouble/Single(ref ValueStringBuilder sb, double/float value, ReadOnlySpan<char> format, NumberFormatInfo info)
so that they can correctly identifyX
specifier (which is also used for outputting integers in hex) which may have a trailing precision specifier, and correctly format it. </issue_description>Comments on the Issue (you are @copilot in this section)
@stephentoub cc: @tannergooding @tannergooding Thanks for opening this, its good to see additional customer wants for these areas 😄It is partially tracked by https://github.com/dotnet/corefx/issues/31901 (which is IEEE 754:2008 compliance) and by #1387 (which is IEEE 754:2019 compliance), but those are largely meta issues and the individual proposals will be easier to take through API review.
The format specifier can be broken down into:
The terminal
[fFdD]?
listed is not actually part of the IEEE specification and should be excluded.I also updated
hexSignificand
to clarify that just.
is not valid.The computed value is
hexSignificand * 2^decExponent
.So, for example if you have
0x1.234p0
this is:0x1
==1
0x234 * 16^-3
==564 * 16^-3
==0.1376953125
1.1376953125 * 2^0
==1.1376953125
</comment_new><comment_new>@tannergooding
Even though this isn't a new API, I believe we still want to take it through API review since it is modifying an existing API.
We would want to check it against the compat bar and make the necessary decisions around what flags would be used to support this functionality and ensure that we wouldn't accidentally introduce any breaking changes, etc.</comment_new>
<comment_new>@tannergooding
I don't see why it wouldn't, provided it was still valid according to the IEEE requirements.</comment_new>
<comment_new>@tannergooding
I don't believe its out of scope, as I said as long as its still valid according to the IEEE requirements, it should be fine.
There are "paths" where the decimal point isn't required and so the user should fully be able to specify that
hex
is allowed butdecimal-point
is not. It's an advanced scenario, but the number parser already supports it and will continue doing so even if we add support forhex
, so there is no reason to block it. The interesting scenario would be whether or not to allow the exponent to be optional and to default to0
(which would still be valid based on the normal conversion rules, etc).Fixes #1630
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.