-
Notifications
You must be signed in to change notification settings - Fork 694
Move segment and start operations to the end of instantiation in JS.md #780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,11 +199,10 @@ function, global, memory and table imports): | |
throw a [`TypeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-typeerror). | ||
* Otherwise, append `v.[[Table]]` to `imports`. | ||
|
||
Let `instance` be the result of evaluating | ||
[`Eval.init`](https://github.com/WebAssembly/spec/blob/master/ml-proto/spec/eval.ml#L319) | ||
with arguments `module` and `imports`. | ||
Note: this synchronously executes the [`start`](Modules.md#module-start-function) | ||
function, if present. | ||
Let `instance` be the result of creating a new | ||
[`Eval.instance`](https://github.com/WebAssembly/spec/blob/master/ml-proto/spec/eval.ml#L15) | ||
given `module` and `imports`. Note: this step does not mutate imported memories | ||
or tables. | ||
|
||
Let `exports` be a list of (string, JS value) pairs that is mapped from | ||
`module.exports` as follows (assuming the ML spec | ||
|
@@ -215,11 +214,12 @@ has been modified so that each export simply has a `name`, `type` and `index`: | |
* If the `type` is function, then export an | ||
[Exported Function Exotic Object](#exported-function-exotic-objects), | ||
reusing an existing object if one exists for the given function definition, | ||
otherwise creating a new object. | ||
otherwise creating a new object given `instance` and `index`. | ||
* If the `type` is global: | ||
* If the global is not immutable, then throw a [`TypeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-typeerror). | ||
* [Assert](https://tc39.github.io/ecma262/#assert): the global is immutable | ||
by MVP validation constraint. | ||
* Let `v` be the global variable's initialized value. | ||
* Otherwise, export [`ToJSValue`](#tojsvalue)`(v)`. | ||
* Export [`ToJSValue`](#tojsvalue)`(v)`. | ||
* If the `type` is memory, then export a `WebAssembly.Memory` object, reusing | ||
an existing object if one exists for the given memory definition, otherwise | ||
creating a new object via [`CreateMemoryObject`](#creatememoryobject). | ||
|
@@ -243,8 +243,30 @@ of `exports`. Let `moduleNamespace` be the result of calling | |
[`ModuleNamespaceCreate(moduleRecord, exportStrings)`](http://tc39.github.io/ecma262/#sec-modulenamespacecreate). | ||
Set `moduleRecord.[[Namespace]]` to `moduleNamespace`. | ||
|
||
Return a new `WebAssembly.Instance` object setting `[[Instance]]` to `instance` | ||
and `exports` to `moduleNamespace`. | ||
Let `instanceObject` be a new `WebAssembly.Instance` object setting | ||
`[[Instance]]` to `instance` and `exports` to `moduleNamespace`. | ||
|
||
If any of the elements of an [Elements section](Modules.md#elements-section) | ||
refers to an imported function which is not an | ||
[Exported Function Exotic Object](#exported-function-exotic-objects), throw a | ||
[`TypeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-typeerror). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this because they don't have a type that call_indirect can check? Why can't it use the respective import signature? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just mirroring the MVP constraints added in #775 that tables only contain WebAssembly functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (And so if we relaxed that restriction it'd be trivial to relax here; it's the same underlying implementation challenge.) |
||
|
||
If, after evaluating the `offset` [initializer expression](Modules.md#initializer-expression) | ||
of every [Data](Modules.md#data-section) and [Element](Modules.md#elements-section) | ||
Segment, any of the segments do not fit in their respective Memory or Table, throw a | ||
[`RangeError`](https://tc39.github.io/ecma262/#sec-native-error-types-used-in-this-standard-rangeerror). | ||
|
||
Apply all Data and Element segments to their respective Memory or Table in the | ||
order in which they appear in the module. Segments may overlap and, if they do, | ||
the final value is the last value written in order. Note: there should be no | ||
errors possible that would cause this operation to fail partway through. After | ||
this operation completes, elements of `instance` are visible and callable | ||
through [imported Tables](Modules.md#imports), even if `start` fails. | ||
|
||
If a [`start`](Modules.md#module-start-function) is present, it is evaluated | ||
given `instance`. Any errors thrown by `start` are propagated to the caller. | ||
|
||
Return `instanceObject`. | ||
|
||
### WebAssembly Module Record | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if a link to (https://github.com/WebAssembly/design/blob/master/JS.md#webassemblyinstance-objects) would be more helpful here than a link to the spec. In (https://github.com/WebAssembly/design/blob/master/JS.md#webassemblyinstance-objects) the link to the spec is helpful because there is some text which tells you what to expect from that link. Here, however, I did not know what this link actually means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is a bit subtle, but there are two things here: the abstract wasm-spec-defined
Eval.instance
(which probably we'll rename and relink once we have a prose spec in the spec repo) and the JSWebAssembly.Instance
object we're constructing which "wraps" it. Here we're making the former; only at the end do we construct the latter. We create the former here because it is needed to create the exported functions below. Does that clear it up for you any?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do understand that Eval.instance != WebAssembly.Instance. The reason why I mentioned (https://github.com/WebAssembly/design/blob/master/JS.md#webassemblyinstance-objects) is because it is the section which contains the definition of Eval.instance in JS.md. Is there a way to create a link to the definition of Eval.instance directly?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, that's what I was attempting to do with https://github.com/WebAssembly/spec/blob/master/ml-proto/spec/eval.ml#L15. That's a stand-in for what will be the prose-spec's abstract definition of a wasm instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you have something different in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are right that the prose spec will eventually fix this problem anyways.