Conversation
# Conflicts: # Cargo.lock
dicej
left a comment
dicej
left a comment
There was a problem hiding this comment.
Looks like a solid start, thanks!
| return (uint)CallbackCode.Yield; | ||
| "#); | ||
| } | ||
|
|
There was a problem hiding this comment.
Might want to add a "TODO" comment here to remember to defer dropping borrowed resources until a result is returned (i.e. just before calling {name}TaskReturn).
| "# | ||
| ); | ||
|
|
||
| let task_return_param = match &sig.results[..] { |
There was a problem hiding this comment.
The task return function could accept up to 16 core parameters (e.g. 2 for a string, 5 for a tuple<string, list<u8>, u32>, etc.).
There was a problem hiding this comment.
I'm hoping I'll be forced to address all these shortcuts as I go through enabling the tests. If not, then I've added a TODO here and I can address the todo's at the end.
crates/csharp/src/function.rs
Outdated
| let requires_async_return_buffer_param = is_async && sig.results.len() >= 1; | ||
| let async_return_buffer = if requires_async_return_buffer_param { | ||
| let buffer = self.emit_allocation_for_type(&sig.results); | ||
| uwriteln!(self.src, "//TODO: store somewhere with the TaskCompletionSource."); |
There was a problem hiding this comment.
I think you can use new TaskCompletionSource(object? state) -> Task.AsyncState
Probably with some (internal?) helper struct/class to hold allocations and handles that you need to cleanup.
There was a problem hiding this comment.
Thanks, I've updated the comment here, as per the remark above I'm hoping to be forced to address all these as I go through enabling the tests.
|
Could you please create and link a gist with the generated code ? Thank you. |
Has the code gen and copies of the harness code from this repo. |
|
Have reverted the code gen interop output as that is a bigger change than can be snuck in here. |
jsturtevant
left a comment
jsturtevant
left a comment
There was a problem hiding this comment.
Looks like a good start to me. Thanks for getting this started! I assume eventually there will need to be some changes to the runtime to support Async?
| * Helpers for the async support. | ||
| */ | ||
|
|
||
| public enum CallbackCode |
There was a problem hiding this comment.
Looks like another type would eventually want to pull out to make them sharable.
There was a problem hiding this comment.
Yes, there is some cleanup/sharing for size that we should do at some point.
I don't think so, we should have everything in place. As I flesh out more tests we will see, but haven't seen anything yet, either here on in the futures PR that requires a change. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
5 participants
This PR add the very minimal changes to enable the first async functions tests to pass. These tests return immediately which means a lot of the callback, waitable, changes are not done yet. There are no TaskCompletionSources created, we just set the return value on the task. User code will look like
https://github.com/bytecodealliance/wit-bindgen/pull/1346/files#diff-f34015927bd91d8f86e20e1a9258891caad407de6a499fcf977cb1165c89b3a5
and
https://github.com/bytecodealliance/wit-bindgen/pull/1346/files#diff-756d40996fd1204ae5975dd4da40e815b827d804c27b13dd36f69b6612ced825
Which should be idiomatic.
Thanks @dicej for the help in Zulip.