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

Convert.ChangeType throws exception when converting Double.MaxValue #5989

Open
mdmoura opened this issue Jun 24, 2016 · 10 comments

Comments

@mdmoura
Copy link

commented Jun 24, 2016

The following code:

    var converted = Convert.ChangeType(Double.MaxValue.ToString(), typeof(Double));

Results in the exception:

   Value was either too large or too small for a Double.

I suppose this is a bug, correct?

@mdmoura mdmoura changed the title Convert.ChangeType throws excpetion when converting Double.MaxValue Convert.ChangeType throws exception when converting Double.MaxValue Jun 24, 2016

@jkotas jkotas added the mscorlib label Jun 24, 2016

@gkhanna79 gkhanna79 added this to the 2.0.0 milestone Mar 6, 2017

@AlexGhiondea AlexGhiondea modified the milestones: Future, 2.0.0 Mar 20, 2017

@AlexGhiondea AlexGhiondea removed their assignment Mar 20, 2017

@AlexGhiondea

This comment has been minimized.

Copy link
Member

commented Mar 20, 2017

@mdmoura thanks for reporting this. It does seem strange that this throws.

Would you be interested in looking at why that is and suggesting a solution?

Please note that this is the behavior we have on Desktop as well so any change should consider the compat impact as well.

@mazong1123

This comment has been minimized.

Copy link
Collaborator

commented Mar 29, 2017

@AlexGhiondea

Look at number.cpp, if the most significant 3 hex numbers are greater than or equal to 0x7FF , we'll hit the overflow exception:

   else if (exp >= 0x7FF) {
        // overflow
        val = I64(0x7FF0000000000000);
    }

In fact, the Double.MaxValue is 0x7FFEFFFFFFFFFFFE , which definitely enters into this trap. More over, try to convert any double number starts with 0x7FF will hit this exception. There's another code just check the first 3 hexes:

if (e == 0x7FF) {
    FC_RETURN_BOOL(false);
}

I've no idea if it is a bug (probably yes) or by design. We have to change the approach for detecting the overflow of double if we wanna fixing this issue.

@AlexGhiondea

This comment has been minimized.

Copy link
Member

commented Mar 31, 2017

@mazong1123 thanks for drilling into this!

@jkotas @joshfree do you guys have any context on this? I assume we can come up with other ways to check for overflow if we believe that is the right solution.

@jkotas

This comment has been minimized.

Copy link
Member

commented Mar 31, 2017

Floating point ToString() values do not roundtrip. If you need value that roundtrips, you have to use the "R" formatting string. Convert.ChangeType(Double.MaxValue.ToString("R"), typeof(Double)); works fine. It is how it has been in the framework since forever. I do not know why it was done that way - I agree that it is not a very intuitive design.

@jkotas

This comment has been minimized.

Copy link
Member

commented Mar 31, 2017

cc @tarekgh since he has been working on similar issues recently

@mazong1123

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2017

I think I've got more context about this issue during working on #10651 . The IEEE Double-precision floating-point format defined:

The exponents 0x000 and 0x7ff have a special meaning:
0x000 is used to represent a signed zero (if F=0) and subnormals (if F≠0); and
0x7ff is used to represent ∞ (if F=0) and NaNs (if F≠0),

I think that's why the code said if the exponent is 0x7ff we should throw overflow exception. I'll come back after #10651 resolved.

@mazong1123

This comment has been minimized.

Copy link
Collaborator

commented Oct 18, 2018

I think this issue is by design as Double.MaxValue.ToString() is not round-trippable as @jkotas said. Are we going to close this issue? @jkotas

@tarekgh

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

@tannergooding

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

Right, we would have to take a breaking change to double.ToString("G") to fix this, which might be acceptable if we fixed R to always print the shortest round-trippable string. There are a few papers on this (and some other languages try to do this), but I haven't dug into it any deeper yet.

@mazong1123

This comment has been minimized.

Copy link
Collaborator

commented Oct 20, 2018

@tannergooding I believe following issues & PRs are related:

#13106
dotnet/corefx#32268
#19905

I'm thinking if I can help on this issue. Due to my limited free time (only weekends), probably some minor or clean up jobs can assign to me so that @tannergooding may concentrate on the paper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.