Open Ekleog opened 3 years ago
Actually I've just been pointed to the fact that Linker::module()
already does it. I have just been misusing Linker
by assuming it was designed to give out an Instance
, instead of to directly give out the exports with get_one_by_name
.
As such, let's repurpose this issue for documentation, to avoid that other people : maybe it should be made clear in the documentation that Linker
s are there for when the user needs to link multiple modules together, and that often instantiate()
should be replaced by module()
and get_one_by_name()
?
Also… maybe it'd make sense to expose the same convenience getters on Linker
as on Instance
? The diff of my switch to using Linker::module
instead of Linker::instantiate
isn't that great :) https://github.com/Ekleog/kannader/commit/ee7de0461f62bb50420503ab7bef98b5e7149165
Thanks for filing this! I agree, it would be good to clarify the docs in this area.
If I read the diff correctly, the main convenience methods you'd want on Linker
are get_func
and get_memory
, right? The Linker
API does want to be careful about cases where there are multiple exports that match—hence the get_one_*
naming. But offhand it does seem like accessors dedicated for querying functions etc. would be useful.
Yes, that's exactly it :)
How do I make sure that the WASM module that I created is a Reactor? There doesn't seem to be much documentation on what makes a Reactor and Command, and why a module would be build as one over another?
Any module with an _initialize function is a reactor I believe. The difference between a command and a reactor is that a command must be destroyed after calling the main function, while a reactor requires running the _initialize function and then you are free to run any exported functions you want on the instance. It is kind of like a binary (command) vs library (reactor).
Do I have to manually put the function there via func_wrap
or something like that? My code is definitely a binary, it has an empty fn main() {}
See https://github.com/lapce/lapce-rust/blob/master/src/main.rs#L31
which expands like so https://github.com/lapce/lapce-plugin-rust/blob/master/src/lib.rs#L99-L125
Edit: Wait did I get that right, a command is the stateful one?
It will have to be compiled as reactor if you want to use wasi. Otherwise at_exit entries and global destructors will run right after the rust main function there is called and before lapce can call handle_rpc: https://github.com/WebAssembly/wasi-libc/blob/9bec2d3aff198770e98544cb6f13add60e1f5fe6/libc-bottom-half/crt/crt1-command.c#L46
I can't seem to use the flag that you mentioned in Zulip. Perhaps if I change the plugin to look like a lib.rs it will be compiled as a reactor automatically?
hbina@akarin ~/g/lapce-rust (master)> cargo +nightly -Zwasi-exec-model=reactor build
error: unknown `-Z` flag specified: wasi-exec-model
You have to pass it to rustc, not cargo. So for example RUSTFLAGS="-Zwasi-exec-model=reactor" cargo +nightly build
or cargo +nightly rustc -- -Zwasi-exec-model=reactor
.
Perhaps if I change the plugin to look like a lib.rs it will be compiled as a reactor automatically?
I don't think it does, but you could try.
The below feature description is actually already implemented, so I repurposed that issue as a documentation issue, see the second message
Feature
Automatically call
_initialize
for reactors, maybe using a special function to create a reactor Module?Benefit
Knowing one has to call
_initialize
is something that is not very documented (well… ok, currently nothing is very documented, but seeing as support for wasi reactors landed in rustc like two days ago it's not surprising… anyway my struggles are documented here: https://github.com/rust-lang/rust/pull/79997#issuecomment-759856446)Implementation
Maybe this would be better with a separate function? But I somehow feel like calling
linker.instantiate(&module)
automatically calling_initialize
would make sense… or maybe add a mandatory argument so people must express the choice of whether they want their module to be initialized at instantiation time or not, and at least know they have to do it manually if they want not to?Alternatives
Keep the statu quo, ie. let the user call
_initialize
manually. IMO that would require much more documentation about the topic, though it's definitely a possibility too.Thank you for all you do on wasm and the surrounding ecosystem! :heart: