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

compile-time internal error on passing &Function to js #1373

Closed
limira opened this issue Mar 21, 2019 · 1 comment · Fixed by #1377
Closed

compile-time internal error on passing &Function to js #1373

limira opened this issue Mar 21, 2019 · 1 comment · Fixed by #1377
Labels

Comments

@limira
Copy link
Contributor

limira commented Mar 21, 2019

I encountered internal error: entered unreachable code with wasm-bindgen when I tried the git master version at merge of #1353 (and still there at the merge of #1372, not sure how far ago when the bug appear. But the bug is not in 0.2.39 release)

You can see the minimal code that produce the error here. In words, I have a library wb-lib that wraps around Foo in foo.js. Foo is a JS class that need a callback that the Rust code will pass a &js_sys::Function to it. An app wb-app depends on wb-lib, it initialize a Foo object.

Compile the provided repo (using a matched version of wasm-bindgen-cli) will cause the compile time error as shown in the backtrace file (also in repo). Note that I run wasm-bindgen --web.

No error if arguments not include a js_sys::Function. No error with v0.2.39.

@limira limira added the bug label Mar 21, 2019
alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Mar 21, 2019
Ended up helping diagnose the problem in the end!
alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Mar 21, 2019
This commit moves our `links` annotation in the `wasm-bindgen` crate to
the `wasm-bindgen-shared` crate. The `links` annotation is used to
ensure that there's only one version of `wasm-bindgen` in a crate graph
because if there are multiple versions then a CLI surely cannot actually
process the wasm binary (as the multiple versions likely have different
formats in their custom sections).

Discovered in rustwasm#1373 it looks like the usage in `wasm-bindgen` isn't
quite sufficient to cause this deduplication. It turns out that
`wasm-bindgen-shared`, a very core dependency, is actually the most
critical to be deduplicated since its the one that defines the format of
the custom section. In rustwasm#1373 a case came up where `wasm-bindgen` was
deduplciated but there were two versions of `wasm-bindgen-shared` in the
crate graph, meaning that a `[patch]` for only `wasm-bindgen` wasn't
sufficient, but rather `web-sys` and/or `js-sys` also needed a `[patch]`
annotation to ensure everyone used the right dependencies.

This commit won't actually fix rustwasm#1373 to the point where it "just works",
but what it does do is present a better error message than an internal
panic of `wasm-bindgen`. The hope is that by moving the `links`
annotation we can catch more errors of this crate graph duplication,
leading to more `[patch]` annotations locally.

Closes rustwasm#1373
@alexcrichton
Copy link
Contributor

Thanks for the report and for the reproduction! I think this should be "fixed" in #1377 where "fixed" here means "produces a better error message". The real fix though for this is to add [patch] annotations for web-sys in addition to wasm-bindgen because the problem was that too many crates showed up in the crate graph, confusing the CLI!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants