Stream: git-wasmtime

Topic: wasmtime / PR #1834 Allow ModuleTranslationState to be co...

Wasmtime GitHub notifications bot (Jun 06 2020 at 00:35):

cfallin opened PR #1834 from module-translation-state-multivalue to master:

This is needed to allow SpiderMonkey to provide function signature types
to the wasm translator when it uses Cranelift as a backend without
using the wasm translator to parse the entire module. There is perhaps a
better long-term design here where we allow an embedding that already
parses the Wasm module (such as SpiderMonkey) to provide information in
a more principled way, this suffices for now.

Patch is inspired by @bnjbvr's patch in
https://bugzilla.mozilla.org/show_bug.cgi?id=1641504, but does not
expose wasmparser types directly, instead using Cranelift types across
the API boundary.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Jun 06 2020 at 00:35):

cfallin requested bnjbvr for a review on PR #1834.

Wasmtime GitHub notifications bot (Jun 08 2020 at 15:32):

bnjbvr created PR Review Comment:

Could the type of the two parameters be &[Type], instead? it would be slightly simpler for users, and wouldn't require the extra call to into_boxed_slice on the Spidermonkey side.

Wasmtime GitHub notifications bot (Jun 08 2020 at 15:32):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Jun 08 2020 at 15:32):

bnjbvr created PR Review Comment:

(am i understanding correctly that there's Something that infers that we want a Result<Vec<T>> from a Vec<Result<T>>? I can imagine how this works, but I didn't know it was available, this is brilliant!)

Wasmtime GitHub notifications bot (Jun 08 2020 at 15:32):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Jun 08 2020 at 19:19):

cfallin updated PR #1834 from module-translation-state-multivalue to master:

This is needed to allow SpiderMonkey to provide function signature types
to the wasm translator when it uses Cranelift as a backend without
using the wasm translator to parse the entire module. There is perhaps a
better long-term design here where we allow an embedding that already
parses the Wasm module (such as SpiderMonkey) to provide information in
a more principled way, this suffices for now.

Patch is inspired by @bnjbvr's patch in
https://bugzilla.mozilla.org/show_bug.cgi?id=1641504, but does not
expose wasmparser types directly, instead using Cranelift types across
the API boundary.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Jun 08 2020 at 19:19):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jun 08 2020 at 19:19):

cfallin created PR Review Comment:

Done!

Wasmtime GitHub notifications bot (Jun 08 2020 at 19:20):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jun 08 2020 at 19:20):

cfallin created PR Review Comment:

Indeed, there is apparently a FromIterator implementation for Result that makes this work!

Wasmtime GitHub notifications bot (Jun 08 2020 at 20:00):

cfallin merged PR #1834.

Last updated: Dec 23 2024 at 12:05 UTC