Bug 36041

Summary:

Remove unnecessary differences in common code between WebCore::StringImpl & JSC::UStringImpl

Product:

WebKit

Reporter:

Gavin Barraclough <barraclough>

Component:

WebCore JavaScript

Assignee:

Gavin Barraclough <barraclough>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Not setting r flag, abusing bugzilla to give me a pretty diff to get better comment per-function changes in my ChangeLog entries!	none
The patch	oliver: review+

Gavin Barraclough

Reported 2010-03-11 17:42:10 PST

Much of the code in WebCore::StringImpl and JSC::UStringImpl is now very similar, but has trivial and unnecessary formatting differences, such as the exact wording of comments, missing ASSERTs, functions implemented in the .h vs .cpp etc.

Attachments
Not setting r flag, abusing bugzilla to give me a pretty diff to get better comment per-function changes in my ChangeLog entries! (36.68 KB, patch) 2010-03-11 17:44 PST, Gavin Barraclough	no flags	Details Formatted Diff Diff
The patch (37.75 KB, patch) 2010-03-11 18:26 PST, Gavin Barraclough	oliver: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Gavin Barraclough

Comment 1 2010-03-11 17:44:19 PST

Created attachment 50564 [details] Not setting r flag, abusing bugzilla to give me a pretty diff to get better comment per-function changes in my ChangeLog entries!

Gavin Barraclough

Comment 2 2010-03-11 18:26:50 PST

Created attachment 50569 [details] The patch Shows as a ~0.5% - 1% progression on SunSpider on my laptop. This may be noise, through is repeatable & predominately seemed to come from moving the infrequently used UStringImpl::create() methods from the .h into the .cpp.

WebKit Review Bot

Comment 3 2010-03-11 18:29:29 PST

Attachment 50569 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/runtime/UStringImpl.h:256: More than one command on the same line [whitespace/newline] [4] JavaScriptCore/runtime/UStringImpl.h:256: More than one command on the same line in if [whitespace/parens] [4] JavaScriptCore/runtime/UStringImpl.h:257: More than one command on the same line [whitespace/newline] [4] WebCore/platform/text/StringImpl.h:170: More than one command on the same line [whitespace/newline] [4] WebCore/platform/text/StringImpl.h:170: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 5 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.

Oliver Hunt

Comment 4 2010-03-11 19:08:05 PST

Comment on attachment 50569 [details] The patch r=me

Gavin Barraclough

Comment 5 2010-03-11 19:15:01 PST

Comitted r55878

Note You need to log in before you can comment on or make changes to this bug.