885798 - Add new ES6 Number constants: EPSILON, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brandon Benvie [:benvie]]

From ES6 spec (May 2013 draft).

Number.EPSILON = 2.220446049250313e-16 (section 15.7.3.7)
Number.MAX_INTEGER = 9007199254740991 (section 15.7.3.8)

Comment 1

[Brandon Benvie [:benvie]]

Sorry, EPSILON is 2.2204460492503130808472633361816e-16 (rounded a bit)

Comment 2

[Till Schneidereit [:till]]

Attachment: Add new ES6 Number constants

As described

Comment 3

[Till Schneidereit [:till]]

Attachment: Add new ES6 Number constants

A qref later: tests!

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 771762 [details] [diff] [review]
Add new ES6 Number constants

Review of attachment 771762 [details] [diff] [review]:
-----------------------------------------------------------------

So, this is basically fine.  But I have concerns about the values being exposed here -- see latest es-discuss -- so I'd like you to hold off on landing this til those questions/concerns are resolved.

::: js/src/jsnum.cpp
@@ +1027,5 @@
>      {0,                         "MIN_VALUE",         0,{0,0,0}},
> +    /* ES6 (May 2013 draft) 15.7.3.7 */
> +    {2.2204460492503130808472633361816e-16, "EPSILON", 0,{0,0,0}},
> +    /* ES6 (May 2013 draft) 15.7.3.8 */
> +    {9007199254740991,          "MAX_INTEGER",       0,{0,0,0}},

I'd prefer (1ULL << 53) - 1 here.

::: js/src/tests/ecma_6/Number/15.7.3.7-EPSILON.js
@@ +11,5 @@
> + * BEGIN TEST *
> + **************/
> +
> +// Test value
> +assertEq(Number.EPSILON, 2.2204460492503130808472633361816e-16);

I'd prefer seeing Math.pow(2, -52) instead here.  (If there were a way to write that out in C++ as a constant expression, I'd want it there, too, but there isn't one.  :-( )

@@ +13,5 @@
> +
> +// Test value
> +assertEq(Number.EPSILON, 2.2204460492503130808472633361816e-16);
> +
> +// Test [[Writable]]: false

Seems better to me to test writability, enumerability, etc. using Object.getOwnPropertyDescriptor, rather than testing the observable effects of these.

::: js/src/tests/ecma_6/Number/15.7.3.8-MAX_INTEGER.js
@@ +11,5 @@
> + * BEGIN TEST *
> + **************/
> +
> +// Test value
> +assertEq(Number.MAX_INTEGER, 9007199254740991);

Math.pow(2, 53) - 1 as well here.  Same enumerability/configurability/writability test request too.

Comment 5

[Till Schneidereit [:till]]

Attachment: Add ES6 Number constants EPSILON and MAX_INTEGER.

Patch with review comments addressed. I'll wait for a final resolution of the discussion on es-discuss[1] before pushing this. It looks like this should be fine as-is, though.

[1]: https://mail.mozilla.org/pipermail/es-discuss/2013-July/031660.html

Comment 6

[Till Schneidereit [:till]]

I pushed a patch containing just EPSILON, for now, and will wait for an outcome of the es-discuss(ion).

https://hg.mozilla.org/integration/mozilla-inbound/rev/43744955a192

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/43744955a192

Comment 8

[Jean-Yves Perrier [:teoli]]

If I understand well, the Number.EPSILON will be in Firefox 25 (if not uplifted, backed out between now and the release). Am I correct?

Comment 9

[4esn0k]

>>I'd prefer seeing Math.pow(2, -52) instead here.  (If there were a way to write that out in C++ as a constant expression, I'd want it there, too, but there isn't one.  :-( )

nextafter(1) - 1

see http://www.cplusplus.com/reference/cmath/nextafter/

Comment 10

[4esn0k]

ah, it is not constant expr., sorry

Comment 11

[Till Schneidereit [:till]]

Catching up with the latest tc39 meeting notes, I found this discussion of Number.MAX_INTEGER:

https://github.com/rwldrn/tc39-notes/blob/master/es6/2013-07/july-25.md#59-semantics-and-bounds-of-numberisinteger-and-numbermax_integer

Personally, I find MM's war story fairly compelling. Waldo, what sayeth you?

Comment 12

[Jeff Walden [:Waldo]]

I don't quite see how his story makes a difference one way or the other.  If the constant is 2**53, you can do < 2**53 to admit only "safe integers" that match his definition.  If the constant is 2**53 - 1, you can do <= for identical semantics.  It's all well and good to not consider 2**53 part of the "safe integer" set, but that doesn't say what constant should be exposed.

I'm having difficulty reading those notes.  Is

"""
MM:

MAX_SAFE_INTEGER = (2^53)-1

isInteger

    Infinity => false
    NaN => false
    value !== truncated value => false
    -0 => true

isSafeInteger

    -0 => true

toInteger

    Does not guarantee a safe integer

ToInteger

    Does not guarantee a safe integer
"""

the part where they actually decided anything specific?

Comment 13

[Jean-Yves Perrier [:teoli]]

I made a first draft for Number.EPSILON doc:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/EPSILON
and added info there:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number
https://developer.mozilla.org/en-US/docs/Web/JavaScript/ECMAScript_6_support_in_Mozilla
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/25

Comment 14

[Rick Waldron [:rwaldron]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #12)
> I don't quite see how his story makes a difference one way or the other.  If
> the constant is 2**53, you can do < 2**53 to admit only "safe integers" that
> match his definition.  If the constant is 2**53 - 1, you can do <= for
> identical semantics.  It's all well and good to not consider 2**53 part of
> the "safe integer" set, but that doesn't say what constant should be exposed.
> 
> I'm having difficulty reading those notes.  Is
> 
> """
> MM:
> 
> MAX_SAFE_INTEGER = (2^53)-1
> 
> isInteger
> 
>     Infinity => false
>     NaN => false
>     value !== truncated value => false
>     -0 => true
> 
> isSafeInteger
> 
>     -0 => true
> 
> toInteger
> 
>     Does not guarantee a safe integer
> 
> ToInteger
> 
>     Does not guarantee a safe integer
> """
> 
> the part where they actually decided anything specific?

I apologize for the lack of clear resolution in the minutes. I've posted to es-discuss: https://mail.mozilla.org/pipermail/es-discuss/2013-August/032990.html

Comment 15

[Rick Waldron [:rwaldron]]

Mark and Allen confirmed on that thread: 

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-15.7.2.8 
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-15.7.2.13 
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-15.7.2.14 


The following is copied from Mark's response for convenience:

> > Number.MAX_SAFE_INTEGER = (2^53)-1;
> 
> yes. Presumably
> 
> Number.MIN_SAFE_INTEGER = -Number.MAX_SAFE_INTEGER;
> 
> Number.isSafeInteger(n) {
>     return typeof n === 'number' && 
>         Math.round(n) === n && 
>         Number.MIN_SAFE_INTEGER <= n &&
>         n <= Number.MAX_SAFE_INTEGER;
> }
> 
> This has the important guarantee that
> 
> isSafeInteger(a) && isSafeInteger(b) && isSafeInteger(a+b) together imply that a+b is the > accurate sum of a and b.
> 
> Also
> 
> isSafeInteger(NaN) // false
> isSafeInteger(new Integer(2)) // false
> isSafeInteger(-0) // true
> isSafeInteger(Infinity) // false
> 
> We also decided that toInteger is not needed given the other similar coercion methods 
> already on Math.

Comment 16

[Stephen Kelly]

Attachment: Add Number.{MAX,MIN}_SAFE_INTEGER

Comment 17

[Tom S]

Comment on attachment 8413194 [details] [diff] [review]
Add Number.{MAX,MIN}_SAFE_INTEGER

Review of attachment 8413194 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect. Thank you for your first contribution.

Comment 18

[Till Schneidereit [:till]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/90515fc1d2cd

Thanks for the contribution! \o/

Comment 19

[Stephen Kelly]

Attachment: Extend the nc_slots enum with new values

This amends the previous commit from the bug report.

The lack of this extension happens not to matter because the
NC_EPSILON and NC_LIMIT enum values happen to not be used
in the file currently.

Comment 20

[Till Schneidereit [:till]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0d7bf90091bb

Thanks again for the follow-up, and especially for seeing it in the first place!

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/90515fc1d2cd
https://hg.mozilla.org/mozilla-central/rev/0d7bf90091bb

Comment 22

[u279076]

Assuming this does not need QA before release. Please needinfo me if you think this needs testing.

Comment 23

[Florian Scholz (Open Web Docs)]

MAX_INTEGER got never pushed (see comment 6) and is also not part of ES6. Changing the summary.

Comment 24

[Florian Scholz (Open Web Docs)]

See comment #13 for Number.EPSILON docs (Firefox 25).

Firefox 31:
https://developer.mozilla.org/en-US/Firefox/Releases/31#JavaScript

References:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER