Bug 165121

Summary: WebAssembly JS API: export a module namespace object instead of a module environment
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: JF Bastien <jfbastien>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jfbastien, keith_miller, mark.lam, msaboff, saam, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 164757    
Bug Blocks: 161709, 169951    
Attachments:
Description Flags
Patch
none
Patch saam: review+

JF Bastien
Reported 2016-11-28 16:03:14 PST
JSWebAssemblyInstance::finishCreation should do this: putDirect(vm, Identifier::fromString(&vm, "exports"), JSValue(m_moduleNamespaceObject.get()), None); Not its constructor. These tests should pass in test_basic_api.js: assert.isUndef(instance.exports.__proto__); assert.eq(Reflect.isExtensible(instance.exports), false); assert.eq(Symbol.iterator in instance.exports, true); assert.eq(Symbol.toStringTag in instance.exports, true); WebAssembly doesn't have circular linking and fancy things yet, so it's slightly simpler than ES6 modules. I'm sure this isn't super hard to fix, but the current code does what we need to get off the ground so I'd rather figure it out later.
Attachments
Patch (16.45 KB, patch)
2016-11-30 02:15 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (17.76 KB, patch)
2016-11-30 02:18 PST, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2016-11-30 01:49:21 PST
I'll look into it.
Yusuke Suzuki
Comment 2 2016-11-30 02:15:28 PST
Created attachment 295708 [details] Patch
Yusuke Suzuki
Comment 3 2016-11-30 02:18:56 PST
Created attachment 295709 [details] Patch
Yusuke Suzuki
Comment 4 2016-11-30 02:21:13 PST
Comment on attachment 295709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295709&action=review > Source/JavaScriptCore/wasm/WasmFormat.cpp:37 > +#endif // COMPILER(GCC) && ASSERT_DISABLED This is B3 way for the following switch's warning.
JF Bastien
Comment 5 2016-11-30 10:11:38 PST
Comment on attachment 295709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295709&action=review Awesome, this was way simpler than I thought it was! :-) Looks good. >> Source/JavaScriptCore/wasm/WasmFormat.cpp:37 >> +#endif // COMPILER(GCC) && ASSERT_DISABLED > > This is B3 way for the following switch's warning. I'm auto-generating that code here: https://bugs.webkit.org/show_bug.cgi?id=164724
Yusuke Suzuki
Comment 6 2016-11-30 19:09:17 PST
Comment on attachment 295709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295709&action=review >>> Source/JavaScriptCore/wasm/WasmFormat.cpp:37 >>> +#endif // COMPILER(GCC) && ASSERT_DISABLED >> >> This is B3 way for the following switch's warning. > > I'm auto-generating that code here: https://bugs.webkit.org/show_bug.cgi?id=164724 Nice. Once the above patch is landed, we can simply drop this!
Saam Barati
Comment 7 2016-11-30 19:31:50 PST
Comment on attachment 295709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295709&action=review r=me > Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:36 > +#include "LLIntThunks.h" Was this file needed?
Yusuke Suzuki
Comment 8 2016-11-30 19:52:18 PST
Comment on attachment 295709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295709&action=review Thanks! >> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:36 >> +#include "LLIntThunks.h" > > Was this file needed? It is necessary for `vmEntryToWasm`.
Yusuke Suzuki
Comment 9 2016-11-30 19:54:48 PST
Committed r209171: <http://trac.webkit.org/changeset/209171>
Note You need to log in before you can comment on or make changes to this bug.