stack has 1 items remaining

[peter-jerry-ye]

I'm working on a new MoonBit implementation, and I wanted to add runner test for simple-call-import. However, I got this error :

stack has 1 items remaining: ["result"]

The emitted instructions are the following, which does not seem balanced to me.

CallWasm { name: "f", sig: WasmSignature { params: [], results: [I32], indirect_params: false, retptr: false } }
Return { amt: 0, func: Function { name: "f", kind: AsyncFreestanding, params: [], result: None, docs: Docs { contents: None }, stability: Unknown } }

To reproduce, with this commit: https://github.com/peter-jerry-ye/wit-bindgen/tree/0b4b5c35bd88b8caf080d69ce6ebd8c03dfcb861

cargo run test --languages moonbit,rust --artifacts target/artifacts ./tests/runtime-async/async/simple-call-import

[alexcrichton]

My hunch is that this is likely due to the exact way you're pushing/popping for some async instructions, so helping to diagnose further would require reading over/reviewing the moonbit implementation. Before that though, have you double-checked that for all the async instructions the moonbit implementatino is doing something similar to the Rust/C implementations?

[peter-jerry-ye]

I'm not sure if we are following the same way as Rust / C. I'm using the stackless way to generate the instructions. And this issue only happens when I try to generate a runner instead of a tester.

[alexcrichton]

Hm ok I'm not entirely sure what's going on but my guess is that you might be using a configuration/mode/something that Rust/C isn't exercising and probably has bugs in it. I would agree that CallWasm or Return should either one or both have "async" somewhere in the name to deal with the handling of the status code.

Looking at the Rust bindings generator in generate_guest_import the async/non-async paths are entirely different, so you might need to mirror that structure? Either that or the core/abi bits will need updates for this

[peter-jerry-ye]

I created here a test case to show the issue: https://github.com/peter-jerry-ye/wit-bindgen/blob/zihang/report-bug/crates/core/tests/call_async_import.rs

To run the test you'll need cargo test --package wit-bindgen-core --test call_async_import.

Within the test, I created a dummy bindgen impl that will make sure that the results get enough arguments. Then I simply called:

let mut bindgen = TestBindgen::default();
abi::call(
    &Resolve::default(),
    *variants,
    LiftLower::LowerArgsLiftResults,
    function,
    &mut bindgen,
    *async_,
);

where the variants may be GuestImport or GuestImportAsync, and function may be func() or async func(), and async_ may be true or false.

With all the combination, it seems that when the variant is GuestImportAsync, it simply fails.

And it seems that none of the current implementation called abi::call with this variant. People seem all trying to make up their own ways to handle the async case. So maybe I misunderstood the API?

[peter-jerry-ye]

I suspect the abi::call for LiftLower::LowerArgsLiftResults emitting Result should change the last part from:

                // Emit the function return
                self.emit(&Instruction::Return {
                    func,
                    amt: usize::from(func.result.is_some()),
                });

to

                // Emit the function return
                self.emit(&Instruction::Return {
                    func,
                    amt: usize::from(func.result.is_some() || variant == AbiVariant::GuestImportAsync), // always 1 for status code?
                });

But this could mean that it may need special handling of the last status code. It would be great if Return could have a field async_ : bool as well.

[alexcrichton]

Unfortunately that wouldn't quite work because the return value of the core wasm import is a status code which needs to be processed, not returned. In the Rust generator it's effectively not using abi::call for async functions but instead hand-weaving the lifting/lowering in separate steps to handle ownership correctly w.r.t. cancellation and such. I suspect you'll need to do something similar where abi::call effectively doesn't work for async imports. (and we should perhaps make that a panic?)

[peter-jerry-ye]

I see. Yes, it seems that abi::call should document that it doesn't support async import, and async import should be handled otherwise (manual lowering, lifting, and handle the status code)