Closed
Bug 1333490
Opened 7 years ago
Closed 7 years ago
wasm: pass a few more shared tests
Categories
(Core :: JavaScript Engine: JIT, defect, P1)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(2 files, 2 obsolete files)
1.97 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
2.04 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Comment 1•7 years ago
|
||
Derailing this bug to include more fixes to the shared test suite.
Assignee | ||
Comment 2•7 years ago
|
||
Defines toStringTag for wasm prototypes.
Attachment #8838669 -
Flags: review?(luke)
Assignee | ||
Comment 3•7 years ago
|
||
Instantiate.length should be 1, not 2: https://github.com/bnjbvr/wasm-spec/pull/1#discussion_r97571381
Attachment #8838670 -
Flags: review?(luke)
Assignee | ||
Comment 4•7 years ago
|
||
Filed https://github.com/WebAssembly/spec/pull/426 for upstream fix.
Attachment #8838671 -
Flags: review?(luke)
Comment 5•7 years ago
|
||
Comment on attachment 8838669 [details] [diff] [review] _1-tostringtag.patch Review of attachment 8838669 [details] [diff] [review]: ----------------------------------------------------------------- Great to see this fixed; one request: ::: js/src/vm/CommonPropertyNames.h @@ +378,5 @@ > macro(wasmcall, wasmcall, "wasmcall") \ > + macro(WebAssemblyModule, WebAssemblyModule, "WebAssembly.Module") \ > + macro(WebAssemblyInstance, WebAssemblyInstance, "WebAssembly.Instance") \ > + macro(WebAssemblyMemory, WebAssemblyMemory, "WebAssembly.Memory") \ > + macro(WebAssemblyTable, WebAssemblyTable, "WebAssembly.Table") \ Common property names are useful for hot cases where it's useful to always have an atom available. These 4 are only used once, during lazy wasm initialization, so I'd rather leave them out of this list. ::: js/src/wasm/WasmJS.cpp @@ +2113,5 @@ > > RootedObject moduleProto(cx), instanceProto(cx), memoryProto(cx), tableProto(cx); > if (!InitConstructor<WasmModuleObject>(cx, wasm, "Module", &moduleProto)) > return nullptr; > + if (!DefineToStringTag(cx, moduleProto, cx->names().WebAssemblyModule)) Could you push this into InitConstructor? It already does some atomization on `name, so you could programmatically build `WebAssembly.${name}`.
Updated•7 years ago
|
Attachment #8838670 -
Flags: review?(luke) → review+
Comment 6•7 years ago
|
||
Comment on attachment 8838671 [details] [diff] [review] _3.typerror.patch Review of attachment 8838671 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8838671 -
Flags: review?(luke) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8838671 [details] [diff] [review] _3.typerror.patch For what it's worth, I've discarded this patch by rearranging my local patch queue for bug 1332691. All these patches will land together.
Attachment #8838671 -
Attachment is obsolete: true
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8838669 -
Attachment is obsolete: true
Attachment #8838669 -
Flags: review?(luke)
Attachment #8840509 -
Flags: review?(luke)
Comment 9•7 years ago
|
||
Comment on attachment 8840509 [details] [diff] [review] tostringtag.patch Review of attachment 8840509 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: js/src/wasm/WasmJS.cpp @@ +2071,5 @@ > return false; > > + UniqueChars tagStr(JS_smprintf("WebAssembly.%s", name)); > + if (!tagStr) > + return false; return ReportOutOfMemory(cx)
Attachment #8840509 -
Flags: review?(luke) → review+
Comment 10•7 years ago
|
||
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/71cc6bef1788 Define toStringTag for wasm prototypes; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/339878f975cc Change WebAssembly.instantiate.length to 1; r=luke
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/71cc6bef1788 https://hg.mozilla.org/mozilla-central/rev/339878f975cc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•