-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fixes of invalid biginteger casting #56710
Changes from all commits
61fc7a0
74b4de8
d170dbb
ca332dc
6d60918
5157bb1
dbbaf21
8c083a0
ed7ebcc
6ed47f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1799,35 +1799,26 @@ public static explicit operator double(BigInteger value) | |
|
||
int sign = value._sign; | ||
uint[]? bits = value._bits; | ||
double returnValue; | ||
|
||
if (bits == null) | ||
return sign; | ||
|
||
int length = bits.Length; | ||
|
||
// The maximum exponent for doubles is 1023, which corresponds to a uint bit length of 32. | ||
// All BigIntegers with bits[] longer than 32 evaluate to Double.Infinity (or NegativeInfinity). | ||
// Cases where the exponent is between 1024 and 1035 are handled in NumericsHelpers.GetDoubleFromParts. | ||
const int InfinityLength = 1024 / kcbitUint; | ||
|
||
if (length > InfinityLength) | ||
if (IsEmpty(bits)) | ||
{ | ||
if (sign == 1) | ||
return double.PositiveInfinity; | ||
returnValue = sign; | ||
} | ||
else | ||
{ | ||
int length = bits.Length; | ||
if (!TryGetInfinity(length, sign, out double? rawValue)) | ||
{ | ||
returnValue = ConvertToDouble(bits, length, sign); | ||
} | ||
else | ||
return double.NegativeInfinity; | ||
{ | ||
returnValue = rawValue.Value; | ||
} | ||
} | ||
|
||
ulong h = bits[length - 1]; | ||
ulong m = length > 1 ? bits[length - 2] : 0; | ||
ulong l = length > 2 ? bits[length - 3] : 0; | ||
|
||
int z = NumericsHelpers.CbitHighZero((uint)h); | ||
|
||
int exp = (length - 2) * 32 - z; | ||
ulong man = (h << 32 + z) | (m << z) | (l >> 32 - z); | ||
|
||
return NumericsHelpers.GetDoubleFromParts(sign, exp, man); | ||
return returnValue; | ||
} | ||
|
||
public static explicit operator decimal(BigInteger value) | ||
|
@@ -2433,6 +2424,36 @@ public long GetBitLength() | |
return bitLength - 1; | ||
} | ||
|
||
private static double ConvertToDouble(uint[] bits, | ||
int length, | ||
int sign) | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be better to try and normalize this to be the same as the already known "good" logic used in our The overall logic is a bit more complex since it needs to handle non integral numbers; but it also correctly handles rounding and various edge cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tannergooding, thatk you for your comment, I had not know about it. I agree, that a widely used method is better than a new one, but I also believe that the provided methods contain more logic than necessary and may work little slower. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tannergooding , I can't understand how to use https://source.dot.net/#System.Private.CoreLib/Number.Parsing.cs,9927df209c6039f5,references , because I can not add reference to System.Private.CoreLib . |
||
ulong highBit, middleBit; | ||
ulong nonZeroDigit; | ||
int leftShiftingSize; | ||
double unsignedNumber, returnValue; | ||
|
||
const int NonZeroDigitsCountOfUInt32 = 32; | ||
|
||
highBit = bits[length - 1]; | ||
middleBit = length > 1 ? bits[length - 2] : 0; | ||
|
||
leftShiftingSize = (length - 2) * NonZeroDigitsCountOfUInt32; | ||
nonZeroDigit = (highBit << NonZeroDigitsCountOfUInt32) | middleBit; | ||
unsignedNumber = Math.ScaleB(nonZeroDigit, leftShiftingSize); | ||
|
||
if (sign < 0) | ||
{ | ||
returnValue = -unsignedNumber; | ||
} | ||
else | ||
{ | ||
returnValue = unsignedNumber; | ||
} | ||
|
||
return returnValue; | ||
} | ||
|
||
/// <summary> | ||
/// Encapsulate the logic of normalizing the "small" and "large" forms of BigInteger | ||
/// into the "large" form so that Bit Manipulation algorithms can be simplified. | ||
|
@@ -2461,6 +2482,54 @@ private static bool GetPartsForBitManipulation(ref BigInteger x, ref Span<uint> | |
return x._sign < 0; | ||
} | ||
|
||
private static bool IsEmpty([NotNullWhen(false)] uint[]? bits) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeap, you are right. I'll fix it after complete review |
||
{ | ||
bool returnValue; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't this simply |
||
|
||
if (bits == null) | ||
{ | ||
returnValue = true; | ||
} | ||
else | ||
{ | ||
returnValue = false; | ||
} | ||
|
||
return returnValue; | ||
} | ||
|
||
private static bool TryGetInfinity(int length, | ||
int sign, | ||
[NotNullWhen(true)] out double? infinityValue) | ||
{ | ||
bool returnValue; | ||
|
||
// The maximum exponent for doubles is 1023, which corresponds to a uint bit length of 32. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The comment of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The last sentence also looks to be no longer true. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tannergooding , I think, comment is evil and Uncle Bob ( http://cleancoder.com/products ) absolutely agree with me. I had wanted to remove this comment, but it can very hard to understand what's the magic value 1024 in line below. I'll try to change this comment to more clear. |
||
// All BigIntegers with bits[] longer than 32 evaluate to Double.Infinity (or NegativeInfinity). | ||
// Cases where the exponent is between 1024 and 1035 are handled in NumericsHelpers.GetDoubleFromParts. | ||
const int InfinityLength = 1024 / kcbitUint; | ||
|
||
if (length > InfinityLength) | ||
{ | ||
if (sign == 1) | ||
{ | ||
infinityValue = double.PositiveInfinity; | ||
} | ||
else | ||
{ | ||
infinityValue = double.NegativeInfinity; | ||
} | ||
returnValue = true; | ||
} | ||
else | ||
{ | ||
infinityValue = null; | ||
returnValue = false; | ||
} | ||
|
||
return returnValue; | ||
} | ||
|
||
internal static int GetDiffLength(uint[] rgu1, uint[] rgu2, int cu) | ||
{ | ||
for (int iv = cu; --iv >= 0;) | ||
|
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.
nit: We don't normally have a parameter per line.