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

Fix the rounding issue #2027

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@FrankYFTang
Copy link
Contributor

FrankYFTang commented Jan 9, 2019

No description provided.

@FrankYFTang

This comment has been minimized.

Copy link
Contributor

FrankYFTang commented Jan 9, 2019

@sffc

This comment has been minimized.

Copy link

sffc commented Jan 9, 2019

The spec says to use half-up rounding for both fraction and scientific.

https://www.ecma-international.org/ecma-402/5.0/index.html#sec-torawprecision

Let e and n be integers such that 10p–1 ≤ n < 10p and for which the exact mathematical value of n × 10e–p+1 – x is as close to zero as possible. If there are two such sets of e and n, pick the e and n for which n × 10e–p+1 is larger.

and

https://www.ecma-international.org/ecma-402/5.0/index.html#sec-torawfixed

Let n be an integer for which the exact mathematical value of n ÷ 10f – x is as close to zero as possible. If there are two such n, pick the larger n.

In both cases, if there is a tie, pick the "larger" one. This part of the spec operates only on positive numbers, so this means to round away from zero.

@sffc

sffc approved these changes Jan 9, 2019

@@ -23,8 +23,8 @@ var numberingSystems = [
];
var testData = {
// Ref tc39/ecma402#128

This comment has been minimized.

@ljharb

ljharb Jan 9, 2019

Member

this comment links to the still-open issue: tc39/ecma402#128 - are we confident about the conclusion both there and here?

@sffc

This comment has been minimized.

Copy link

sffc commented Jan 9, 2019

I've commented before on tc39/ecma402#128, and the current web reality, which is consistent with my interpretation of the spec, is that rounding behavior in the spec should not be influenced by floating-point noise.

(123.445).toLocaleString("en", {maximumFractionDigits:2})  // 123.45 in Chrome and Firefox

I also found the PR that added the current test262 behavior: #857

It does make sense to resolve tc39/ecma402#128 first, and then update test262.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment