Bug 33163 – Unify JSC::UStringImpl's & WebCore::StringImpl's string hashing methods.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

Copy & paste is great.

[Gavin Barraclough]

Created attachment 45810 [details]
The patch

[WebKit Review Bot]

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

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/runtime/UStringImpl.h:163:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/runtime/UStringImpl.h:164:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/wtf/HashFunctions.h:217:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/HashFunctions.h:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/HashFunctions.h:294:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 5

[WebKit Review Bot]

Attachment 45810 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/161642

[Sam Weinig]

Comment on attachment 45810 [details]
The patch

YES.

[Gavin Barraclough]

Created attachment 45826 [details]
make inclusion of the wtf/unicode/Unicode.h explicit.

[WebKit Review Bot]

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

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/runtime/UStringImpl.h:163:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/runtime/UStringImpl.h:164:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/wtf/HashFunctions.h:25:  wtf includes should be <wtf/file.h> instead of "wtf/file.h".  [build/include] [4]
JavaScriptCore/wtf/HashFunctions.h:217:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/HashFunctions.h:255:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/HashFunctions.h:294:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 6

[Gavin Barraclough]

Comment on attachment 45826 [details]
make inclusion of the wtf/unicode/Unicode.h explicit.

Clearing review flag, landing based on Sam's original r+

- only set review on this to get the chromium buildbot to take a sniff at it. :-)

[Gavin Barraclough]

Committed revision 52758.

[Eric Seidel (no email)]

I do not understand why, but this change seems to have broken all of the bots:

http://trac.webkit.org/changeset/52758
http://build.webkit.org/builders/Leopard%20Intel%20Release%20%28Tests%29/builds/8930
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/3854
http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/8585

Compiles are failing.  Either of DumpRenderTree support library or for windows:
   Creating library C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\lib\JavaScriptCore.lib and object C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\lib\JavaScriptCore.exp
JavaScriptCore.exp : error LNK2001: unresolved external symbol "public: static unsigned int __cdecl JSC::UStringImpl::computeHash(char const *,int)" (?computeHash@UStringImpl@JSC@@SAIPBDH@Z)
C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\bin\JavaScriptCore.dll : fatal error LNK1120: 1 unresolved externals

I recommend we roll this out.

[Eric Seidel (no email)]

Mark rolled it out in http://trac.webkit.org/changeset/52768.

[Gavin Barraclough]

Fixed, with thanks to bdash, relanding.
Committed revision 52776.