BigInt to Number conversion
Categories
(Core :: JavaScript Engine, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: terpri, Assigned: terpri)
References
Details
Attachments
(2 files, 5 obsolete files)
1.87 KB,
patch
|
terpri
:
review+
|
Details | Diff | Splinter Review |
2.10 KB,
patch
|
terpri
:
review+
|
Details | Diff | Splinter Review |
Implement BigInt to Number conversion for the Number constructor.
Assignee | ||
Comment 1•6 years ago
|
||
ToNumeric is a generalized version of ToNumber that converts its argument to a numeric primitive value (either a Number or a BigInt).
Assignee | ||
Comment 2•6 years ago
|
||
Comment 3•6 years ago
|
||
Comment on attachment 8983488 [details] [diff] [review] Part 1: Implement ToNumeric operation. Review of attachment 8983488 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay, I forgot to get to this yesterday. ::: js/src/jsnum.cpp @@ +1626,5 @@ > + } > + > + // Step 2. > +#ifdef ENABLE_BIGINT > + if (vp.isBigInt()) { Nit: no {}
Comment 4•6 years ago
|
||
Comment on attachment 8983489 [details] [diff] [review] Part 2: Convert BigInt arguments to the Number constructor. Review of attachment 8983489 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/BigIntType.cpp @@ +197,5 @@ > +double > +BigInt::numberValue(BigInt* x) > +{ > + // mpz_get_d may cause a hardware overflow trap, so use > + // mpz_get_d_2exp to get the fractional part and exponent Is this (much) slower than mpz_get_d? Wondering if we should have a follow-up bug to optimize this later for BigInts that are in a "safe" range (if that even makes sense?).
Updated•6 years ago
|
Assignee | ||
Comment 5•6 years ago
|
||
remove a pair of braces, carry forward r+
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #4) > Comment on attachment 8983489 [details] [diff] [review] > Part 2: Convert BigInt arguments to the Number constructor. > > Review of attachment 8983489 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/vm/BigIntType.cpp > @@ +197,5 @@ > > +double > > +BigInt::numberValue(BigInt* x) > > +{ > > + // mpz_get_d may cause a hardware overflow trap, so use > > + // mpz_get_d_2exp to get the fractional part and exponent > > Is this (much) slower than mpz_get_d? Wondering if we should have a > follow-up bug to optimize this later for BigInts that are in a "safe" range > (if that even makes sense?). I'm not sure which is better, but both functions are basically wrappers for the internal mpn_get_d function, so I'll probably revisit this if SpiderMonkey switches to using the low-level GMP API. It should make sense to optimize it for small integers if GMP doesn't already.
Assignee | ||
Updated•6 years ago
|
Comment 7•6 years ago
|
||
Hi, when trying to apply part 1, I get the following error: applying bug-1466893---Part-2-Convert-BigInt-arguments-to-t.patch patching file js/src/vm/BigIntType.h Hunk #1 FAILED at 67 1 out of 1 hunks FAILED -- saving rejects to file js/src/vm/BigIntType.h.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh bug-1466893---Part-2-Convert-BigInt-arguments-to-t.patch :terpri, can you have a look at this?
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
ToNumeric is a generalized version of ToNumber that converts its argument to a numeric primitive value (either a Number or a BigInt).
Assignee | ||
Comment 9•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 10•6 years ago
|
||
ToNumeric is a generalized version of ToNumber that converts its argument to a numeric primitive value (either a Number or a BigInt).
Assignee | ||
Comment 11•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 12•6 years ago
|
||
(In reply to Eliza Balazs [:ebalazs_] from comment #7) > Hi, when trying to apply part 1, I get the following error: > applying bug-1466893---Part-2-Convert-BigInt-arguments-to-t.patch > patching file js/src/vm/BigIntType.h > Hunk #1 FAILED at 67 > 1 out of 1 hunks FAILED -- saving rejects to file js/src/vm/BigIntType.h.rej > patch failed, unable to continue (try -v) > patch failed, rejects left in working directory > errors during apply, please fix and qrefresh > bug-1466893---Part-2-Convert-BigInt-arguments-to-t.patch > :terpri, can you have a look at this? I rebased this onto mozilla-central (and exported through hg instead of git); this should apply correctly now.
Assignee | ||
Updated•6 years ago
|
Comment 13•6 years ago
|
||
Pushed by rgurzau@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/631e90e9e074 Part 1: Implement ToNumeric operation. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/7cdfba2700eb Part 2: Convert BigInt arguments to the Number constructor. r=jandem
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/631e90e9e074 https://hg.mozilla.org/mozilla-central/rev/7cdfba2700eb
Comment 15•5 years ago
|
||
small bug:
Number(100000000000000008193n)
returns 100000000000000000000
, while it should return 100000000000000016384
Note: Number(100000000000000008192n) === 100000000000000000000
see https://tc39.github.io/proposal-bigint/#sec-number-constructor-number-value
(rounding should be to nearest, ties to even)
Well, this bug is not important at all, :-)
Thank you!
Comment 16•5 years ago
|
||
(In reply to 4esn0k from comment #15)
Oh, you have switched to V8's implementation, so ignore my previous comment.
Comment 17•5 years ago
|
||
Firefox beta has the next issue:
Number(0b1000000000000000000000000000000000000000000000000000011n) gives 0b1000000000000000000000000000000000000000000000000000000, not
0b1000000000000000000000000000000000000000000000000000100
Comment 18•5 years ago
|
||
wingo, robin: thoughts on comment 17?
Comment 19•5 years ago
|
||
Filed bug 1558538 for comment 17, I done goofed bigly.
Description
•