Bug 33163 - Unify JSC::UStringImpl's & WebCore::StringImpl's string hashing methods.
Summary: Unify JSC::UStringImpl's & WebCore::StringImpl's string hashing methods.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-04 11:32 PST by Gavin Barraclough
Modified: 2010-01-04 17:34 PST (History)
3 users (show)

See Also:


Attachments
The patch (13.77 KB, patch)
2010-01-04 11:36 PST, Gavin Barraclough
sam: review+
Details | Formatted Diff | Diff
make inclusion of the wtf/unicode/Unicode.h explicit. (13.79 KB, patch)
2010-01-04 13:29 PST, Gavin Barraclough
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2010-01-04 11:32:15 PST
Copy & paste is great.
Comment 1 Gavin Barraclough 2010-01-04 11:36:03 PST
Created attachment 45810 [details]
The patch
Comment 2 WebKit Review Bot 2010-01-04 11:40:47 PST
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
Comment 3 WebKit Review Bot 2010-01-04 11:41:02 PST
Attachment 45810 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/161642
Comment 4 Sam Weinig 2010-01-04 11:42:13 PST
Comment on attachment 45810 [details]
The patch

YES.
Comment 5 Gavin Barraclough 2010-01-04 13:29:02 PST
Created attachment 45826 [details]
make inclusion of the wtf/unicode/Unicode.h explicit.
Comment 6 WebKit Review Bot 2010-01-04 13:31:13 PST
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
Comment 7 Gavin Barraclough 2010-01-04 13:48:05 PST
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. :-)
Comment 8 Gavin Barraclough 2010-01-04 14:18:45 PST
Committed revision 52758.
Comment 9 Eric Seidel (no email) 2010-01-04 16:29:30 PST
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.
Comment 10 Eric Seidel (no email) 2010-01-04 16:35:16 PST
Mark rolled it out in http://trac.webkit.org/changeset/52768.
Comment 11 Gavin Barraclough 2010-01-04 17:34:41 PST
Fixed, with thanks to bdash, relanding.
Committed revision 52776.