MangoPeachGrape opened PR #10566 from MangoPeachGrape:c-api/component-model/compile to bytecodealliance:main:
First little bit of #9812, "compile a component", with some changes suggested in the original pr:
- No longer depends on cranelift, feature gated compile function on
WASMTIME_FEATURE_COMPILER.- Moved config switch to config.h.
- Prepared the file structure for multiple header files, should the same be done for the rust side?
Based on the efforts of @lanfeust69 and @rockwotj.
MangoPeachGrape requested wasmtime-core-reviewers for a review on PR #10566.
MangoPeachGrape requested fitzgen for a review on PR #10566.
alexcrichton submitted PR review:
Thanks!
alexcrichton created PR review comment:
Could this be called
wasmtime_component_newto look similar to modules?
alexcrichton created PR review comment:
Could this be changed to
<wasmtime/component/component.h>like other wasmtime-specific imports?
alexcrichton commented on PR #10566:
Also if you're up for it feel free to fill out this header to match the
module.hheader, e.g. with deserializing and such. Reflection on imports/exports though is going to be a "big deal" so deferring that is probably best.
MangoPeachGrape updated PR #10566.
MangoPeachGrape commented on PR #10566:
Also if you're up for it feel free to fill out this header to match the
module.hheader, e.g. with deserializing and such. Reflection on imports/exports though is going to be a "big deal" so deferring that is probably best.Added
wasmtime_component_serialize()andwasmtime_component_serialize(). Should I dowasmtime_module_deserialize_file()as well, or any other functions?For the documentation in the header file I did
s/module/component/, so it might not be accurate. In general, do you want documentation on everything in the header files in these early commits, or is that something that can be added in PRs down the line?
MangoPeachGrape edited a comment on PR #10566:
Also if you're up for it feel free to fill out this header to match the
module.hheader, e.g. with deserializing and such. Reflection on imports/exports though is going to be a "big deal" so deferring that is probably best.Added
wasmtime_component_serialize()andwasmtime_component_deserialize(). Should I dowasmtime_module_deserialize_file()as well, or any other functions?For the documentation in the header file I did
s/module/component/, so it might not be accurate. In general, do you want documentation on everything in the header files in these early commits, or is that something that can be added in PRs down the line?
alexcrichton commented on PR #10566:
wasmtime_component_deserialize_filesounds good to add yeah, and maybewasmtime_component_cloneas well, but otherwise it looks like other functions aren't as applicable yet so that's probably good for now.For docs I find it best to add it when the functions come in, but I'll admit during review I generally gloss over docs. I can give them a once-over to confirm everything is right after deserialize_file is added
MangoPeachGrape updated PR #10566.
MangoPeachGrape commented on PR #10566:
I'm unsure if all the
consts are correct, for example inwasmtime_component_clone(const wasmtime_component_t *component), becausewasmtime_module_clone(wasmtime_module_t *m)doesn't have const?Should the rust file also be split up, similar to the header files?
alexcrichton commented on PR #10566:
Ah yeah the const there is ok, and it's one where we aren't necessarily the best-principled, but adding
constwhen it maps to&Tin Rust doesn't hurt for sure.For file organization, yeah if you're up for it I think it would be a good idea. We'll be filling this out more in the future so I think it makes sense to separate everything out by file and starting here feels a bit odd as it's so small but it's predicted to get larger and be more justified in the future.
MangoPeachGrape updated PR #10566.
MangoPeachGrape commented on PR #10566:
For file organization, yeah if you're up for it I think it would be a good idea. We'll be filling this out more in the future so I think it makes sense to separate everything out by file and starting here feels a bit odd as it's so small but it's predicted to get larger and be more justified in the future.
Done!
What should the scope be for the next PR?
MangoPeachGrape updated PR #10566.
alexcrichton submitted PR review.
alexcrichton commented on PR #10566:
Personally I think one major missing piece of foundation for the C API is a good testing story. We don't have many tests for the C API and right now they're all classified as examples rather than unit tests. They're also awkward to write because C is pretty awkward to write.
To solve this one thing we've wanted to do was to fold the wasmtime-cpp repository (C++ bindings build on the C API) into this repository. That would make it much easier to write tests and then we could whip up some cmake as well to get tests.
I know writing tests isn't the most glamorous thing but it's a major thing I'd personally like to see before much of the rest of the C API becomes load bearing because I think the conversion of values is going to be significantly trickier than the wasm C API and warrant more thorough testing.
MangoPeachGrape updated PR #10566.
MangoPeachGrape commented on PR #10566:
Yes, tests would be great!
All the tests would use GoogleTest, would you still seperate the tests for the C api and the C++ api, or would you have those combined?
Is this something you would want me to take a jab at? I'm not that familiar with the codebase, so I would need some guiding where to put everything.
alexcrichton commented on PR #10566:
GoogleTest would be fine yeah, and I think it'd be reasonable to test everything together. I'd expected we'd test C++ APIs by deafult (by moving
wasmtime.hhto this repository) and then we could write one-off tests in C if necessary.If you're willing to work on this that'd be great! Most of us in this repo aren't really all that well-versed in C/C++ repository management so if you know of practices/idioms around organization/testing/CI/etc that'd be very helpful. Most of the source would probably want to live in
crates/c-apior some sub-folder, and I'm happy to help out with CI integration as well.
alexcrichton merged PR #10566.
MangoPeachGrape commented on PR #10566:
@alexcrichton, Some questions that that came up while moving the C++ api into this repository:
- Should the C++ examples go to
examples/, and the tests go tocrates/c-api/tests/?- Should we guard the C++ api to be only built if the consumer wants it (some consumers might not be able to build C++)?
- Should C/C++ tests be built by default, or only be enabled by a CMake option when desired?
alexcrichton commented on PR #10566:
Yeah let's put examples in
examples/{example}.{rs,c,cpp}and in theory we can fill out that matrix over time. For tests I thinkcrates/c-api/testsis a good spot too.The C++ API build step from an end consumable artifact point of view should just be a header file so I think it's fine to always enable/copy that around. For the tests I think it's also reasonable to require C++ to build tests since that's just for developers which should be easier in general. Given that I'd hope we can skip CMake options to conditionalize the C++ usage and basically just always use it
Last updated: Dec 06 2025 at 07:03 UTC