Bug 147883 – [ES6] Implement computed accessors

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Geoffrey Garen]

Example test case:

=====

var x = 'y',
    valueSet,
    obj = {
      get [x] () { return 1 },
      set [x] (value) { valueSet = value }
    };
obj.y = 'foo';
return obj.y === 1 && valueSet === 'foo';

=====

This will probably require three new opcodes: op_put_getter_by_val, op_put_setter_by_val, op_put_getter_setter_by_val.

Like the equivalent by_id opcodes, these opcodes will mostly be a clone of get_by_val and put_by_val, with slightly different behavior for being getters and setters, and for not consulting the prototype chain.

[Matthew Mirman]

Created attachment 259029 [details]
Parsing Patch

[Filip Pizlo]

Comment on attachment 259029 [details]
Parsing Patch

Do any of these tests cover actually loading or storing to the accessor property?

[Ryosuke Niwa]

Comment on attachment 259029 [details]
Parsing Patch

This patch doesn't handle interactions with non-accessor properties of the same name.
e.g.
var foo = 'a';
{a: 1, get [foo]() {}}
{get [foo]() {}, a: 1}
See http://trac.webkit.org/changeset/184324 which addressed this problem for accessors with regular name.
We probably need to add new op codes to add accessors by value.

[Ryosuke Niwa]

Also see https://bugs.webkit.org/show_bug.cgi?id=142690.

[Matthew Mirman]

(In reply to comment #4)
> Also see https://bugs.webkit.org/show_bug.cgi?id=142690.

(In reply to comment #2)
> Comment on attachment 259029 [details]
> Parsing Patch
> 
> Do any of these tests cover actually loading or storing to the accessor
> property?

As suggested by ggaren, this patch only addresses parsing and will still fail if the code is run.

[Matthew Mirman]

Created attachment 259042 [details]
Parsing Patch

Adds support for classes.

Some overlap with https://bugs.webkit.org/show_bug.cgi?id=142690  will need to pull before submitting.

[Ryosuke Niwa]

Comment on attachment 259042 [details]
Parsing Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=259042&action=review

> Source/JavaScriptCore/ChangeLog:4
> +        Implements parsing for computed accessor properties on getters and setters.
> +        https://bugs.webkit.org/show_bug.cgi?id=147883

Can we file a new WebKit bug that blocks this bug and post this patch there?
This bug is about implementing the full feature.

[Build Bot]

Comment on attachment 259042 [details]
Parsing Patch

Attachment 259042 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/59403

New failing tests:
js/parser-syntax-check.html

[Build Bot]

Created attachment 259048 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 259042 [details]
Parsing Patch

Attachment 259042 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/59400

New failing tests:
js/parser-syntax-check.html

[Build Bot]

Created attachment 259049 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Yusuke Suzuki]

I'll extract Matthew's parsing patch to the separated issue.

[Yusuke Suzuki]

Created attachment 260661 [details]
Patch

WIP: not tested yet

[Yusuke Suzuki]

Since the executing part does not become so large, I'll attempt to implement parsing & executing part in the one patch in this bug.

[Yusuke Suzuki]

Created attachment 260772 [details]
Patch

[Geoffrey Garen]

Comment on attachment 260772 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 260772 [details]
Patch

Clearing flags on attachment: 260772

Committed r189504: <http://trac.webkit.org/changeset/189504>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.