Wasmtime GitHub notifications bot (Apr 25 2025 at 20:00): Wasmtime GitHub notifications bot (Apr 25 2025 at 20:00):MangoPeachGrape opened PR #10675 from MangoPeachGrape:c-api/component-model/get-func to bytecodealliance:main:
Only
get_func()as of now, should https://github.com/bytecodealliance/wasmtime/pull/9812#discussion_r1884473950 come in a later PR?One thing of note, if name isn't UTF-8, it returns false instead of returning an error, is that fine?
Wasmtime GitHub notifications bot (Apr 25 2025 at 20:00):MangoPeachGrape requested dicej for a review on PR #10675.
Wasmtime GitHub notifications bot (Apr 25 2025 at 20:00):MangoPeachGrape requested wasmtime-core-reviewers for a review on PR #10675.
Wasmtime GitHub notifications bot (Apr 25 2025 at 20:45):alexcrichton requested alexcrichton for a review on PR #10675.
Wasmtime GitHub notifications bot (Apr 25 2025 at 20:51):alexcrichton commented on PR #10675:
Thanks! This has triggered some further thoughts which are both preexisting and related to this PR as well:
- For names I realize now we shouldn't use
const char*, but instead we should be usingwasm_name_t*. Unicode can't be represented asconst char*due to the lack of ability to support interior nul characters, so we'll want to change this and other APIs to usewasm_name_tinstead.- For this I think we'll want to base lookup of an item on an
ComponentExportIndexinstead of a string, although a string can be used to acquire anComponentExportIndex. Effectively I think we should bind{Instance,Component}::get_export_indexand then an export index is used to lookup a function. That'll handle nested instances as well as top-level exports in the same manner.
Wasmtime GitHub notifications bot (Apr 25 2025 at 20:53):alexcrichton commented on PR #10675:
Er, sent too soon:
One thing of note, if name isn't UTF-8, it returns false instead of returning an error, is that fine?
Yeah sounds good!
One other slightly orthogonal thing, I think it'd be nice to help clean up the C tests a bit. If you'd like I think it'd be reasonable to export the C API implementation pointer from C++ structures, that way you could sort of intermix C++ and C where the C++ APIs could be used to handle auto-destruction and such. That might also make it easier to bind optional errors and assert that errors don't happen. Otherwise could the
CHECKmacro be deduplicated in a header instead of duplicated between files?
Wasmtime GitHub notifications bot (Apr 27 2025 at 14:28):MangoPeachGrape updated PR #10675.
Wasmtime GitHub notifications bot (Apr 27 2025 at 14:33):MangoPeachGrape updated PR #10675.
Wasmtime GitHub notifications bot (Apr 28 2025 at 14:54):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2025 at 14:54):alexcrichton created PR review comment:
Could this function return the nullable pointer directly perhaps?
Wasmtime GitHub notifications bot (Apr 28 2025 at 14:54):alexcrichton created PR review comment:
Oh for this I think we'll want to have this be
const wasm_name_t*in C and not take ownership here.Also I apologize again for yet more whiplash... Looking at
wasmtime_linker_tit looks like the functions there idiomatically take a pointer/length instead of&wasm_name_t(e.g.wasmtime_linker_define). I had forgotten about that wherewasm_name_tI think is mostly used for return values as opposed to arguments. In light of that could the idioms of the component linker be similar where a pointer/length is given? (sorry I should have checked this earlier)
Wasmtime GitHub notifications bot (Apr 28 2025 at 17:24):MangoPeachGrape updated PR #10675.
Wasmtime GitHub notifications bot (Apr 28 2025 at 17:25):MangoPeachGrape updated PR #10675.
Wasmtime GitHub notifications bot (Apr 28 2025 at 17:38):MangoPeachGrape updated PR #10675.
Wasmtime GitHub notifications bot (Apr 28 2025 at 18:57):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2025 at 19:06):MangoPeachGrape updated PR #10675.
Wasmtime GitHub notifications bot (Apr 28 2025 at 19:13):MangoPeachGrape commented on PR #10675:
Next up values, which are required for function invocation?
Wasmtime GitHub notifications bot (Apr 28 2025 at 19:20):alexcrichton has enabled auto merge for PR #10675.
Wasmtime GitHub notifications bot (Apr 28 2025 at 19:21):alexcrichton commented on PR #10675:
Yeah that seems reasonable. I'll be honest though in that I don't really know how to do values. Everything I've thought of historically is either extremely chatty over the C API boundary or extremely allocation-heavy, neither of which I feel is great... If you've got ideas though please dive in!
Wasmtime GitHub notifications bot (Apr 28 2025 at 19:45):alexcrichton merged PR #10675.
Last updated: Jan 10 2026 at 02:36 UTC