Bug 89321 – Move functions to create strings from integers out of JavaScriptCore and into WTF

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Darin Adler]

Move functions to create strings from integers out of JavaScriptCore and into WTF

[Darin Adler]

Created attachment 148037 [details]
Patch

[WebKit Review Bot]

Attachment 148037 [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
Source/WTF/wtf/text/Numeral.h:32:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:34:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:36:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:37:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:40:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:41:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:44:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:46:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 13 in 12 files


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

[Build Bot]

Comment on attachment 148037 [details]
Patch

Attachment 148037 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12979081

[Geoffrey Garen]

FWIW, llvm has a sub-library named llvm-mc, which can also do disassembly.

[Geoffrey Garen]

Oops. Wrong bug.

[Geoffrey Garen]

Comment on attachment 148037 [details]
Patch

Please fix Windows export:

17>WebCoreTestSupport.lib(Internals.obj) : error LNK2019: unresolved external symbol "public: static class WTF::PassRefPtr<class WTF::StringImpl> __cdecl WTF::StringImpl::createNumeral(unsigned long)" (?createNumeral@StringImpl@WTF@@SA?AV?$PassRefPtr@VStringImpl@WTF@@@2@K@Z) referenced in function "public: static class WTF::PassRefPtr<class WTF::StringImpl> __cdecl WTF::StringImpl::createNumeral(unsigned int)" (?createNumeral@StringImpl@WTF@@SA?AV?$PassRefPtr@VStringImpl@WTF@@@2@I@Z)

r=me

[Darin Adler]

Created attachment 148049 [details]
Patch with an attempt at Windows build fix for EWS

[WebKit Review Bot]

Attachment 148049 [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
Source/WTF/wtf/text/Numeral.h:32:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:34:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:36:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:37:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:40:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:41:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:44:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:46:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 13 in 13 files


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

[Build Bot]

Comment on attachment 148049 [details]
Patch with an attempt at Windows build fix for EWS

Attachment 148049 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12980059

[Darin Adler]

Created attachment 148230 [details]
Patch

[WebKit Review Bot]

Attachment 148230 [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
Source/WTF/wtf/text/Numeral.h:32:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:34:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:36:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:37:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:40:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:41:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:44:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:46:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 13 in 13 files


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

[Build Bot]

Comment on attachment 148230 [details]
Patch

Attachment 148230 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13000249

[Darin Adler]

Created attachment 149058 [details]
Patch

[WebKit Review Bot]

Attachment 149058 [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
Source/WTF/wtf/text/Numeral.h:32:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:34:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:35:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:36:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:37:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:38:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:40:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:41:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:42:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:43:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:44:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:45:  More than one command on the same line  [whitespace/newline] [4]
Source/WTF/wtf/text/Numeral.h:46:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 13 in 12 files


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

[Build Bot]

Comment on attachment 149058 [details]
Patch

Attachment 149058 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13032465

[Jessie Berlin]

You need to export a couple of symbols in WebKit2.def:

Index: Source/WebKit2/win/WebKit2.def
===================================================================
--- Source/WebKit2/win/WebKit2.def	(revision 121016)
+++ Source/WebKit2/win/WebKit2.def	(working copy)
@@ -166,6 +166,8 @@
         ?create@HTMLContentElement@WebCore@@SA?AV?$PassRefPtr@VHTMLContentElement@WebCore@@@WTF@@PAVDocument@2@@Z
         ?create@Range@WebCore@@SA?AV?$PassRefPtr@VRange@WebCore@@@WTF@@V?$PassRefPtr@VDocument@WebCore@@@4@V?$PassRefPtr@VNode@WebCore@@@4@H1H@Z
         ?create@ShadowRoot@WebCore@@SA?AV?$PassRefPtr@VShadowRoot@WebCore@@@WTF@@PAVElement@2@AAH@Z
+        ?createNumeral@StringImpl@WTF@@SA?AV?$PassRefPtr@VStringImpl@WTF@@@2@I@Z
+        ?createNumeral@StringImpl@WTF@@SA?AV?$PassRefPtr@VStringImpl@WTF@@@2@H@Z
         ?createWrapper@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@PAVJSDOMGlobalObject@1@PAVNode@1@@Z
         ?documentState@HistoryItem@WebCore@@QBEABV?$Vector@VString@WTF@@$0A@@WTF@@XZ
         ?equal@WTF@@YA_NPBVStringImpl@1@PBE@Z

[Darin Adler]

Thanks, Jessie! I got one (in an earlier version of the patch), but missed the other.

[Darin Adler]

Looks like someone did this, surprisingly similarly to what I had planned, but apparently not using my code.

[Benjamin Poulain]

(In reply to comment #18)
> Looks like someone did this, surprisingly similarly to what I had planned, but apparently not using my code.

I had originally planned to do it in two patches but you suggested to do everything at once https://bugs.webkit.org/show_bug.cgi?id=86312 :)

It is amazing how similar our conversion code looks.