Bug 207297 – [JSC] Computed function properties compute their keys twice

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ross Kirsling]

Basic repro:

    ```
    let count1 = 0;
    let key1 = { toString() { count1++; return 'foo'; } };
    ({ [key1]: 'bar' });

    let count2 = 0;
    let key2 = { toString() { count2++; return 'foo'; } };
    ({ [key2]() { return 'bar'; } });

    print(count1, count2);
    ```

    λ eshost -s test.js
    #### ch, sm, v8, xs
    1 1

    #### jsc
    1 2

Notes:

- This also reproes if:
  - we write the function property as `[key]: function () { return 'bar'; }` or `[key]: () => 'bar'` instead
  - we set toString to null and use valueOf instead

- Relevant test262 failures:
  https://github.com/tc39/test262/blob/master/test/language/computed-property-names/to-name-side-effects/class.js
  https://github.com/tc39/test262/blob/master/test/language/computed-property-names/to-name-side-effects/numbers-class.js
  https://github.com/tc39/test262/blob/master/test/language/computed-property-names/to-name-side-effects/object.js

- Bug 170934 seems related but separate.

[Ross Kirsling]

Created attachment 390185 [details]
Patch

[Ross Kirsling]

Comment on attachment 390185 [details]
Patch

The current approach is very simple but incurs a ToPropertyKey instruction even for non-function computed properties.

I suppose it would be better to split emitSetFunctionNameIfNeeded into needsSetFunctionName and emitSetFunctionName so that this can be avoided.

[Ross Kirsling]

Created attachment 390291 [details]
Patch

[Ross Kirsling]

Ping?

[Keith Miller]

Comment on attachment 390291 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 390291 [details]
Patch

Clearing flags on attachment: 390291

Committed r256846: <https://trac.webkit.org/changeset/256846>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59559881>