-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 emscripten support #4443
Draft
walkingeyerobot
wants to merge
13
commits into
rustwasm:main
Choose a base branch
from
walkingeyerobot:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
add emscripten support #4443
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ely that are consumed by Emscripten.
library_bindgen.js.
Merge the two generated .js into one library_bindgen.js.
Notably, to work with emscripten, the emitted js has explicit dependencies listed for the imports.
Expand emscripten support to supporting JS imported closures.
Because the generated .js file when targeting emscripten in wasm-bindgen is meant to be consumed by emscripten rather than a standalone executable, we need some custom testing logic for emscripten.
Add the necessary emscripten test mode.
e02edbe
to
79c884a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds
--target emscripten
option, which will output JS that Emscripten can consume and integrate into its own JS.This is still a work-in-progress, but it's getting kind of beefy, so I thought it would be best to start review on this soon. There's still feature work to be done (i.e. I don't think futures work). My preference would be to submit this with only the current set of supporting features, mark
--target emscripten
as experimental, and then send smaller PRs to add more support.Also adds a basic test. This test builds some rust code and links it with Emscripten, but the test runner invokes wasm-bindgen. It then tests to make sure the output looks reasonable. Normally, the wasm-bindgen cli tool is invoked by Emscripten at link time. Because cargo does not support binary dependencies, and because wasm-bindgen is effectively being used as a dependency of Emscripten, it's very difficult to do end-to-end testing in the wasm-bindgen repository. I'd like to add more comprehensive testing, but I think it's more appropriate to either Emscripten or to a new repository separate from wasm-bindgen and Emscripten that can depend on both of those repositories at HEAD.
It turns out wasm-bindgen also does not support Emscripten exceptions or PIC, so we've disabled both of those compiler options. This is mostly due to lack of features in the interpreter. Exception support is something I'd like to eventually add. PIC appears to be off by default for
wasm32-unknown-unknown
and on by default forwasm32-unknown-emscripten
. PIC can be supported through changes to wasm-bindgen's interpreter, but that seemed a bit out of scope for this PR.I've also made a corresponding PR to Emscripten: emscripten-core/emscripten#23493
I've also made a PR to wasm-pack: rustwasm/wasm-pack#1469. However, there has been no activity in wasm-pack by maintainers for ~4 months, and I suspect it's unlikely to get attention in the near future. The purpose of the wasm-pack PR is to provide a more straightforward workflow for Rust users already familiar with targeting Wasm.