Bug 66346 - Incorporate newer, faster dtoa library
Summary: Incorporate newer, faster dtoa library
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 67562
  Show dependency treegraph
 
Reported: 2011-08-16 16:06 PDT by Mark Hahnenberg
Modified: 2011-09-06 11:03 PDT (History)
10 users (show)

See Also:


Attachments
New dtoa library (337.98 KB, patch)
2011-08-16 16:41 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
New dtoa library (344.61 KB, patch)
2011-08-16 17:09 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing build errors (342.29 KB, patch)
2011-08-17 11:24 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing build errors (341.95 KB, patch)
2011-08-17 11:46 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing build errors (343.48 KB, patch)
2011-08-17 12:18 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing windows build (321.31 KB, patch)
2011-08-24 14:10 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing chromium build (321.21 KB, patch)
2011-08-24 14:59 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing chromium again (321.26 KB, patch)
2011-08-25 11:40 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing windows build again (323.88 KB, patch)
2011-08-25 15:40 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Fixing chromium tests (325.35 KB, patch)
2011-08-31 11:27 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Sunspider results (2.44 KB, text/plain)
2011-09-06 09:48 PDT, Mark Hahnenberg
no flags Details
V8 results (839 bytes, text/plain)
2011-09-06 09:48 PDT, Mark Hahnenberg
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2011-08-16 16:06:42 PDT
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!
Comment 1 Mark Hahnenberg 2011-08-16 16:07:47 PDT
<rdar://problem/9946258>
Comment 2 Mark Hahnenberg 2011-08-16 16:41:04 PDT
Created attachment 104118 [details]
New dtoa library
Comment 3 WebKit Review Bot 2011-08-16 16:44:46 PDT
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.
Comment 4 Mark Hahnenberg 2011-08-16 16:46:07 PDT
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.
Comment 5 Gyuyoung Kim 2011-08-16 16:46:29 PDT
Comment on attachment 104118 [details]
New dtoa library

Attachment 104118 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9402485
Comment 6 Early Warning System Bot 2011-08-16 16:54:06 PDT
Comment on attachment 104118 [details]
New dtoa library

Attachment 104118 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9407469
Comment 7 Mark Hahnenberg 2011-08-16 17:09:06 PDT
Created attachment 104124 [details]
New dtoa library
Comment 8 WebKit Review Bot 2011-08-16 17:12:32 PDT
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.
Comment 9 Gyuyoung Kim 2011-08-16 17:16:07 PDT
Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9401540
Comment 10 Early Warning System Bot 2011-08-16 17:26:09 PDT
Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9405438
Comment 11 WebKit Review Bot 2011-08-16 18:14:19 PDT
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
Comment 12 Collabora GTK+ EWS bot 2011-08-16 19:58:43 PDT
Comment on attachment 104124 [details]
New dtoa library

Attachment 104124 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9402608
Comment 13 Mark Hahnenberg 2011-08-17 11:24:52 PDT
Created attachment 104199 [details]
Fixing build errors
Comment 14 WebKit Review Bot 2011-08-17 11:26:40 PDT
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.
Comment 15 Gyuyoung Kim 2011-08-17 11:37:38 PDT
Comment on attachment 104199 [details]
Fixing build errors

Attachment 104199 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9420050
Comment 16 Early Warning System Bot 2011-08-17 11:43:47 PDT
Comment on attachment 104199 [details]
Fixing build errors

Attachment 104199 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9414780
Comment 17 Mark Hahnenberg 2011-08-17 11:46:18 PDT
Created attachment 104207 [details]
Fixing build errors
Comment 18 WebKit Review Bot 2011-08-17 12:03:48 PDT
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.
Comment 19 Early Warning System Bot 2011-08-17 12:11:23 PDT
Comment on attachment 104207 [details]
Fixing build errors

Attachment 104207 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9424009
Comment 20 Mark Hahnenberg 2011-08-17 12:18:05 PDT
Created attachment 104211 [details]
Fixing build errors
Comment 21 WebKit Review Bot 2011-08-17 12:21:55 PDT
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.
Comment 22 WebKit Review Bot 2011-08-17 15:33:31 PDT
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
Comment 23 Gavin Barraclough 2011-08-17 15:42:34 PDT
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.
Comment 24 Mark Rowe (bdash) 2011-08-17 21:19:40 PDT
Comment on attachment 104211 [details]
Fixing build errors

Why is none of the new code in WTF inside the WTF namespace?
Comment 25 Mark Hahnenberg 2011-08-24 14:10:29 PDT
Created attachment 105065 [details]
Fixing windows build
Comment 26 WebKit Review Bot 2011-08-24 14:14:02 PDT
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.
Comment 27 WebKit Review Bot 2011-08-24 14:25:10 PDT
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
Comment 28 Mark Hahnenberg 2011-08-24 14:59:42 PDT
Created attachment 105077 [details]
Fixing chromium build
Comment 29 WebKit Review Bot 2011-08-24 15:04:05 PDT
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.
Comment 30 WebKit Review Bot 2011-08-24 20:18:16 PDT
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
Comment 31 Mark Hahnenberg 2011-08-25 11:40:37 PDT
Created attachment 105227 [details]
Fixing chromium again
Comment 32 WebKit Review Bot 2011-08-25 11:43:21 PDT
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.
Comment 33 Mark Hahnenberg 2011-08-25 15:40:59 PDT
Created attachment 105266 [details]
Fixing windows build again
Comment 34 WebKit Review Bot 2011-08-25 15:43:40 PDT
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.
Comment 35 WebKit Review Bot 2011-08-25 18:02:56 PDT
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
Comment 36 Mark Hahnenberg 2011-08-31 11:27:10 PDT
Created attachment 105800 [details]
Fixing chromium tests
Comment 37 WebKit Review Bot 2011-08-31 11:30:22 PDT
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.
Comment 38 WebKit Review Bot 2011-09-02 15:08:46 PDT
Comment on attachment 105800 [details]
Fixing chromium tests

Clearing flags on attachment: 105800

Committed r94452: <http://trac.webkit.org/changeset/94452>
Comment 39 WebKit Review Bot 2011-09-02 15:08:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 40 Csaba Osztrogonác 2011-09-04 00:40:53 PDT
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?
Comment 41 Csaba Osztrogonác 2011-09-05 00:13:12 PDT
Fixed: http://trac.webkit.org/changeset/94514
Comment 42 Patrick R. Gansterer 2011-09-05 02:50:44 PDT
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!
Comment 43 Gavin Barraclough 2011-09-06 00:28:51 PDT
> 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?
Comment 44 Mark Hahnenberg 2011-09-06 09:48:27 PDT
Created attachment 106430 [details]
Sunspider results
Comment 45 Mark Hahnenberg 2011-09-06 09:48:53 PDT
Created attachment 106431 [details]
V8 results
Comment 46 Mark Hahnenberg 2011-09-06 09:50:04 PDT
> 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.
Comment 47 Gavin Barraclough 2011-09-06 11:03:13 PDT
Thanks Mark!