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
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-L352cc @alexcrichton , we discussed this on a call, I believe this is the changeset we agreed upon
rvolosatovs has marked PR #7783 as ready for review.
rvolosatovs requested pchickey for a review on PR #7783.
rvolosatovs requested wasmtime-core-reviewers for a review on PR #7783.
pchickey requested alexcrichton for a review on PR #7783.
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:
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton submitted PR review:
Looks great to me, thanks! Mind adding a test as well for this?
rvolosatovs updated PR #7783.
rvolosatovs updated PR #7783.
rvolosatovs requested alexcrichton for a review on PR #7783.
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.
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.
alexcrichton merged PR #7783.
Last updated: Dec 23 2024 at 13:07 UTC