[spec] Initial JS API formal spec for the GC proposal #352
Conversation
This commit adds formal spec test for the JS API portion of the GC proposal. The semantics is based on the "no-frills" JS API proposal in the following document: https://docs.google.com/document/d/17hCQXOyeSgogpJ0I0wir4LRmdvu4l7Oca6e1NkbVN8M/ The text assumes that the Wasm GC formal spec will expose certain operations to the embedder, such as `ref.cast`. Comments have been left where references between specs will need to be fixed up.
document/js-api/index.bs
Outdated
| 1. If |v| is an [=Exported Function=], | ||
| 1. Let |funcaddr| be the value of |v|'s \[[FunctionAddress]] internal slot. | ||
| 1. Return [=ref.func=] |funcaddr|. | ||
| 1. Let |casted| be [=ref_cast=](|heaptype|, |funcaddr|). |
There was a problem hiding this comment.
Casts do not change the value, so if the cast succeeds, it would simply return the original funcaddr. But the cast may also fail, so the result type of this operator would have to include an error case.
Hence it is probably more natural for the embedding API to provide a predicate like
ref_typecheck(addr, heaptype) : bool
instead. (Perhaps we even want to separate getting the ref's own type from a subtype function on two types.)
There was a problem hiding this comment.
Thanks for catching that, I wonder if it would make sense to call the operation ref_test to mirror the ref.test instruction that is similar in functionality.
I could also see the route with using subtype making sense too, it could also help make some of the conditions more precise in the spec text (where it says "if type is of form X and is a subtype of Y").
There was a problem hiding this comment.
The latest commit uses ref_test for now (not tied to this naming, could use something else or go with a different way of checking) and should check the failure case.
document/js-api/index.bs
Outdated
| 1. Return [=ref.null=] |heaptype|. | ||
| 1. Otherwise, | ||
| 1. Throw a {{TypeError}}. | ||
| 1. If |heaptype| is [=i31=], |
There was a problem hiding this comment.
This condition seems wrong: i31's need to be generated for type any or eq as well, and the conversion should be the same (and accept the same values) regardless of the type. Likewise, you'll need to allow arbitrary JS values for any, not just ints or GC objects.
I think all non-null values ought to be converted uniformly by first performing internalize and then a type check. The internalize function will then have to do the job to convert numbers to i31 if they are within range, and treat them as host values otherwise. The same conversion has to happen when the internalize instruction is invoked in Wasm. Furthermore, externalize has to be the inverse.
This indeed implies that the JS spec needs to amend the core spec by customising the meaning of internalize/externalize. The core spec cannot really define it, because it doesn't know what host values are. We'll need some hook in the core spec for this host-specific semantics.
There was a problem hiding this comment.
I think all non-null values ought to be converted uniformly by first performing internalize and then a type check.
I've changed it to use to use this approach now: 2d39b22#diff-73846d0b7899a94fe050d3c0ba447da607bebedf80f8b5ed31cb4d164756f048R1170
This indeed implies that the JS spec needs to amend the core spec by customising the meaning of internalize/externalize.
Also added a new section in the spec that details the requirements on externalize & internalize: 2d39b22#diff-73846d0b7899a94fe050d3c0ba447da607bebedf80f8b5ed31cb4d164756f048R1348
document/js-api/index.bs
Outdated
| 1. If |w| is of the form [=ref.array=] |arrayaddr|, return the result of creating [=a new GC Exported Object=] from |arrayaddr|. | ||
| 1. If |w| is of the form [=ref.struct=] |structaddr|, return the result of creating [=a new GC Exported Object=] from |structaddr|. | ||
| 1. If |w| is of the form [=ref.i31=] |i31addr|, | ||
| 1. Let |i32| be [=i31_get=](|i31addr|). | ||
| 1. Return [=the Number value=] for |i32|. |
There was a problem hiding this comment.
I believe that externalize needs to be invoke here somehow. In particular, that will also handle values that are not Wasm GC values, such as internalised JS values. The core spec will have to introduce the notion of host object for those.
There was a problem hiding this comment.
In the latest commit, I made it call extern.externalize on all of the internal references: 2d39b22#diff-73846d0b7899a94fe050d3c0ba447da607bebedf80f8b5ed31cb4d164756f048R1118
Also added a ref.host as a placeholder for whatever the core spec will use for this. It's created by internalize and just stores an external address so externalize can retrieve the original value.
* Renamed "GC Exported Object" to "Exported GC Object"
* Made [[DefineOwnProperty]], [[PreventExtensions]], and
[[SetPrototypeOf]] return false instead of throwing.
(this will cause operations to throw after they check
for a false return value)
* Creating an Exported GC Object now puts it in a dedicated
exported GC object cache instead of using the extern cache
(this only worked when externalize was called on the stored
address, but now we store the internal address instead)
* extern_internalize and extern_externalize have their behavior
specified in this spec.
* Use the conversion functions above more in the JS spec.
* i31refs are now converted in the conversion functions, so it
does not matter what type is used at the boundary (i31 or any)
* Added ref.host value type (still to be defined in the Wasm spec)
that is used for any value that the JS API doesn't explicitly
convert into an internal reference (i.e., non-i31s and non-GC objects).
* Consolidate null cases in ToWebAssemblyValue
* Added [[ObjectKind]] slot in Exported GC Object. This is just
used so the spec knows whether to make a ref.array or ref.struct
from the object address.
Adds a test for the draft JS API spec from this PR: WebAssembly#352 This primarily tests the internal object methods described in the spec. It doesn't yet test the ToWebAssemblyValue / ToJSValue conversions. Also syncs a portion of the module builder code with V8 to allow constructing a subset of GC-using modules.
| <div algorithm> | ||
| The <dfn>\[[GetPrototypeOf]] internal method of an Exported GC Object</dfn> <var ignore>O</var> takes no arguments and returns null. It performs the following steps when called: | ||
|
|
||
| 1. Return null. |
There was a problem hiding this comment.
I updated [[GetPrototypeOf]] to return null to match the updated google doc, also see this comment: #355 (comment)
|
@rossberg Thanks for the review on this earlier, is it ok to merge this already? We discussed this in the GC subgroup meeting today and nobody mentioned any blockers to merging (and we can adjust to match the core spec later, as you said). |
|
Sure, I believe I already approved the PR. |
Ok thanks! I don't think I have merge access so if you or someone else with merge access (@tlively maybe?) could do it, that'd be great. |
https://bugs.webkit.org/show_bug.cgi?id=246769 Reviewed by Yusuke Suzuki. Implements the Wasm GC draft JS API described in: https://docs.google.com/document/d/17hCQXOyeSgogpJ0I0wir4LRmdvu4l7Oca6e1NkbVN8M/ WebAssembly/gc#352 This specifies that GC structs and arrays are opaque objects with internal methods (on WebAssemblyGCObjectBase) that either lead to throwing exceptions or returning some kind of null result. This patch doesn't yet implement the ToWebAssemblyValue changes needed to roundtrip these values back to Wasm or to access globals and tables with these values from JS. The patch also eliminates some of the restrictions on passing GC values that were added before the JS API was specified. * JSTests/wasm/gc/arrays.js: * JSTests/wasm/gc/js-api.js: Added. (runWasmGCObjectTests): (testStruct): * JSTests/wasm/gc/structs.js: * Source/JavaScriptCore/wasm/WasmCallingConvention.h: (JSC::Wasm::WasmCallingConvention::callInformationFor const): * Source/JavaScriptCore/wasm/js/JSToWasm.cpp: (JSC::Wasm::marshallJSResult): * Source/JavaScriptCore/wasm/js/JSWebAssemblyArray.cpp: (JSC::JSWebAssemblyArray::finishCreation): * Source/JavaScriptCore/wasm/js/WasmToJS.cpp: (JSC::Wasm::wasmToJS): * Source/JavaScriptCore/wasm/js/WebAssemblyGCObjectBase.cpp: (JSC::WebAssemblyGCObjectBase::getOwnPropertySlot): (JSC::WebAssemblyGCObjectBase::getOwnPropertySlotByIndex): (JSC::WebAssemblyGCObjectBase::put): (JSC::WebAssemblyGCObjectBase::putByIndex): (JSC::WebAssemblyGCObjectBase::deleteProperty): (JSC::WebAssemblyGCObjectBase::deletePropertyByIndex): (JSC::WebAssemblyGCObjectBase::getOwnPropertyNames): (JSC::WebAssemblyGCObjectBase::defineOwnProperty): (JSC::WebAssemblyGCObjectBase::getPrototype): (JSC::WebAssemblyGCObjectBase::setPrototype): (JSC::WebAssemblyGCObjectBase::isExtensible): (JSC::WebAssemblyGCObjectBase::preventExtensions): * Source/JavaScriptCore/wasm/js/WebAssemblyGCObjectBase.h: Canonical link: https://commits.webkit.org/261544@main
Adds a test for the draft JS API spec from this PR: WebAssembly#352 This primarily tests the internal object methods described in the spec. It doesn't yet test the ToWebAssemblyValue / ToJSValue conversions. Also syncs a portion of the module builder code with V8 to allow constructing a subset of GC-using modules.
Adds a test for the draft JS API spec from this PR: WebAssembly#352 This primarily tests the internal object methods described in the spec. It doesn't yet test the ToWebAssemblyValue / ToJSValue conversions. Also syncs a portion of the module builder code with V8 to allow constructing a subset of GC-using modules.
Adds a test for the draft JS API spec from this PR: WebAssembly#352 This primarily tests the internal object methods described in the spec. It doesn't yet test the ToWebAssemblyValue / ToJSValue conversions. Also syncs a portion of the module builder code with V8 to allow constructing a subset of GC-using modules.
This PR adds formal JS API spec text based on the "no-frills" proposal in #279 and https://docs.google.com/document/d/17hCQXOyeSgogpJ0I0wir4LRmdvu4l7Oca6e1NkbVN8M/.
I've also opened an issue to discuss certain aspects of this PR: #353