-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Caller get_export() implemented for Extern::Func. #2108
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update tests as well:
diff --git a/tests/all/func.rs b/tests/all/func.rs
index ddce6b1e6..953976e1e 100644
--- a/tests/all/func.rs
+++ b/tests/all/func.rs
@@ -409,7 +409,7 @@ fn caller_memory() -> anyhow::Result<()> {
let f = Func::wrap(&store, |c: Caller<'_>| {
assert!(c.get_export("m").is_some());
- assert!(c.get_export("f").is_none());
+ assert!(c.get_export("f").is_some());
assert!(c.get_export("g").is_none());
assert!(c.get_export("t").is_none());
});
Subscribe to Label Actioncc @peterhuene
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 |
Signed-off-by: Christopher Agia <chrisagia@google.com>
Thanks for the PR! Could you outline a bit the motivation for this change? Our intention is to sort of keep the In any case though I'm curious to hear more about your use case, since we might still be able to cater to it one way or another! |
@alexcrichton @agiachris is working on a test framework for Proxy-Wasm modules (and hopefully WASI in the future), which uses Wasmtime as the runtime, and we needed a way for |
Ok thanks for the background. It looks like y'all are primarily using this to lookup the calling module's Could you also update the documentation though to leave the clause that it's only implemented for some types? Additionally can you add some words to the documentation that for functions it should in general follow the guidelines of the interface types proposal for accessing exports and calling them? Finally, one thing I wanted to point out, y'all will want to be extremely careful when accessing the caller's memory. The main idea is that once you get a raw pointer into wasm memory you can't call wasm again because that pointer could get relocated. In proxy-wasm/test-framework#1 the |
@alexcrichton thanks for the suggestion, we will make those changes. I've gone ahead and updated the documentation for the implementation only supporting specific types (Memory and Func), however I'm unfamiliar with the interface types guidelines. Could you please suggest the necessary documentation text for this? |
Signed-off-by: Christopher Agia <chrisagia@google.com>
Thanks! |
To my knowledge, this has not been discussed in an issue. However, comments in the overwritten code make clear that the get_export() function is only currently implemented for Extern::Memory, with potential future support for Extern::Func - which is the update of this PR. This update simply enables user to lookup and return Extern::Func types from Caller structure.