Bug 66346 – Incorporate newer, faster dtoa library

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Hahnenberg]

Our current dtoa library is a bit dated.  There is a newer, faster one that has been lifted out of the V8 project.  We should use it!

[Mark Hahnenberg]

<rdar://problem/9946258>

[Mark Hahnenberg]

Created attachment 104118 [details]
New dtoa library

[WebKit Review Bot]

Attachment 104118 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:66:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:44:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:45:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:51:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:77:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:78:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:85:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:89:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:97:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:112:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:113:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:116:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Hahnenberg]

The style bot freaked out because the new library was extracted from V8, and Google's coding style is like the bitwise-not of ours.  We should probably refrain from modifying the library to fit our style so we can more easily update it if any changes are made to it upstream.

[Gyuyoung Kim]

Comment on attachment 104118 [details]
New dtoa library

Attachment 104118 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9402485

[Early Warning System Bot]

Comment on attachment 104118 [details]
New dtoa library

Attachment 104118 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9407469

[Mark Hahnenberg]

Created attachment 104124 [details]
New dtoa library

[WebKit Review Bot]

Attachment 104124 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:66:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:44:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:45:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:51:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:77:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:78:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:85:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:89:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:97:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:112:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:113:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:116:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9401540

[Early Warning System Bot]

Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9405438

[WebKit Review Bot]

Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9408408

[Collabora GTK+ EWS bot]

Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9402608

[Mark Hahnenberg]

Created attachment 104199 [details]
Fixing build errors

[WebKit Review Bot]

Attachment 104199 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:66:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:44:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:45:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:51:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:77:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:78:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:85:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:89:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:97:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:112:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:113:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:116:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 104199 [details]
Fixing build errors

Attachment 104199 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9420050

[Early Warning System Bot]

Comment on attachment 104199 [details]
Fixing build errors

Attachment 104199 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9414780

[Mark Hahnenberg]

Created attachment 104207 [details]
Fixing build errors

[WebKit Review Bot]

Attachment 104207 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:66:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:44:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:45:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:51:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:77:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:78:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:85:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:89:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:97:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:112:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:113:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:116:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 104207 [details]
Fixing build errors

Attachment 104207 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9424009

[Mark Hahnenberg]

Created attachment 104211 [details]
Fixing build errors

[WebKit Review Bot]

Attachment 104211 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:66:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:44:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:45:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:51:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:77:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:78:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:85:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:89:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:97:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:106:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:107:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:112:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:113:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:116:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 104211 [details]
Fixing build errors

Attachment 104211 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9414929

New failing tests:
animations/combo-transform-rotate+scale.html
animations/animation-shorthand.html
animations/matrix-anim.html
animations/fill-mode-transform.html
animations/change-one-anim.html
animations/animation-matrix-negative-scale-unmatrix.html
animations/computed-style.html
animations/keyframes-rule.html
http/tests/appcache/video.html
animations/combo-transform-translate+scale.html
animations/lineheight-animation.html
animations/fill-unset-properties.html
animations/big-rotation.html
animations/missing-from-to-transforms.html
animations/keyframe-timing-functions-transform.html
animations/dynamic-stylesheet-loading.html
animations/animation-shorthand-removed.html
animations/animation-direction-normal.html
canvas/philip/tests/2d.fillStyle.get.semitransparent.html
animations/animation-add-events-in-handler.html

[Gavin Barraclough]

Hey Mark,

From a quick look at this patch, it looks like you might be changing WebCore's CSS code to use the same number-to-string conversion as for JS?  Sorry if I'm mistaken, but if so, this isn't correct - the CSS spec prohibits exponential representation, all values should be simple decimal.

G.

[Mark Rowe (bdash)]

Comment on attachment 104211 [details]
Fixing build errors

Why is none of the new code in WTF inside the WTF namespace?

[Mark Hahnenberg]

Created attachment 105065 [details]
Fixing windows build

[WebKit Review Bot]

Attachment 105065 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:72:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:46:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:47:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:62:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:73:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:80:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:87:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:91:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:114:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:115:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:122:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 105065 [details]
Fixing windows build

Attachment 105065 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9502005

[Mark Hahnenberg]

Created attachment 105077 [details]
Fixing chromium build

[WebKit Review Bot]

Attachment 105077 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:72:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:46:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:47:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:62:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:73:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:80:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:87:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:91:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:114:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:115:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:122:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 105077 [details]
Fixing chromium build

Attachment 105077 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9495193

New failing tests:
fast/css/readonly-pseudoclass-opera-003.html
http/tests/media/video-cancel-load.html
fast/css/shadow-dom-scope.html
fast/css/pseudo-required-optional-005.html
http/tests/media/video-error-abort.html
http/tests/appcache/video.html
fast/css/unknown-pseudo-element-matching.html
http/tests/security/local-video-poster-from-remote.html
fast/css/first-letter-block-form-controls-crash.html
http/tests/media/video-served-as-text.html
http/tests/media/text-served-as-text.html
http/tests/media/reload-after-dialog.html
fast/dom/HTMLInputElement/input-slider-update.html
http/tests/media/pdf-served-as-pdf.html
fast/css/pseudo-in-range-invalid-value.html
fast/css/pseudo-in-range.html
fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
fast/dom/HTMLInputElement/input-slider-update-styled.html
fast/dom/boolean-attribute-reflection.html
http/tests/media/media-can-load-when-hidden.html

[Mark Hahnenberg]

Created attachment 105227 [details]
Fixing chromium again

[WebKit Review Bot]

Attachment 105227 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:72:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:46:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:47:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:62:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:73:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:80:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:87:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:91:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:114:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:115:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:122:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Hahnenberg]

Created attachment 105266 [details]
Fixing windows build again

[WebKit Review Bot]

Attachment 105266 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:72:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:46:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:47:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:62:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:73:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:80:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:87:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:91:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:114:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:115:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:122:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 105266 [details]
Fixing windows build again

Attachment 105266 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9511745

New failing tests:
fast/css/readonly-pseudoclass-opera-003.html
http/tests/media/video-cancel-load.html
fast/css/shadow-dom-scope.html
fast/css/pseudo-required-optional-005.html
http/tests/media/video-error-abort.html
http/tests/appcache/video.html
fast/css/unknown-pseudo-element-matching.html
http/tests/security/local-video-poster-from-remote.html
fast/css/first-letter-block-form-controls-crash.html
http/tests/media/video-served-as-text.html
http/tests/media/text-served-as-text.html
http/tests/media/reload-after-dialog.html
fast/dom/HTMLInputElement/input-slider-update.html
http/tests/media/pdf-served-as-pdf.html
fast/css/pseudo-in-range-invalid-value.html
fast/css/pseudo-in-range.html
fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
fast/dom/HTMLInputElement/input-slider-update-styled.html
fast/dom/boolean-attribute-reflection.html
http/tests/media/media-can-load-when-hidden.html

[Mark Hahnenberg]

Created attachment 105800 [details]
Fixing chromium tests

[WebKit Review Bot]

Attachment 105800 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:68:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/cached-powers.h:72:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:28:  #ifndef header guard has wrong style, please use: WTF_diy_fp_h  [build/header_guard] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:46:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:47:  Missing space inside { }.  [whitespace/braces] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:62:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:73:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:80:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:87:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:91:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  set_f is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:108:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  set_e is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:109:  _value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:114:  f_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:115:  e_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:118:  One space before end of line comments  [whitespace/comments] [5]
Source/JavaScriptCore/wtf/dtoa/diy-fp.h:122:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 234 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 105800 [details]
Fixing chromium tests

Clearing flags on attachment: 105800

Committed r94452: <http://trac.webkit.org/changeset/94452>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Reopen the bug report, because it  broke many JSC tests and layout tests on the Qt bot. Could you guys respect the WebKit policies and watch the bots?

[Csaba Osztrogonác]

Fixed: http://trac.webkit.org/changeset/94514

[Patrick R. Gansterer]

Committed r94520: <http://trac.webkit.org/changeset/94520>

I agree with ossy: Please watch the bots!
Do we have any guidelines for 3rd party code? The change breaks many of our coding style guidelines and I don't know why even the simple ones (like include config.h) get ignored.
The new dtoa library does some stuff too, which is already done in WTF. I'm not a big fan of maintaining the #ifdef twice.

And finally: What does "The new library is much faster than the old one." mean exactly? I usually get a clear r-, when I don't include exact performance data into the ChangeLog!

[Gavin Barraclough]

> Do we have any guidelines for 3rd party code? The change breaks many of our coding style guidelines and I don't know why even the simple ones (like include config.h) get ignored.

There is precedent to having some flexibility on coding style for 3rd party code, since this makes it easier to continue to integrate future improvements.  However yes, one would expect some adaption to fit within our codebase, and yes, files definitely need to be including config.h.

> The new dtoa library does some stuff too, which is already done in WTF. I'm not a big fan of maintaining the #ifdef twice.

OOI are you just referring to replicating existing behavior of dtoa & DecimalNumber? - or of other basic types?  If re dtoa & DecimalNumber, then I believe the intention is to deprecate the existing code, but that this will involve a bit of work (specifically because these are used from WebCore by CSS code, that requires slightly different dtoa behavior that the new library may require a little adaption to produce).

> And finally: What does "The new library is much faster than the old one." mean exactly? I usually get a clear r-, when I don't include exact performance data into the ChangeLog!

I have to agree on this one, we really should be justifying changes like this with performance numbers.  Mark, what does this progress? - could you post some SunSpider/V8 benchmark numbers?

[Mark Hahnenberg]

Created attachment 106430 [details]
Sunspider results

[Mark Hahnenberg]

Created attachment 106431 [details]
V8 results

[Mark Hahnenberg]

> I have to agree on this one, we really should be justifying changes like this with performance numbers.  Mark, what does this progress? - could you post some SunSpider/V8 benchmark numbers?

Sorry about that.  I attached some results to the Radar bug but neglected to do that here as well.

[Gavin Barraclough]

Thanks Mark!