Stream: git-wasmtime

Topic: wasmtime / PR #9822 Add async example to bindgen_examples

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2024 at 15:12):

ifsheldon requested fitzgen for a review on PR #9822.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2024 at 15:12):

ifsheldon requested wasmtime-core-reviewers for a review on PR #9822.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 14 2024 at 15:12):

ifsheldon opened PR #9822 from ifsheldon:async_example to bytecodealliance:main:

Closes #9776

I reused most of example 4 to avoid extra complication. Also adds a reference to code example wasi_async, so it'd better if this gets merged after #9788.

cc @alexcrichton

view this post on Zulip Wasmtime GitHub notifications bot (Dec 16 2024 at 15:37):

alexcrichton commented on PR #9822:

Thanks, and looks good to me! I think there's some errors on CI though?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 16 2024 at 17:19):

ifsheldon commented on PR #9822:

Can you take a look into the clippy error please? The error seems to originate from bindgen!. I suspect that some code in bindgen! forgets to import Box when a WIT spec is inlined. In my own example that imports from a WIT file, there's no problem. In the example 4 where async is not used, it's also fine. I have no clue why that happened. The difference between _7_async.rs and _4_imported_resources.rs is just the line async: true,

view this post on Zulip Wasmtime GitHub notifications bot (Dec 16 2024 at 21:34):

alexcrichton commented on PR #9822:

Ah I believe this is exposing a preexisting bug in the bindgen! macro, notably the async output is not compatible with #![no_std] crates. The wasmtime crate itself is such a crate which is where this is coming from.

I'm not sure of a way to work around this other than fixing the issue at hand (tracking down the Box and importing it from wasmtime::component::__internal::Box)

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2024 at 02:41):

ifsheldon commented on PR #9822:

I think for the purpose of documentation, it's okay to let clippy skip this file.

this is exposing a preexisting bug in the bindgen! macro, notably the async output is not compatible with #![no_std] crates. The wasmtime crate itself is such a crate which is where this is coming from.

IIUC, #9823 maybe unblock this?

I'm not sure of a way to work around this other than fixing the issue at hand

Can you try to fix this please? I'm not very familiar proc_macros or bindgen internals.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2024 at 18:56):

alexcrichton commented on PR #9822:

Yes #9823 would probably help. Unfortunately I don't have time right now to fix this myself, and this can't land unless it's passing CI.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 18 2024 at 02:42):

ifsheldon commented on PR #9822:

OK...... Can you find the issue tracking the "preexisting bug" you mentioned? I can't find one to link here. Or, I can file a new one.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 18 2024 at 18:52):

alexcrichton commented on PR #9822:

I don't believe there's a preexisting issue since this is I think the first time it's come up, but feel free to file an issue of course

view this post on Zulip Wasmtime GitHub notifications bot (Dec 19 2024 at 15:26):

ifsheldon commented on PR #9822:

OK, let's take a step back. Why need to build bindgen_examples inside wasmtime anyway? I think we can just move all the bindgen examples out of wasmtime crate.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 19 2024 at 22:23):

alexcrichton commented on PR #9822:

It's not necessarily strictly required, but it's quite useful to be distributed as part of docs.rs integration. That gives nice API docs all in one place. Personally I think it'd probably be less effort to fix the code generation than it would be to move all the documentation somewhere else.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 15:17):

ifsheldon updated PR #9822.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 15:20):

ifsheldon commented on PR #9822:

OK, fine. I made a new commit which fixes the bug of bindgen. I know I should have open another PR/issue but it seems I need to provide an example to reproduce the bug, which is this very example, so I think it's okay to just fix it in this PR.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 16:04):

alexcrichton commented on PR #9822:

Thanks! I think some test expectations may need an update, but otherwise looks good :+1:

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 17:35):

ifsheldon updated PR #9822.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 17:48):

ifsheldon updated PR #9822.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 18:01):

ifsheldon updated PR #9822.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 19:34):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 20 2024 at 19:52):

alexcrichton merged PR #9822.

Last updated: Dec 23 2024 at 12:05 UTC