Fix a number of anyref related bugs in our passes
#1692
Merged
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
This functionality got lost in recent refactorings for WebIDL bindings unfortunately, so this commit touches things up to ensure that the anyref table initialization in anyref-mode is hooked up correctly, even when tests are enabled. This invovled moving injection of the start function to the webidl processing pass and ensuring its intrinsic is registered in the internal maps of wasm-bindgen.
With more than two anyref stack arguments we were accidentally storing the anyref values one higher in the stack than intended, so fix this off-by-one by switching up some addition logic.
This is currently required by our ABI for wasm-bindgen where `None` js values going out have an index of 0 and are intended to be `undefined`. This also refactors initialization a bit to be slightly more generic over the constants we already have defined in this module.
This `base` value is the raw value coming out of the first call to `table.grow`. Throwing in the addition of `JSIDX_RESERVED` just wasn't right!
fitzgen
approved these changes
Aug 1, 2019
| @@ -145,7 +145,7 @@ intrinsics! { | |||
| #[symbol = "__wbindgen_anyref_heap_live_count"] | |||
| #[signature = fn() -> I32] | |||
| AnyrefHeapLiveCount, | |||
| #[symbol = "__wbindgen_init_nyref_table"] | |||
|
So now that node has proper |
|
Soon we can I think! The support just (like 24 hours ago) landed on the master branch of nodejs, so I think we'll need to wait for the next release and then we should be able to download a recent verison of Node to test with. |
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.
This commit goes through and fixes a number of bugs around the
anyrefpass inwasm-bindgen. Most of the issues were around initialization and tests, but after this commit the full test suite passes in Node.js. Yay!