-
Notifications
You must be signed in to change notification settings - Fork 257
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
Add multi-memory example #35
Conversation
@bsmth Do you have reviewer rights here? |
I do, yes. I can take a look - adding myself as a reviewer |
Thanks @bsmth ! |
FYI, would be good if this could be merged somewhat soon. The release is this week. |
Thanks, Hamish. To get this running I did: # Get wabt tools (https://github.com/WebAssembly/wabt)
brew install wabt
wat2wasm --enable-multi-memory multi-memory.wat -o multi-memory.wasm Should we capture that somewhere? Can be on the content side |
Back over to you, @hamishwillee - do you want to check the comments I've left? I think we can merge afterwards 🚢 |
Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
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.
LGTM pending the comment about overrunning the memory, the code comment looks like a good way to flag what's happening
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
@bsmth Thanks for merging (and happy with your changes)
I am not sure. The "right" think to do would be to allow support for building WASM in live examples. However that's a bit of a step because you'd need to be able to dynamically generate wasm from wat. It is actually not possible for this example because the tools that are used by interactive examples don't support the multimemory (which is what we'd used). I don't think we'd do it in content. The best compromise way to do it would be to add "Running/Setup" into this project's README. Thoughts? |
Thanks, Hamish! I wasn't sure if we covered how to do this in a generic way in mdn/content docs, but I see we have this: https://developer.mozilla.org/en-US/docs/WebAssembly/Text_format_to_Wasm#converting_the_text_.wat_into_a_binary_.wasm_file which is exactly what I was looking for, so problem solved. We could link to that from the README, I think that's a good help. On a related note, I see there's some work ongoing for wat support: |
Thanks @bsmth - that's cool, particularly the bob update. Though it is hard to tell from https://crates.io/crates/wasm-pack or the bob updates whether the features will "all" be supported (or even which ones are). Fixed up readme here #36 |
Good question, Florian / Vadim would have better context on it, it looks promising so far! |
@bsmth As discussed in #35 (comment) This adds links to WABT, the docs on MDN, and to the table for supported features. --------- Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
This adds an example of how to use multiple memories in Web Assembly.
Still working on docs
This is part of work for mdn/content#32777