Number.toExponential/toFixed/toPrecision all contain a spaghetti of duplicated code & unnecessary complexity. Add a new DecimalNumber class to encapsulate double to string conversion, share the implementations of rounding & decimal-fraction/exponential formatting. This patch changes some layout test results - in all these cases we were not previously accurate to spec defined behaviour, and we are still not - but overall this changes reduces the overall magnitude of error due to rounding differences. The underlying problem is that we should be using dtoa to generate results to a specified accuracy, rather than relying on pre-rounding the input values. . We should look at reenabling our dtoa implementation to work in this fashion as a separate change. Up to 2x progression on mircobenchmarks of toFixed/toPrecision for small results with low precision.
Created attachment 65210 [details] The Patch
Attachment 65210 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/DecimalNumber.h:26: #ifndef header guard has wrong style, please use: DecimalNumber_h [build/header_guard] [5] JavaScriptCore/wtf/DecimalNumber.h:105: Place brace on its own line for function definitions. [whitespace/braces] [4] JavaScriptCore/wtf/dtoa.cpp:2295: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 3 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 65210 [details] did not build on mac: Build output: http://queues.webkit.org/results/3739578
Attachment 65210 [details] did not build on qt: Build output: http://queues.webkit.org/results/3744618
Created attachment 65213 [details] Fix couple of issues with last patch.
Attachment 65213 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/DecimalNumber.h:105: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 65214 [details] Fixes for JSVALUE32_64 builds.
Attachment 65214 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/DecimalNumber.h:105: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 65213 [details] did not build on mac: Build output: http://queues.webkit.org/results/3772598
Attachment 65213 [details] did not build on qt: Build output: http://queues.webkit.org/results/3733605
Attachment 65214 [details] did not build on qt: Build output: http://queues.webkit.org/results/3754561
Attachment 65214 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3754563
Attachment 65214 [details] did not build on win: Build output: http://queues.webkit.org/results/3792353
Attachment 65214 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3731549
I love this patch, do you know what is causing all the EWS to be red?
(In reply to comment #15) > I love this patch, do you know what is causing all the EWS to be red? Looks like I need to include #include <math.h> for Qt, Windows will certainly need export changes. Should be trivial things. Will look at uploading a new patch in a bit.
Created attachment 65321 [details] Missing include
Attachment 65321 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/DecimalNumber.h:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 65321 [details] Missing include r=me fix the bloody style issue!!!!
Fixed in r65959.