Skip to content
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

rewrite spec to be more consistent #15

Closed
wants to merge 1 commit into from

Conversation

sokra
Copy link

@sokra sokra commented Oct 15, 2018

This PR make as lot of changes to the spec to try to make it more consistent.

One bigger change is how function-imports are handled. This comes with the motivation that people often export arrow functions instead of function declarations. Before this change only function declarations were possible.

In some cases export type was renamed to import type, as the type of an export in a JS module is not known before evaluation. So decision is now only base on information statically available in WASM.

I also tried to make it more clear that exports are resolved while instantiation and the behavior for reexports is based on that. So reexporting wasm exports in a JS module is not a special case.

I also allowed wasm cycles, but I'm also ok with disallowing this behavior.

| | Error | Error | Error | Error | snapshot |

\*While WebAssembly only has the concept of globals, JS could export either a regular JS value or a `WebAssembly.Global`.
| import type | global | memory | table | function |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import type seems more appropriate here--the path is chosen based on how the WebAssembly module declares its imports. I really appreciate this fix.

@@ -88,7 +88,7 @@ Three things happen for each module during the Construction phase.

### Instantiation

During instantiation, exports and imports are wired up to each other. In JS, functions are initialized at this point. All other values are undefined until evaluation.
During instantiation, exports and imports are wired up to each other. In JS, function declarations are initialized at this point. All other values are undefined or in TDZ until evaluation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this clarification

@@ -97,18 +97,18 @@ For WebAssembly, instantiation will include initialization for all [external typ
- memories
- globals

Because WebAssembly does not currently have a type that can handle arbitrary JS values, JS value imports can not be handled with live bindings and would be snapshotted when they are imported. This means that in the short term, non-function imports coming from JS modules would be assigned a value of `undefined`, which will currently result in an error.
Because WebAssembly does not currently have a type that can handle arbitrary JS values, JS value imports can not be handled. This means that in the short term, non-function imports coming from JS modules will currently result in an error.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "live binding" part here is pretty crucial: If something starts out as a Number, we might be able cast that into an appropriate WebAssembly type on module instantiation for a const global; but if we want to implement live bindings for mutable globals, there's no clear time that the cast could run, and then the value would no longer be the right WebAssembly type.


It may be possible to provide live bindings for JS values if an `any` type is added to WebAssembly. At this point the restriction prohibiting `undefined` imports could also be lifted. However, we would still want to throw an error for non-function imports in this case.
It may be possible to provide live bindings for JS values if an `anyref` type is added to WebAssembly. At this point the restriction prohibiting non-function imports could also be lifted.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this change


Because of this, instantiation may need to be split into two separate sub-phases for WebAssembly. In this approach, the current instantiation algorithm would be run in the first sub-phase, ensuring that all JS functions are initialized. Then, in the second sub-phase, WebAssembly would be able to snapshot these function exports.
For non-function imports the `[[Module]]` of *ResolvedBinding Record* must be a WebAssembly mdoule, otherwise an Error is thrown. As imports need to be provided before instantiation, cycles are not allowed in this case.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Nit: In this high-level document, let's try to stay away from excessively technical-sounding language. We'll make a spec soon which hooks into all the technical details. And, s/mdoule/module/)

\*While WebAssembly only has the concept of globals, JS could export either a regular JS value or a `WebAssembly.Global`.
| import type | global | memory | table | function |
|-------------|--------|--------|-------|--------------|
| | Error | Error | Error | live binding |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering, why do you propose a live binding for functions? Is this about supporting cases like const fn = () => {}? I think this would be well-supported by #13, see these semantics. The snapshot is taken after that const declaration is evaluated!


The reason we want to throw an error in this case is two fold:

- **Reduce confusion for developers.** Since non-function imports work in JS->JS dependency chains, it is unintuitive that they would be undefined in JS->wasm dependency chains. Throwing an error makes clear what's happening.
- **Allow for future support.** It may be possible to support non-function imports to wasm modules if an `any` type is added. We close off this avenue if we allow users to import values that can be undefined, though, as code may start depending on that behavior.
- **Reduce confusion for developers.** WebAssembly snapshots imports while instantiation, but most exports from JS modules are still `undefined` or in TDZ at this time. Since non-function imports work in JS->JS dependency chains, it is unintuitive that they would be undefined in JS->wasm dependency chains. Throwing an error makes clear what's happening.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This snapshotting time is the proposal at master, and this paragraph ("shapshots imports while instantiation") is characteristic of it, but I like the PR at #13, which changes the time to be a bit later (during execution) and allows more cases to be well-handled.

| | Error | Error | Error | Error | snapshot |

\*While WebAssembly only has the concept of globals, JS could export either a regular JS value or a `WebAssembly.Global`.
| import type | global | memory | table | function |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import type seems more appropriate here--the path is chosen based on how the WebAssembly module declares its imports. I really appreciate this fix.

1. JS module is parsed
1. Instantiation
1. JS module is instantiation and bindings are created to exports.
1. wasm module is instantiated. A trampoline functions is created for function imports, which points to the exported binding. All non-function imports throw an error.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's consider these trampolines just when needed to handle the circular case, in a follow-on proposal, see #17

@@ -159,7 +160,7 @@ function getCount() {
export {getCount};
```

##### Value imports
##### Global imports
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, from the perspective of a JS programmer, "value" makes sense.

@littledan
Copy link
Collaborator

I think we'll go with the semantics checked in right now, rather than the semantics in this PR, due to the inability to handle importing Memory, Tables, and in the future, nominal types.

@littledan littledan closed this May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants