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-
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
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
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
Keith Miller
Comment 1 2016-01-13 15:50:32 PST
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
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.