Number.prototype.toString has rounding errors with non-decimal radix
Reported by
christ...@burschka.de,
Nov 17 2016
|
||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.0 Safari/537.36 Steps to reproduce the problem: Run: for (i=2; i <= 10; i++) console.log(i, ((i+1)/i).toString(i)); What is the expected behavior? Expected output: 2 "1.1" 3 "1.1" 4 "1.1" 5 "1.1" 6 "1.1" 7 "1.1" 8 "1.1" 9 "1.1" 10 "1.1" What went wrong? Actual output: 2 "1.1" 3 "1.0222222222222222222222222222222221100120012110201212210012221212021101101000220212102010210201021211112122002022112121112020202222110221220100011202022110012121202022222220110002110102100211220222020210020202212200001200210102221010121122221220202202101211002201110201020011020010120112020101120222101101120010212221112101122210220221101022211220211202122120112102221022201110200002121200001211111220201222011102012020212120122221111100201020201100212010020220011120021012112022020121122021022210122221010202012020111011012020020221121011020202210012102010012011011210221222222121122220002220021222211122022110010201211102022111012202120110221212121222121121020102110101011110212010122100111112202120000001020221000202220000101221202102000200202102110110111002220021201021210122112012100002001211100001200202202121022010121110112101210010222210010111210222202002011211110102102011220202202200010022102022121221121020120102012201000210020211202202212201011112002222020010201021101112101120122220202100011001211111211212020220202112021222112220210200200020022121000021012101110221200121122221200112211" 4 "1.1" 5 "1.0444444444444444444444424201223414320341214044210313403144024244033410023123330313200132101410244340311014040100100020410013221331022241023222114233403232043101401214440133242040101444444204112130313411140410124300020201203012301243134332443203240114414214020243304113340202114042013100332403204234440423240402021121020323403340410333401144032244212221231214434422214130441430411412031430003021230411334402421414441332233222220441313322403202212343043213010140211012404212124344203102320330410220123003213231304201143231344444432111410020112034112012114312404101142444024340303213140041222332441430342434204134132314131042122120440403433112123043403134104412330041314134332304340204210433423304022112112210323341322421042401214431001134211431014404213032103224243424011241224002021424134313433134311413434224344433123113413401110323013101234332202314430200433122114233403430212221320231134103421422433223232304343431420404213032202022400430330241442031440021004141123001402013023214334231220413024030400423142230142022221123421004331243220110414214231302042144043040143201321032033011300344443322441" 6 "1.1000000000000000000013424133010154403315300325144043" 7 "1.0666666666666666666113343451123605541130235106023063301226600455416230634044204116003511321402023210646062611156445505556451406515466031331053235430616135644203556516330541054215600346421226512536051500225541412252443056631433200016412160255665243135365156020514161663045304016631612526632302356012456550563653021303330206130631660411055246246115114366344431063606131301132222022363620632112261101015606325501236552200420160513212101213015005122521466353413410216064245043202232342006012362430013531426534266541321440652533352163226625343160530635211016111600233661362503623045523465541416402355100263253526416025603536644566552646064420552100264553332244626631133604150246510500250605003201140336532455560446234152316151411212514210122452623362211146446430521104553316143546534230360335646250200131146015654262605151505140314406063004152133103123362104126266456463346301642632453214463326166551240641656221335345146614254121453321205230232000423110655265432451032004412151320501346013205656530661661545624022554223321422351010120162666036030030005201242052464246351346633545411466324132364055352311" 8 "1.1" 9 "1.1000000000000000073580711608601631536040508611814341211433253020605288026124523855214074405020286823276506203121716828113050352146340658663160361413683021806328021062703253505551417261632187687735013075814182375142305742178040226435807484354002643337656125232070274816288842843368805518111862771663534015277205373820243383226837671420644234100701261805010475140204403788524335223033403518356836113178232664763863303055111632784733127125767255670325107667250530675003023038568187735045824868122637155084671374356408521535475786688436665166423817652611662032551034348846822126204701825846105611634405021505426000535680446555410366332305507852336368301756083524062816221003417822534513371360721617464486234104615804824810143535116474347353063611226077205585458687416811040210254066263241276424168461803317138354123657370534148814083064368270661204143328873060353531082814844071835602528181213547555460372534502504614135323236138160572682656538388445284633186211241283526408403843041037222042511708220805406401678452225443770624673567471052157135722562683117558810713147855275708773335158357753036866738" 10 "1.1" Did this work before? N/A Chrome version: 56.0.2924.0 Channel: dev OS Version: Flash Version: The rounding error seems to appear in all bases except 10 and 2^n.
,
Nov 17 2016
It would be great if we have a sample test case as a repo. This will help us in bisecting to find the regressed build. M54 is not a realistic milestone for a fix, but team is on the process of triaging and will add appropriate Milestone.
,
Nov 18 2016
Able to reproduce the issue on MAC 10.11.6, Windows 10 and Ubuntu 14.04 using chrome reported version #56.0.2924.0 and latest canary #56.0.2922.0. This is a non regression issue as it is observed from M30, M35, M40, M45 and M50 old builds. Hence, marking it as untriaged to get more inputs from dev team. Thanks...!!
,
Nov 22 2016
FP operations and this is probably within the spec but still it feels kinda off a lot especially looking at 3 and 5. Is this something we want to fix?
,
Nov 22 2016
https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html IEEE floats can only represent multiples of 2 accurately.
,
Nov 22 2016
That's not really a justification, because decimals like 1.1 *are* represented accurately when printed, even though they have no exact IEEE representation. So if the NumberToString algorithm for base 10 approximates to the shortest round-trip representation, why not let DoubleToRadixCString do the same thing for different bases? However, I've since found the actual V8 code and seen that the problem is already explained in this TODO comment: https://github.com/v8/v8/blob/b104a67ef0a50beab2dc85a6ac5732c696d07698/src/conversions.cc#L458 Even if this is within the spec, it doesn't feel like a good implementation as (without a way to specify a precision, which the spec doesn't have) there's no way to get usable representations for non-whole numbers. (For comparison, the spidermonkey implementation yields the shortest round-trip representation in every base: https://github.com/mozilla/gecko-dev/blob/afc64a42d6a2362c01b2c81d5a14c26ba3f3c485/js/src/jsdtoa.cpp#L299 )
,
Nov 22 2016
Interestingly, Safari and Firefox seem to print out 1.1 on all iterations for this test case. Firefox seems to have gotten their code from an earlier version of V8, actually. I wonder if we used to give shorter output here.
,
Nov 22 2016
,
Nov 22 2016
Firefox only uses V8's algorithm for base 10
,
Nov 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/21b0dbedfd470b67c47f84428e1af95a506e49e5 commit 21b0dbedfd470b67c47f84428e1af95a506e49e5 Author: yangguo <yangguo@chromium.org> Date: Thu Nov 24 10:30:19 2016 Reimplement Number.prototype.toString with non-default radix. The old algorithm produces unnecessary decimal digits. The new one converts the significand of the input double into an uint64_t to be just as precise as necessary. R=tebbi@chromium.org BUG= chromium:658712 , chromium:666376 Review-Url: https://codereview.chromium.org/2520363002 Cr-Commit-Position: refs/heads/master@{#41255} [modify] https://crrev.com/21b0dbedfd470b67c47f84428e1af95a506e49e5/src/builtins/builtins-number.cc [modify] https://crrev.com/21b0dbedfd470b67c47f84428e1af95a506e49e5/src/conversions.cc [modify] https://crrev.com/21b0dbedfd470b67c47f84428e1af95a506e49e5/test/mjsunit/number-tostring.js [modify] https://crrev.com/21b0dbedfd470b67c47f84428e1af95a506e49e5/test/webkit/fast/js/number-toString-expected.txt [modify] https://crrev.com/21b0dbedfd470b67c47f84428e1af95a506e49e5/test/webkit/fast/js/toString-number-expected.txt |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by schenney@chromium.org
, Nov 17 2016