Stream: git-wasmtime

Topic: wasmtime / PR #4219 Enable passing host functions to comp...

Wasmtime GitHub notifications bot (Jun 03 2022 at 21:06):

alexcrichton opened PR #4219 from component-import to main:

This commit implements the ability to pass a host function into a
component. The wasmtime::component::Linker type now has a func_wrap
method allowing it to take a host function which is exposed internally
to the component and available for lowering.

This is currently mostly a "let's get at least the bare minimum working"
implementation. That involves plumbing around lots of various bits of
the canonical ABI and getting all the previous PRs to line up in this
one to get a test where we call a function where the host takes a
string. This PR also additionally starts reading and using the
may_{enter,leave} flags since this is the first time they're actually
relevant.

Overall while this is the bare bones of working this is not a final spot
we should end up at. One of the major downsides is that host functions
are represented as:

F: Fn(StoreContextMut<'_, T>, Arg1, Arg2, ...) -> Result<Return>

while this naively seems reasonable this critically doesn't allow
Return to actually close over any of its arguments. This means that if
you want to return a string to wasm then it has to be String or
Rc<str> or some other owned type. In the case of String this means
that to return a string to wasm you first have to copy it from the host
to a temporary String allocation, then to wasm. This extra copy for
all strings/lists is expected to be prohibitive. Unfortuantely I don't
think Rust is able to solve this, at least on stable, today.

Nevertheless I wanted to at least post this to get some feedback on it
since it's the final step in implementing host imports to see how others
feel about it.

<!--

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 03 2022 at 21:06):

alexcrichton requested fitzgen for a review on PR #4219.

Wasmtime GitHub notifications bot (Jun 06 2022 at 17:03):

alexcrichton updated PR #4219 from component-import to main.

Wasmtime GitHub notifications bot (Jun 06 2022 at 18:36):

alexcrichton updated PR #4219 from component-import to main.

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen created PR review comment:

Are you still planning on splitting ComponentValue into Lift and Lower traits?

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen created PR review comment:

    /// returned directly, namely the arguments and return value all start from

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen created PR review comment:

The "this" at the start of the second sentence is a bit ambiguous here. Suggest rewording like so:

    // Perform a dynamic check that this instance can indeed be left. Exiting
    // the component is disallowed, for example, when the `realloc` function
    // calls a canonical import.

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen created PR review comment:

Should this return impl Drop instead of Any?

Wasmtime GitHub notifications bot (Jun 06 2022 at 21:28):

fitzgen created PR review comment:

    /// Representation of arguments to this function when the return value is

Wasmtime GitHub notifications bot (Jun 07 2022 at 13:13):

alexcrichton created PR review comment:

Indeed! Just haven't gotten around to it yet.

Wasmtime GitHub notifications bot (Jun 07 2022 at 13:13):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 07 2022 at 13:15):

alexcrichton updated PR #4219 from component-import to main.

Wasmtime GitHub notifications bot (Jun 07 2022 at 14:39):

alexcrichton merged PR #4219.

Last updated: Nov 22 2024 at 17:03 UTC