agiachris opened PR #2108 from main
to main
:
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.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[x] This has been discussed in issue #..., or if not, please tell us why
here.[x] 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.[x] This PR contains test cases, if meaningful.
- [x] 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.
-->
PiotrSikora submitted PR Review.
PiotrSikora created PR Review Comment:
Style:
s/return Some(Extern::Func(func));/Some(Extern::Func(func))/
(i.e. noreturn
, no;
).
PiotrSikora created PR Review Comment:
Style:
s/return Some(Extern::Memory(mem));/Some(Extern::Memory(mem))/
(i.e. noreturn
, no;
).
PiotrSikora submitted PR Review.
PiotrSikora created PR Review Comment:
Style:
s/return None,/None,/
(i.e. noreturn
).
agiachris updated PR #2108 from main
to main
:
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.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[x] This has been discussed in issue #..., or if not, please tell us why
here.[x] 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.[x] This PR contains test cases, if meaningful.
- [x] 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.
-->
PiotrSikora submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Could this be refactored a bit to avoid duplicating the
let handle = ...
?Something like:
let export = instance.lookup(name)?: let handle = ...; Some(match export { Export::Memory(m) => Export::Memory(Memory::from_wasmtime_memory(m, handle)), Export::Function(f) => /* ... */, _ => return None, }) ~~~
alexcrichton created PR Review Comment:
I think that the documentation on the
Caller
type itself needs to be updated to indicate it's more than just memory now, and this function can probably point to that documentation to guide about usages of the function.On the
Caller
struct itself I think it can be updated to something like:This structure can be taken as the first parameter of a closure passed to
[Func::wrap
], and it can be used to learn information about the caller of
the function, such as the calling module's memory, exports, etc.The primary purpose of this structure is to provide access to the
caller's information, namely it's exported memory and exported functions. This
allows functions which take pointers as arguments to easily read the memory the
pointers point into, or if a function is expected to callmalloc
in the wasm
module to reserve space for the output you can do that.Note that this
Caller
type a pretty temporary mechanism for accessing the
caller's information until interface types has been fully standardized and
implemented. The interface types proposal will obsolete this type and this will
be removed in the future at some point after interface types is implemented. If
you're relying on thisCaller
type it's recommended to become familiar with
interface types to ensure that your use case is covered by the proposal.
agiachris submitted PR Review.
agiachris created PR Review Comment:
Great, will make these updates. Thank you!
agiachris updated PR #2108 from main
to main
:
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.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[x] This has been discussed in issue #..., or if not, please tell us why
here.[x] 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.[x] This PR contains test cases, if meaningful.
- [x] 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.
-->
alexcrichton merged PR #2108.
Last updated: Dec 23 2024 at 12:05 UTC