RESOLVED FIXED 153799
[JSC] Implement Object.getOwnPropertyDescriptors() proposal
https://bugs.webkit.org/show_bug.cgi?id=153799
Summary [JSC] Implement Object.getOwnPropertyDescriptors() proposal
Caitlin Potter (:caitp)
Reported 2016-02-02 13:34:55 PST
[JSC] Implement Object.getOwnPropertyDescriptors() proposal
Attachments
[JSC] Implement Object.getOwnPropertyDescriptors() proposal (13.84 KB, patch)
2016-02-02 13:36 PST, Caitlin Potter (:caitp)
no flags
Patch (13.85 KB, patch)
2016-02-02 13:52 PST, Caitlin Potter (:caitp)
no flags
Archive of layout-test-results from ews103 for mac-yosemite (902.75 KB, application/zip)
2016-02-02 14:39 PST, Build Bot
no flags
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (748.08 KB, application/zip)
2016-02-02 14:44 PST, Build Bot
no flags
Archive of layout-test-results from ews117 for mac-yosemite (817.07 KB, application/zip)
2016-02-02 14:45 PST, Build Bot
no flags
Patch (17.09 KB, patch)
2016-02-02 14:51 PST, Caitlin Potter (:caitp)
no flags
Patch (16.90 KB, patch)
2016-02-02 18:12 PST, Caitlin Potter (:caitp)
no flags
Caitlin Potter (:caitp)
Comment 1 2016-02-02 13:36:22 PST
Created attachment 270511 [details] [JSC] Implement Object.getOwnPropertyDescriptors() proposal
Caitlin Potter (:caitp)
Comment 2 2016-02-02 13:37:55 PST
This proposal (https://github.com/tc39/proposal-object-getownpropertydescriptors) was bumped to Stage 3 at the last meeting, thought I'd send a patch for implementing it. Tests are ported from my V8 implementation, which is itself influenced by the new Test262 entries
Caitlin Potter (:caitp)
Comment 3 2016-02-02 13:52:37 PST
Geoffrey Garen
Comment 4 2016-02-02 14:15:29 PST
Comment on attachment 270513 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270513&action=review r=me > Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js:85 > +// FIXME: enable test once Proxy has been implemented. See below. > Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js:125 > +// FIXME: enable test once Proxy has been implemented. Let's make this a separate test, expected to fail. That way, updating it once we have Proxies will be automatic.
Build Bot
Comment 5 2016-02-02 14:39:46 PST
Comment on attachment 270513 [details] Patch Attachment 270513 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/774088 New failing tests: js/Object-getOwnPropertyNames.html
Build Bot
Comment 6 2016-02-02 14:39:51 PST
Created attachment 270518 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 7 2016-02-02 14:44:11 PST
Comment on attachment 270513 [details] Patch Attachment 270513 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/774091 New failing tests: js/Object-getOwnPropertyNames.html
Build Bot
Comment 8 2016-02-02 14:44:16 PST
Created attachment 270519 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 9 2016-02-02 14:45:17 PST
Comment on attachment 270513 [details] Patch Attachment 270513 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/774080 New failing tests: js/Object-getOwnPropertyNames.html
Build Bot
Comment 10 2016-02-02 14:45:22 PST
Created attachment 270521 [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
Caitlin Potter (:caitp)
Comment 11 2016-02-02 14:51:25 PST
Caitlin Potter (:caitp)
Comment 12 2016-02-02 14:52:35 PST
Comment on attachment 270513 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270513&action=review >> Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js:125 >> +// FIXME: enable test once Proxy has been implemented. > > Let's make this a separate test, expected to fail. That way, updating it once we have Proxies will be automatic. Done, and fixed the failing layout test
Caitlin Potter (:caitp)
Comment 13 2016-02-02 14:52:39 PST
Comment on attachment 270513 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270513&action=review >> Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js:125 >> +// FIXME: enable test once Proxy has been implemented. > > Let's make this a separate test, expected to fail. That way, updating it once we have Proxies will be automatic. Done, and fixed the failing layout test
Darin Adler
Comment 14 2016-02-02 16:47:31 PST
Comment on attachment 270524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270524&action=review r=ggaren > Source/JavaScriptCore/ChangeLog:21 > + * runtime/ObjectConstructor.cpp: > + (JSC::objectConstructorGetOwnPropertyDescriptors): > + * runtime/ObjectConstructor.h: > + * tests/es6.yaml: > + * tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js: Added. > + (shouldBe): > + (shouldThrow): > + (shouldBeDataProperty): > + * tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js: Added. > + (shouldBe): > + (shouldThrow): > + (shouldBeDataProperty): > + (testMeta): > + (testPrototypeProperties.F): Not a great change log. Would be nice to write comments here and remove the lists of function names where they aren’t helpful. > Source/JavaScriptCore/runtime/ObjectConstructor.cpp:261 > + for (auto it = properties.begin(); it != properties.end(); ++it) { > + const Identifier& propertyName = *it; Should use a modern for loop: for (auto& propertyName : properties) {
Caitlin Potter (:caitp)
Comment 15 2016-02-02 18:12:59 PST
Caitlin Potter (:caitp)
Comment 16 2016-02-02 18:13:54 PST
Comment on attachment 270524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270524&action=review >> Source/JavaScriptCore/ChangeLog:21 >> + (testPrototypeProperties.F): > > Not a great change log. Would be nice to write comments here and remove the lists of function names where they aren’t helpful. Done >> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:261 >> + const Identifier& propertyName = *it; > > Should use a modern for loop: > > for (auto& propertyName : properties) { Done
WebKit Commit Bot
Comment 17 2016-02-02 19:17:59 PST
Comment on attachment 270538 [details] Patch Clearing flags on attachment: 270538 Committed r196040: <http://trac.webkit.org/changeset/196040>
WebKit Commit Bot
Comment 18 2016-02-02 19:18:07 PST
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.