Bug 71597 - Reduce the number of putWithAttributes
Summary: Reduce the number of putWithAttributes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-04 17:08 PDT by Sam Weinig
Modified: 2011-11-06 13:09 PST (History)
2 users (show)

See Also:


Attachments
Patch (6.85 KB, patch)
2011-11-04 17:13 PDT, Sam Weinig
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2011-11-04 17:08:07 PDT
Reduce the number of putWithAttributes
Comment 1 Sam Weinig 2011-11-04 17:13:41 PDT
Created attachment 113736 [details]
Patch
Comment 2 WebKit Review Bot 2011-11-04 17:15:10 PDT
Attachment 113736 [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/JavaScriptCore/runtime/JSObject.h:110:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Adam Roben (:aroben) 2011-11-04 17:16:46 PDT
Comment on attachment 113736 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113736&action=review

Wait for the EWS bots so you don't break everyone else.

> Source/JavaScriptCore/ChangeLog:19
> +        Remove four unused JSObject::putWithAttributes overloads and remove make
> +        one of the remain two overloads not virtual, since no one overloads it.

Typo: "remove make one of the remain two"
Typo: overloads should be overrides
Comment 4 Sam Weinig 2011-11-05 13:29:07 PDT
Committed r99357: <http://trac.webkit.org/changeset/99357>
Comment 5 Ryosuke Niwa 2011-11-06 12:48:29 PST
It appears that this patch caused one jsc regression?
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/34474
Comment 6 Sam Weinig 2011-11-06 13:06:54 PST
(In reply to comment #5)
> It appears that this patch caused one jsc regression?
> http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/34474

I don't think so. Those test fail from occasionally based no the current time.
Comment 7 Ryosuke Niwa 2011-11-06 13:09:33 PST
(In reply to comment #6)
> (In reply to comment #5)
> > It appears that this patch caused one jsc regression?
> > http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/34474
> 
> I don't think so. Those test fail from occasionally based no the current time.

I see. Thanks for the clarification.