WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
153081
[ES6] Support subclassing Function.
https://bugs.webkit.org/show_bug.cgi?id=153081
Summary
[ES6] Support subclassing Function.
Keith Miller
Reported
2016-01-13 15:01:16 PST
[ES6] Support subclassing Function.
Attachments
Patch
(13.07 KB, patch)
2016-01-13 15:50 PST
,
Keith Miller
ggaren
: review+
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ews117 for mac-yosemite
(805.18 KB, application/zip)
2016-01-13 16:52 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews100 for mac-yosemite
(887.41 KB, application/zip)
2016-01-13 16:53 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2
(801.09 KB, application/zip)
2016-01-13 16:58 PST
,
Build Bot
no flags
Details
View All
Add attachment
proposed patch, testcase, etc.
Keith Miller
Comment 1
2016-01-13 15:50:32 PST
Created
attachment 268908
[details]
Patch
Geoffrey Garen
Comment 2
2016-01-13 16:07:07 PST
Comment on
attachment 268908
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=268908&action=review
r=me
> Source/JavaScriptCore/ChangeLog:8 > + This patch adds subclassing the Function object. It also, fixes an existing
No comma.
> Source/JavaScriptCore/ChangeLog:10 > + the superclass' prototype property.
superclass's.
> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3230 > + // FIXME: It's possible that these checks should be a single bytecode like: op_is_object_type.
This shouldn't be a FIXME. You should weight the tradeoffs now, and decide. If you decide this should change, you should file a bug. I don't think it's very bad to have two branches here, since class construction is rare.
Mark Lam
Comment 3
2016-01-13 16:08:15 PST
Comment on
attachment 268908
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=268908&action=review
>> Source/JavaScriptCore/ChangeLog:8 >> + This patch adds subclassing the Function object. It also, fixes an existing > > No comma.
missing "to" in "subclassing *to* the"?
> Source/JavaScriptCore/runtime/JSFunction.h:155 > - JSFunction(VM&, FunctionExecutable*, JSScope*); > +// JSFunction(VM&, FunctionExecutable*, JSScope*);
Delete this instead of commenting out.
Keith Miller
Comment 4
2016-01-13 16:43:45 PST
Comment on
attachment 268908
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=268908&action=review
>>> Source/JavaScriptCore/ChangeLog:8 >>> + This patch adds subclassing the Function object. It also, fixes an existing >> >> No comma. > > missing "to" in "subclassing *to* the"?
Fixed both issues. But I changed the first sentence to "This patch adds subclassing the Function object." => "This patch enables subclassing the Function object."
>> Source/JavaScriptCore/ChangeLog:10 >> + the superclass' prototype property. > > superclass's.
Fixed.
>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3230 >> + // FIXME: It's possible that these checks should be a single bytecode like: op_is_object_type. > > This shouldn't be a FIXME. You should weight the tradeoffs now, and decide. If you decide this should change, you should file a bug. > > I don't think it's very bad to have two branches here, since class construction is rare.
I agree that it's not too bad. Not only are class creations rare, but it's also rare to have the .prototype property be a function.
>> Source/JavaScriptCore/runtime/JSFunction.h:155 >> +// JSFunction(VM&, FunctionExecutable*, JSScope*); > > Delete this instead of commenting out.
Fixed.
Keith Miller
Comment 5
2016-01-13 16:45:14 PST
It looks like the test failures are just a missed rebaseline of class-syntax-extends.js. I will update before I land when the rest of the builders are done.
Build Bot
Comment 6
2016-01-13 16:52:29 PST
Comment on
attachment 268908
[details]
Patch
Attachment 268908
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/688069
New failing tests: js/class-syntax-extends.html
Build Bot
Comment 7
2016-01-13 16:52:31 PST
Created
attachment 268915
[details]
Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 8
2016-01-13 16:53:18 PST
Comment on
attachment 268908
[details]
Patch
Attachment 268908
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/688078
New failing tests: js/class-syntax-extends.html
Build Bot
Comment 9
2016-01-13 16:53:20 PST
Created
attachment 268917
[details]
Archive of layout-test-results from ews100 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 10
2016-01-13 16:58:04 PST
Comment on
attachment 268908
[details]
Patch
Attachment 268908
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/688081
New failing tests: js/class-syntax-extends.html
Build Bot
Comment 11
2016-01-13 16:58:05 PST
Created
attachment 268919
[details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Keith Miller
Comment 12
2016-01-14 12:46:02 PST
Committed
r195070
: <
http://trac.webkit.org/changeset/195070
>
Ryan Haddad
Comment 13
2016-01-14 13:40:43 PST
** The following JSC stress test failures have been introduced: es6.yaml/es6/Function_is_subclassable_Function.prototype.apply.js.default es6.yaml/es6/Function_is_subclassable_Function.prototype.call.js.default es6.yaml/es6/Function_is_subclassable_can_be_called.js.default es6.yaml/es6/Function_is_subclassable_can_be_used_with_new.js.default es6.yaml/es6/Function_is_subclassable_correct_prototype_chain.js.default <
https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20JSC%20%28Tests%29/builds/2483
>
Keith Miller
Comment 14
2016-01-14 13:43:02 PST
Fixing. Not sure why those didn't come up locally... I figured those ones would fail too but I should have checked they were marked as normal.
Ryan Haddad
Comment 15
2016-01-14 13:50:19 PST
(In reply to
comment #14
)
> Fixing. Not sure why those didn't come up locally... I figured those ones > would fail too but I should have checked they were marked as normal.
Thank you!
Keith Miller
Comment 16
2016-01-14 14:45:35 PST
Tests should be fixed in
https://trac.webkit.org/r195077
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