Stream: git-wasmtime

Topic: wasmtime / PR #7783 fix: allow dynamic owned resources to...

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 18:59):

rvolosatovs opened PR #7783 from rvolosatovs:fix/dyn-owned-resource-borrow to bytecodealliance:main:

This enables using owned resources in places where borrowed resources of the same type are expected (e.g. dynamically-typed functions/methods defined in Linker::func_new)
Note, that this is already possible in e.g. https://github.com/bytecodealliance/wasmtime/blob/4b2425dc3044ec57ddbfa94bcbe2d774df249d03/crates/wasmtime/src/component/resources.rs#L340-L352

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 19:00):

rvolosatovs edited PR #7783:

This enables using owned resources in places where borrowed resources of the same type are expected (e.g. dynamically-typed functions/methods defined in Linker::func_new)
Note, that this is already possible in e.g. https://github.com/bytecodealliance/wasmtime/blob/4b2425dc3044ec57ddbfa94bcbe2d774df249d03/crates/wasmtime/src/component/resources.rs#L340-L352

cc @alexcrichton , we discussed this on a call, I believe this is the changeset we agreed upon

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 20:38):

rvolosatovs has marked PR #7783 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 20:38):

rvolosatovs requested pchickey for a review on PR #7783.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 20:38):

rvolosatovs requested wasmtime-core-reviewers for a review on PR #7783.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 21:22):

pchickey requested alexcrichton for a review on PR #7783.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2024 at 23:44):

github-actions[bot] commented on PR #7783:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 00:37):

alexcrichton submitted PR review:

Looks great to me, thanks! Mind adding a test as well for this?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 12:22):

rvolosatovs updated PR #7783.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 12:24):

rvolosatovs updated PR #7783.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 12:25):

rvolosatovs requested alexcrichton for a review on PR #7783.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 12:30):

rvolosatovs commented on PR #7783:

Added a test case, but also uncovered a bug while doing that https://github.com/bytecodealliance/wasmtime/issues/7793

Honestly, I'm not sure what's the use case for using the resource borrows pointing to non-existent resources and it appears to me that neither of Resources::new_borrow cases should actually work (without panic though), but perhaps I don't understand the semantics here fully.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 16:54):

alexcrichton submitted PR review:

Thanks for the test! I believe that issue is orthogonal to this, so I'm going to mark this for merge while that other issue is sorted out.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2024 at 17:29):

alexcrichton merged PR #7783.

Last updated: Nov 22 2024 at 17:03 UTC