Bug 172129 – WebAssembly: exports is a getter

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[JF Bastien]

Make sure we have the right semantics: https://github.com/WebAssembly/design/pull/1062

[JF Bastien]

Created attachment 310489 [details]
patch

[JF Bastien]

Created attachment 310490 [details]
patch

Rebase.

[Saam Barati]

Comment on attachment 310490 [details]
patch

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

r=me

> JSTests/wasm/js-api/test_basic_api.js:74
> +        assert.throws(() => WebAssembly.Instance.prototype.exports = undefined, TypeError, `Attempted to assign to readonly property.`);

Please add a test where you do getOwnProperty and assert the property descriptor is what you expect. Also, please add tests where you throw because the |this| is not an instance.

> Source/JavaScriptCore/wasm/js/WebAssemblyInstancePrototype.cpp:48
> + exports webAssemblyInstanceProtoFuncExports DontEnum|Accessor 0

Is it a configurable property?

[JF Bastien]

(In reply to Saam Barati from comment #3)
> Comment on attachment 310490 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310490&action=review
> 
> r=me
> 
> > JSTests/wasm/js-api/test_basic_api.js:74
> > +        assert.throws(() => WebAssembly.Instance.prototype.exports = undefined, TypeError, `Attempted to assign to readonly property.`);
> 
> Please add a test where you do getOwnProperty and assert the property
> descriptor is what you expect.

AFAICT a [[Get]] without [[Set]] has getOwnPropertyDescriptor of {"enumerable":false,"configurable":true} does that seem right to you? I'm not super clear on property descriptors, and getters...

None of the accessors specify their own property info.

> Also, please add tests where you throw
> because the |this| is not an instance.

Will do, I tested locally and forgot to add.

> > Source/JavaScriptCore/wasm/js/WebAssemblyInstancePrototype.cpp:48
> > + exports webAssemblyInstanceProtoFuncExports DontEnum|Accessor 0
> 
> Is it a configurable property?

[Saam Barati]

(In reply to JF Bastien from comment #4)
> (In reply to Saam Barati from comment #3)
> > Comment on attachment 310490 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=310490&action=review
> > 
> > r=me
> > 
> > > JSTests/wasm/js-api/test_basic_api.js:74
> > > +        assert.throws(() => WebAssembly.Instance.prototype.exports = undefined, TypeError, `Attempted to assign to readonly property.`);
> > 
> > Please add a test where you do getOwnProperty and assert the property
> > descriptor is what you expect.
> 
> AFAICT a [[Get]] without [[Set]] has getOwnPropertyDescriptor of
> {"enumerable":false,"configurable":true} does that seem right to you? I'm
> not super clear on property descriptors, and getters...
> 
> None of the accessors specify their own property info.
It looks like we should default to configurable:false
https://tc39.github.io/ecma262/#table-4

> 
> > Also, please add tests where you throw
> > because the |this| is not an instance.
> 
> Will do, I tested locally and forgot to add.
> 
> > > Source/JavaScriptCore/wasm/js/WebAssemblyInstancePrototype.cpp:48
> > > + exports webAssemblyInstanceProtoFuncExports DontEnum|Accessor 0
> > 
> > Is it a configurable property?

[JF Bastien]

Created attachment 310607 [details]
patch

From the discussion it sounds like the enumerable / configurable properties are sill up in the air. I forked to an issue:
  https://github.com/WebAssembly/design/issues/1069
I'll create an issue here and fix if things change (for all accessors, not just this one).

[WebKit Commit Bot]

Comment on attachment 310607 [details]
patch

Rejecting attachment 310607 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 310607, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in JSTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/3773872

[JF Bastien]

Created attachment 310609 [details]
patch

No oops.

[WebKit Commit Bot]

Comment on attachment 310609 [details]
patch

Clearing flags on attachment: 310609

Committed r217097: <http://trac.webkit.org/changeset/217097>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.