Move segment and start operations to the end of instantiation in JS.md #780
Conversation
| If any of the elements of an [Elements section](Modules.md#elements-section) | ||
| refer 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.
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.
This is just mirroring the MVP constraints added in #775 that tables only contain WebAssembly functions.
There was a problem hiding this comment.
(And so if we relaxed that restriction it'd be trivial to relax here; it's the same underlying implementation challenge.)
|
Any other comments? @gahaas? |
| 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) |
There was a problem hiding this comment.
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.
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 JS WebAssembly.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.
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?
There was a problem hiding this comment.
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.
Did you have something different in mind?
There was a problem hiding this comment.
I guess you are right that the prose spec will eventually fix this problem anyways.
|
@gahaas / @rossberg-chromium Any further comments? |
|
LGTM |
1 similar comment
|
LGTM |
The way JS.md is currently written, failures can occur after the Elements and/or Data segments have been written to globally-visible (imported) tables/memories and/or after
startis called. This sort of partial initialization seems undesirable so this PR moves all the globally-visible operations to the very end so failure makes more sense and you can't end up with partially-written segments.