Supporting the type method on WebAssembly.Global, WebAssembly.Memory and WebAssembly.Table in the wasm JS-API would allow us to inspect the type of those objects as well as create new objects with the same type by passing the type return by the new type method to the constructor. This is outlined in https://github.com/WebAssembly/js-types/blob/master/proposals/js-types/Overview.md#extensions-to-api-functions
Created attachment 421514 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment on attachment 421514 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421514&action=review Commented. > Source/JavaScriptCore/wasm/js/JSWebAssemblyGlobal.cpp:115 > + default: > + RELEASE_ASSERT_NOT_REACHED(); > + } We need to handle all types that are supported in WebAssembly.Global (e.g. funcref). > Source/JavaScriptCore/wasm/js/WebAssemblyGlobalPrototype.cpp:90 > + return JSValue::encode(global->type(globalObject)); Use RELEASE_AND_RETURN(throwScope, JSValue::encode(global->type(globalObject))); > Source/JavaScriptCore/wasm/js/WebAssemblyMemoryPrototype.cpp:112 > + JSObject* memoryType = memory->type(globalObject); > + RETURN_IF_EXCEPTION(throwScope, { }); > + > + RELEASE_AND_RETURN(throwScope, JSValue::encode(memoryType)); Do, RELEASE_AND_RETURN(throwScope, JSValue::encode(memory->type(globalObject))); > Source/JavaScriptCore/wasm/js/WebAssemblyTablePrototype.cpp:178 > + JSObject* tableType = table->type(globalObject); > + return JSValue::encode(tableType); Do RELEASE_AND_RETURN(throwScope, JSValue::encode(table->type(globalObject)));
<rdar://problem/75030783>
Created attachment 422927 [details] Patch
Created attachment 423326 [details] Patch
Created attachment 423340 [details] Patch
This is ready for review. I saw that some windows tests are failing but taking a look at the crashed test I don't think it's related to my patch.
(In reply to jtallon from comment #8) > This is ready for review. I saw that some windows tests are failing but > taking a look at the crashed test I don't think it's related to my patch. Could you set r? for the patch requesting the review?
Comment on attachment 423340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423340&action=review > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:139 > + JSObject* result = constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); We are setting `inlineCapacity` to 2 here, but there are cases where we will potentially have 3 properties. This will make such object allocate a butterfly when we have the third property, increasing the amount of memory we use for this object, and slowing down the access of such property (because it will be stored inside the butterfly). Since `maximum` property is conditional, I think we could move the creation of `result` after we know if it will be included or not. > Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:133 > + JSObject* result = constructEmptyObject(globalObject, globalObject->objectPrototype(), 3); Ditto here. But in this case we are setting 3 while we don't know if `maximum` is going to be present. In such situation, we are allocating some space that might be useless.
Created attachment 423589 [details] Patch
Comment on attachment 423589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423589&action=review > Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:143 > + auto maximum = m_table->maximum(); I have a legit question. Is there any reason why `JSWebAssemblyMemory::maximum` can throw, but `JSWebAssemblyTable::maximum` doesn't?
Comment on attachment 423589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423589&action=review > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:140 > + RETURN_IF_EXCEPTION(throwScope, { }); Yes, this will not throw an error. Let's remove it. > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:143 > + RETURN_IF_EXCEPTION(throwScope, { }); Ditto.
Created attachment 424329 [details] Patch
Created attachment 424533 [details] Patch
Created attachment 424625 [details] Patch
Comment on attachment 424625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424625&action=review r=me with comments > Source/JavaScriptCore/wasm/js/JSWebAssemblyGlobal.cpp:96 > + result->putDirect(vm, Identifier::fromString(vm, "mutable"), jsBoolean(isMutable)); Use result->putDirect(vm, Identifier::fromString(vm, "mutable"), jsBoolean(m_global->mutability() == Wasm::GlobalInformation::Mutable)); > Source/JavaScriptCore/wasm/js/JSWebAssemblyGlobal.cpp:98 > + Wasm::Type value = m_global->type(); valueType would be better. > Source/JavaScriptCore/wasm/js/JSWebAssemblyGlobal.cpp:99 > + JSString* valueStr = nullptr; Use `valueString`. > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:154 > + Wasm::MemorySharingMode shared = m_memory->sharingMode(); > + if (shared == Wasm::MemorySharingMode::Default) > + result->putDirect(vm, Identifier::fromString(vm, "shared"), jsBoolean(false)); > + else > + result->putDirect(vm, Identifier::fromString(vm, "shared"), jsBoolean(true)); Make it, result->putDirect(vm, Identifier::fromString(vm, "shared"), jsBoolean(m_memory->sharingMode() == Wasm::MemorySharingMode::Shared)); > Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:134 > + JSString* elementStr = nullptr; Use `elementString` instead. We like non-abbreviated names. > Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:138 > + elementStr = jsNontrivialString(vm, "externref"); Let's make it switch to list things in a readable way. JSString* elementString = nullptr; switch (element) { case Wasm::TableElementType::Funcref: elementString = jsNontrivialString(vm, "funcref"); break; case Wasm::TableElementType::Externref: elementString = jsNontrivialString(vm, "externref"); break; default: RELEASE_ASSERT_NOT_REACHED(); break; }
Created attachment 424758 [details] Patch
Comment on attachment 424758 [details] Patch r=me, nice!
Committed r275438: <https://commits.webkit.org/r275438> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424758 [details].