Summary: | Remove toPrimitive from JSCell | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Hahnenberg <mhahnenberg> | ||||||||||
Component: | JavaScriptCore | Assignee: | Mark Hahnenberg <mhahnenberg> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | darin, ggaren, oliver, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 68389 | ||||||||||||
Bug Blocks: | 67690 | ||||||||||||
Attachments: |
|
Description
Mark Hahnenberg
2011-09-09 16:32:35 PDT
Created attachment 106945 [details]
Patch
Comment on attachment 106945 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106945&action=review > Source/JavaScriptCore/ChangeLog:9 > + all of the implicit functionality provided by the virutal toPrimitive method sp > Source/JavaScriptCore/runtime/JSObject.h:911 > + return *const_cast<JSValue*>(this); no const_cast, please > Source/JavaScriptCore/runtime/JSObject.h:912 > + if (isCell()) Test isCell first for efficiency. Create separate toPrimitives for JSCell* and JSObject*. Created attachment 107708 [details]
Patch
Comment on attachment 107708 [details] Patch Attachment 107708 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9729359 Comment on attachment 107708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107708&action=review > Source/JavaScriptCore/runtime/JSCell.cpp:124 > + if (isString()) > + return ((JSString*)this)->toPrimitive(exec, preferredType); > + return ((JSObject*)this)->toPrimitive(exec, preferredType); Use C++ static_cast, please. It's more explicit, and provides a little more type safety than a C-style cast. Are the bot complaints a real issue? Created attachment 107892 [details]
Patch
Comment on attachment 107892 [details]
Patch
r=me
Comment on attachment 107892 [details] Patch Clearing flags on attachment: 107892 Committed r95466: <http://trac.webkit.org/changeset/95466> All reviewed patches have been landed. Closing bug. Why did this patch move JSValue::toPrimitive from JSCell.h to JSString.h? That change makes no sense to me! (In reply to comment #10) > Why did this patch move JSValue::toPrimitive from JSCell.h to JSString.h? That change makes no sense to me! Good question. This is the corrected (for compilation) version of an incorrect (for style) version. Rolling it out to resubmit the corrected (for both) version. Created attachment 107922 [details]
Patch
Comment on attachment 107922 [details] Patch Clearing flags on attachment: 107922 Committed r95516: <http://trac.webkit.org/changeset/95516> All reviewed patches have been landed. Closing bug. |