-
Notifications
You must be signed in to change notification settings - Fork 695
Consolidate explanation of modules into a new Modules.md and improve explanation #270
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
Conversation
* [Polyfill to JavaScript](Polyfill.md); | ||
* [AST semantics](AstSemantics.md); | ||
* [Binary encoding](BinaryEncoding.md); | ||
* Implementation [in the browser](Web.md) and [outside the browser](NonWeb.md). | ||
|
||
**Note**: This content is still in flux and open for discussion. |
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 think we should keep this note for now.
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'm happy to, I was just thinking that this was originally added when V1.md had a bunch of text and so now it seems a bit ad hoc and asymmetric since most files don't have it. The important thing is the Readme.md has it bold. Still want to keep?
@jfbastien Which line are you referring to? |
Hmm, it looks like commenting on an incremental diff doesn't link well :-s |
Updated to address feedback. |
I thought that I had; are there any outstanding issues not addressed in the current version? |
d65a477
to
3c5f390
Compare
@ncbray Rebased to trunk |
This is the only way I found to view a complete diff: In general I think the consolidation, editing, and clarification is good. The new ideas should probably be discussed with a wider audience? At first glance, WASM importing WASM being type checked is potentially contentious/complicated. It is a weird corner case, normally imports are “whatever the embedder wants” and this burns a hole that seems tricky to define. There are also crazier things I’ve been thinking about, such as allowing a signed/unsigned distinction in imports so values could be fixed up before passing them to JS. How does that interact? It does seem reasonable that the implementation could type check imports to some degree (WASM or otherwise) but ironing out the details... hmmm. Prototypes would be nice. If checking imports is complicated we could always drop it on the floor. I have gotten the impression that content sniffing is considered harmful by a lot of web folks. (Implementation complexity and security issues arising from content confusion.) Polymorphic behavior based on URL content (JS or WASM) may be problematic unless we can require correct MIME types from the server 100% of the time. I’ll talk to the web folks on this side of the fence who have voiced concerns before. Content sniffing may also interact strangely with the “format layering” idea we’re kicking around for file format. It will always possible to wrap a WASM instance in a ES6 module (manually or autogen) to prototype all of this without needed to change the loader. Does ES6 module integration need to be part of MVP? (I don't have strong feelings, it just noticed it seems separable.) Nits / related comments:
|
I've been naively assuming that this is what "Files Changed" is showing. Anyhow, for reasons given in #282, out-of-line comments are preferable for non-nit comments.
Thus far, people have been relieved to hear this proposed (e.g., @flagxor, iirc) and while it may seem asymmetric at first glance, if you consider that an import is probing the host environment not just for a name, but for a (name, signature) pair (and faulting when not found), then all this amounts to is that, for a JS import, the signature is ignored in the probe (b/c there is no concept of signature in JS) while for a wasm import, there is a signature and so a "type mismatch" is really just a failed lookup. I would expect builtin imports to work the same way (less special cases to implement), so that actually makes JS the odd one out. Do you think it'd be more clear to describe this design this way?
Great idea! I think it could be done independent of JS by saying that the set of types available to import/export signature declarations additionally include
I have too, but I think a lot of this is from situations that are 100x harrier (e.g., character encoding, quirks mode and images) because the sniffing only came about after the fact. However, in our case, we're starting with a pretty strict input requirement (invalid characters trigger hard syntax errors in JS) and a pretty clear a priori break which we can test for web compat extensively before deploying. So while I'm definitely interested to hear specific concerns, I'm optimistic that our case is easier than those before.
One way or another, we have to define how a wasm module gets loaded and how it looks to JS. If we don't integrate with ES6 modules, we'll have to design something separate which I think will have worse ergonomics and lead to duplication (spec and impl) and accidental incongruencies. If ES6 modules were imposing significant costs on the wasm design, then we'd have to weigh the pros/cons, but so far, and as I've tried to demonstrate in this PR, I'm not seeing any warts imposed on wasm due to ES6 modules atm.
By representing references symbolically (via an index into a module-local table of functions/globals/imports) instead of with hard-coded addresses, I'm not sure what extra information is needed to do relocations when dynamic linking. Do you have anything in mind?
Agreed; definitely looking forward to the next steps of giving a more formal/executable definition to all of this. For now, though, I think it's valuable to have this in place to address a lot of the immediate confusion. |
@ncbray What about generalizing that idea of signed/unsigned distinction to a general type, like ... "pointer to tuple (int32, sint)" or any little piece of metadata? Wasm doesn't necessarily have to define a metadata format, and could just offer a place to put it, with optional binary matching on import - e.g. if the export says
Agreed! In PRs, I would strongly advocate giving a "possible specific design" instead of leaving it up to the imagination. |
Alright, have lgtm from @jfbastien and I don't see any specific outstanding objections to merging. Happy to iterate further in future PRs. |
Consolidate explanation of modules into a new Modules.md and improve explanation
This PR is an attempt to consolidate the discussion of modules and their integration with ES6 modules into one place and also provide a better explanation.
Independently, there's been some agreement to work on a new "level 0" version of the loader spec with the goal of being "just enough to be able to ship ES6 modules and WebAssembly modules". When there is a URL to point to, I'll update the TODO link in Modules.md.
Especially interested for feedback from @ajklein.
(On a side note, with this PR, MVP.md is now pretty empty and ad hoc, it'd be good to tidy it up. I might do that if noone beats me to it.)