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

toPrecision() in Edge gives incorrect output in rare cases #3929

Closed
dajp1 opened this issue Oct 11, 2017 · 8 comments
Closed

toPrecision() in Edge gives incorrect output in rare cases #3929

dajp1 opened this issue Oct 11, 2017 · 8 comments
Assignees
Labels
Milestone

Comments

@dajp1
Copy link

dajp1 commented Oct 11, 2017

My apologies if this is the wrong place to submit this.

I have found what I suspect is a bug in Edge. In JavaScript, the lines:

a = 0.0003;
b = a.toPrecision(3);

result in b containing "0.000200". (IE, Chrome and Firefox all give the "0.000300" that I was expecting.)

See http://jsfiddle.net/dajp/2b47qLg0/

I'm using Edge 40.15063.0.0.
This is causing me some difficulties. Many thanks.

@MikeHolman
Copy link
Contributor

Definitely seems like a real bug and something we should fix. That code is crazy complicated, so I can't immediately understand the problem.

Looks like something is going wrong inside NumberUtilities::FDblToStr's call to FDblToRgbPrecise though.

@MikeHolman MikeHolman added this to the vNext milestone Oct 18, 2017
@MikeHolman MikeHolman added the Bug label Oct 18, 2017
@curtisman curtisman modified the milestones: vNext, 1.8 Nov 29, 2017
@obastemur
Copy link
Collaborator

0.0003 IEEE754 => 0.0003000000142492353916168212890625

So, toPrecision(3) indeed should not return 0.0002

@obastemur
Copy link
Collaborator

obastemur commented Nov 29, 2017

@dajp1 the expected output for your sample should be 0.00 not 0.0003 (since the precision is 3 and there are three zeroes pre any number.) When precision is set to 4 ChakraCore calculates the correct result.

However ChakraCore (as reported) results in 0.0002 when precision is 3.

@dajp1
Copy link
Author

dajp1 commented Nov 29, 2017

@obastemur I don't agree. As far as I can see, the specification says that toPrecision() should return a string to the specified number of significant digits, and examples always start with the first non-zero digit.

In my case I was expecting "0.000300" since that has three significant digits starting with the first non-zero digit, and this is what I get on every browser I have tried except Edge. If precision was set to 4, I would expect "0.0003000", which is indeed what I get (on Edge, IE, Firefox and Chrome). The bug only appears when the number of significant digits is set to three or fewer.

@dajp1 dajp1 closed this as completed Nov 29, 2017
@dajp1 dajp1 reopened this Nov 29, 2017
@dajp1
Copy link
Author

dajp1 commented Nov 29, 2017

Sorry - newbie mistake, I pressed the wrong button. Didn't mean to close this issue.

@obastemur
Copy link
Collaborator

obastemur commented Nov 29, 2017

As far as I can see, the specification says that toPrecision() should return a string to the specified number of significant digits, and examples always start with the first non-zero digit.

I agree on that. A counting issue(mixed with toFixed :/ ) on my end. Well, either way ChakraCore was returning the wrong result :)

chakrabot pushed a commit that referenced this issue Dec 1, 2017
…on to number of digits

Merge pull request #4352 from obastemur:fp_tp_fix

Previously, `normalizeHBound` was being marked based on caller's interest with number of digits. However, the result was missing higherBound push when `ndigit == ib`. (border case)

Fixes #3929
@redchair123
Copy link

Stumbled upon a similar issue with (-6.9e-11).toExponential(4). Edge 13/14 give the correct string '-6.9000e-11' but 15/16 give '-6.8000e-11'. @obastemur does the patch also cover this case?

@obastemur
Copy link
Collaborator

Just tested. Yes, the fix covers that case too.

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

No branches or pull requests

5 participants