Closed
Bug 885798
Opened 11 years ago
Closed 10 years ago
Add new ES6 Number constants: EPSILON, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla31
Tracking | Status | |
---|---|---|
firefox31 | --- | fixed |
People
(Reporter: bbenvie, Assigned: steveire)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, feature, Whiteboard: [js:t][qa-][DocArea=JS])
Attachments
(3 files, 2 obsolete files)
3.52 KB,
patch
|
Details | Diff | Splinter Review | |
3.22 KB,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
1.06 KB,
patch
|
Details | Diff | Splinter Review |
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)
Reporter | ||
Comment 1•11 years ago
|
||
Sorry, EPSILON is 2.2204460492503130808472633361816e-16 (rounded a bit)
Updated•11 years ago
|
Assignee: general → till
Updated•11 years ago
|
Attachment #771708 -
Attachment is obsolete: true
Attachment #771708 -
Flags: review?(jwalden+bmo)
Comment 4•11 years ago
|
||
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.
Attachment #771762 -
Flags: review?(jwalden+bmo) → review+
Comment 5•11 years ago
|
||
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
Updated•11 years ago
|
Attachment #771762 -
Attachment is obsolete: true
Comment 6•11 years ago
|
||
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
Status: NEW → ASSIGNED
Whiteboard: [leave open][js:t]
Comment 7•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/43744955a192
Updated•11 years ago
|
Keywords: dev-doc-needed
Comment 8•11 years ago
|
||
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?
>>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•11 years ago
|
||
ah, it is not constant expr., sorry
Comment 11•11 years ago
|
||
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?
Flags: needinfo?(jwalden+bmo)
Comment 12•10 years ago
|
||
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?
Flags: needinfo?(jwalden+bmo)
Comment 13•10 years ago
|
||
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•10 years ago
|
||
(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•10 years ago
|
||
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.
Assignee | ||
Comment 16•10 years ago
|
||
Updated•10 years ago
|
Attachment #8413194 -
Flags: review?(evilpies)
Comment 17•10 years ago
|
||
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.
Attachment #8413194 -
Flags: review?(evilpies) → review+
Comment 18•10 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/90515fc1d2cd Thanks for the contribution! \o/
Updated•10 years ago
|
Whiteboard: [leave open][js:t] → [js:t]
Updated•10 years ago
|
Keywords: feature
Summary: Add new ES6 Number constants → Add new ES6 Number constants: EPSILON, MAX_INTEGER, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER
Assignee | ||
Comment 19•10 years ago
|
||
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•10 years ago
|
||
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!
Assignee: till → steveire
Comment 21•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/90515fc1d2cd https://hg.mozilla.org/mozilla-central/rev/0d7bf90091bb
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 22•10 years ago
|
||
Assuming this does not need QA before release. Please needinfo me if you think this needs testing.
status-firefox31:
--- → fixed
Whiteboard: [js:t] → [js:t][qa-]
Updated•10 years ago
|
Whiteboard: [js:t][qa-] → [js:t][qa-][DocArea=JS]
Comment 23•10 years ago
|
||
MAX_INTEGER got never pushed (see comment 6) and is also not part of ES6. Changing the summary.
Summary: Add new ES6 Number constants: EPSILON, MAX_INTEGER, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER → Add new ES6 Number constants: EPSILON, MAX_SAFE_INTEGER, MIN_SAFE_INTEGER
Comment 24•10 years ago
|
||
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
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•