Skip to content
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

Add docs to wasmtime-c-api-macros generated methods #8957

Merged

Conversation

Robbepop
Copy link
Contributor

@Robbepop Robbepop commented Jul 15, 2024

I didn't know what some of those methods even do. The only docs I could find where the Wasmtime doxygen docs which only stated that most of the methods are simply not implemented.

So in parts I hope to improve Wasmtime C-API by improving the docs situation and in other parts I hope to find their meaning with this PR. :)

Due to missing knowledge I consider the automatically generated docs (of this PR) of get/set_host_info, as_ref and as_ref_const as temporary until we (I) know better.

Rendered auto generated docs:

image

@Robbepop Robbepop requested a review from a team as a code owner July 15, 2024 11:19
@Robbepop Robbepop requested review from elliottt and removed request for a team July 15, 2024 11:19
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Looks like you've found that some of these aren't supported which is intentional, but otherwise having docs is deifnitely an oversight 👍

@alexcrichton alexcrichton added this pull request to the merge queue Jul 15, 2024
Merged via the queue into bytecodealliance:main with commit 180b6dd Jul 15, 2024
39 checks passed
@Robbepop
Copy link
Contributor Author

Robbepop commented Jul 15, 2024

Hmmm, I really wanted to learn what those APIs I mentioned in my first post are about. They are not properly documented in the standard C API (or I failed to find their docs) and it is still unclear to me what they are supposed to do. But at least Wasmtime now has a bit better documentation story for its Rust C API. :)

@alexcrichton
Copy link
Member

Oh sorry, I can try to add more color but honestly I'm not certain what they are myself. The upstream wasm-c-api proposal to wasm has been largely inactive for quite some time now and AFAIK there's not much drive/push to revinvigorate it or keep it going. In that sense the best option would be to probably open an issue on the repository itself.

If I had to hazard a guess, I believe that the intention was that all pointer-lookalike-things could be turned into generic references which could then be converted back out at some point. I believe the original goal was to eventually unify the wasm type system itself to allow passing around first-class-references to more than just functions but also tables/memories/etc. In the interim time I don't believe that's panned out and in Wasmtime we don't implement any of these because they don't actually map to internals of Wasmtime.

AFAIK there also hasn't been much usage of the wasm-c-api header file as a means of integrating with a wasm engine. All usage I've seen additionally requires implementation-specific APIs (e.g. wasmtime_* in the Wasmtime C API). Many out-of-browser engines I've seen also don't implement this API as well.

Overall I don't think things are in a state where it's expected someone can walk in and see a vibrant and well documented ecosystem of engines and users all interoperating with this API. I believe that was an original goal, but it hasn't quite panned out.

@Robbepop
Copy link
Contributor Author

Robbepop commented Jul 15, 2024

@alexcrichton Thank you so much for the elaborated response. It is also really good to know (albeit sad) about the state of the Wasm C API maintenance. I hoped it was achieving its set goals. At least I plan to implement it in Wasmi soon(TM) to allow non-Rust environments to use Wasmi.

Also thanks a lot for your elaborated guess what you think those APIs are meant to do. What you said makes kinda sense to me. I will take your suggestion of posting further questions directly in the repository. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants