WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
67875
Remove toPrimitive from JSCell
https://bugs.webkit.org/show_bug.cgi?id=67875
Summary
Remove toPrimitive from JSCell
Mark Hahnenberg
Reported
2011-09-09 16:32:35 PDT
We should remove JSCell::toPrimitive in favor of JSValue::toPrimitive. The JSValue version in non-virtual, and we're trying to remove all virtual methods from JSCell.
Attachments
Patch
(10.58 KB, patch)
2011-09-09 17:23 PDT
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(6.46 KB, patch)
2011-09-16 12:56 PDT
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(8.60 KB, patch)
2011-09-19 11:28 PDT
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Patch
(7.60 KB, patch)
2011-09-19 15:01 PDT
,
Mark Hahnenberg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1
2011-09-09 17:23:39 PDT
Created
attachment 106945
[details]
Patch
Mark Hahnenberg
Comment 2
2011-09-13 15:05:25 PDT
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*.
Mark Hahnenberg
Comment 3
2011-09-16 12:56:25 PDT
Created
attachment 107708
[details]
Patch
WebKit Review Bot
Comment 4
2011-09-17 02:20:10 PDT
Comment on
attachment 107708
[details]
Patch
Attachment 107708
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/9729359
Geoffrey Garen
Comment 5
2011-09-18 15:44:50 PDT
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?
Mark Hahnenberg
Comment 6
2011-09-19 11:28:39 PDT
Created
attachment 107892
[details]
Patch
Geoffrey Garen
Comment 7
2011-09-19 12:06:35 PDT
Comment on
attachment 107892
[details]
Patch r=me
WebKit Review Bot
Comment 8
2011-09-19 13:31:59 PDT
Comment on
attachment 107892
[details]
Patch Clearing flags on attachment: 107892 Committed
r95466
: <
http://trac.webkit.org/changeset/95466
>
WebKit Review Bot
Comment 9
2011-09-19 13:32:04 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 10
2011-09-19 14:08:38 PDT
Why did this patch move JSValue::toPrimitive from JSCell.h to JSString.h? That change makes no sense to me!
Mark Hahnenberg
Comment 11
2011-09-19 14:19:45 PDT
(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.
Mark Hahnenberg
Comment 12
2011-09-19 15:01:42 PDT
Created
attachment 107922
[details]
Patch
WebKit Review Bot
Comment 13
2011-09-19 21:32:18 PDT
Comment on
attachment 107922
[details]
Patch Clearing flags on attachment: 107922 Committed
r95516
: <
http://trac.webkit.org/changeset/95516
>
WebKit Review Bot
Comment 14
2011-09-19 21:32:23 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug