wasmtime / PR #10651 c-api: Module definitions in compone... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #10651 c-api: Module definitions in compone...

Wasmtime GitHub notifications bot (Apr 23 2025 at 11:14):

MangoPeachGrape opened PR #10651 from MangoPeachGrape:c-api/component-model/module-def to bytecodealliance:main:

See previous comment https://github.com/bytecodealliance/wasmtime/pull/10598#issuecomment-2818696550.
Sketched out a similar approach to the one in #9812. I haven't finished it, as I'm quite unsure if this is the way to go.

Would something like this be better?:
// insert into the NameMap (error if shadow)
wasmtime_component_linker_define_instance(linker, "aa");

// fetch "aa" from the NameMap (error if not defined), then define the module inside it
wasmtime_component_linker_define_module(linker, "aa.module", module);
That would require adding function to LinkerInstance, similar to into_instance(), but instead of inserting to the map, it would fetch.

Is that a valid/better approach?

Also, is seperating the path parts by . fine, or should the path be passed as const char**?

Also also, @alexcrichton, could you provide me a nested test component, as I couldn't for the life of me get the syntax correct? Thanks

Wasmtime GitHub notifications bot (Apr 23 2025 at 11:14):

MangoPeachGrape requested alexcrichton for a review on PR #10651.

Wasmtime GitHub notifications bot (Apr 23 2025 at 11:14):

MangoPeachGrape requested wasmtime-core-reviewers for a review on PR #10651.

Wasmtime GitHub notifications bot (Apr 23 2025 at 14:56):

alexcrichton commented on PR #10651:

Ok I want to definitely apologize -- what I'm thinking now is "let's go back to what you had before".

While I agree with my sentiment in https://github.com/bytecodealliance/wasmtime/pull/10598#issuecomment-2818696550 it's not a hard-and-fast rule we adhere to in the C API. There are a number of other locations where return values are effectively borrowed from the input value. In that sense it's not like we have to 100% avoid it, it's just best if we can IMO.

Currently the component::Linker API is such that once you call .instance(...) you can't call it again, so what I was originally thinking with that comment was not actually possible where you'd sort of push/pop contexts and/or build up a "path" as you're doing here. Basically the base API is a bit restrictive, and I'm also not sure that we want to change that.

Ok though so on this PR specifically, my thoughts are:

We'll want to avoid . as a path separator (or any other string) to avoid possible clashing with import names in general.

I'm souring on my own idea after seeing what it takes to implement it. I think it would be best to avoid having mirror data structures in the C API on reflection and probably best if we stick to being a thin wrapper around the Rust API.

That all leads me to thinking we should go back to the original API you had sketched out, where the C API is a wrapper around Box<LinkerInstance<'_, T>> more-or-less. I'm really sorry I led you astray with my suggestion, I should have more fully thought through the suggestion before having you put work into it! For that I'm sorry :(

WDYT though about going back to using LinkerInstance raw in the C API? That would I think make the implementation of the C API easier but would require some stern words in the documentation about the lifetime of the resulting pointer and the order in which things must be used/destroyed for example. If you're ok with this approach then I'd recommend re-adding wasmtime_component_linker_instance_t and then have a function wasmtime_component_linker_root which takes a wasmtime_component_linker_t and returns the instance. From that instance you could then project more instances (e.g. wasmtime_component_linker_instance_add_instance (wow that is becoming a mouthful, maybe bikeshed that...) to return a sub-wasmtime_component_linker_instance_t or wasmtime_component_linker_instance_add_module for adding a module as-is)

Wasmtime GitHub notifications bot (Apr 23 2025 at 18:36):

MangoPeachGrape updated PR #10651.

Wasmtime GitHub notifications bot (Apr 23 2025 at 18:45):

MangoPeachGrape commented on PR #10651:

No problem! Let me know if you think of better names, or see any other issues. I'll do docs and tests tomorrow.

Wasmtime GitHub notifications bot (Apr 23 2025 at 18:55):

alexcrichton submitted PR review:

Nice yeah this looks great, thanks again and sorry for the whiplash!

With tests + docs looks good to me :+1:

Wasmtime GitHub notifications bot (Apr 24 2025 at 17:52):

MangoPeachGrape updated PR #10651.

Wasmtime GitHub notifications bot (Apr 24 2025 at 18:02):

MangoPeachGrape commented on PR #10651:

Let me know if you can think of a better way to convey the lifetime rules.

What's next, values?

Wasmtime GitHub notifications bot (Apr 24 2025 at 20:03):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Apr 24 2025 at 20:03):

alexcrichton commented on PR #10651:

Before values, perhaps instantiation? Then maybe loading exports like functions, and then after that function invocations which would be a forcing function on values?

Wasmtime GitHub notifications bot (Apr 24 2025 at 20:24):

alexcrichton merged PR #10651.

Wasmtime GitHub notifications bot (Apr 25 2025 at 16:09):

MangoPeachGrape commented on PR #10651:

@alexcrichton WDYM by instantiation? Something other than wasmtime_component_linker_instantiate()?

Wasmtime GitHub notifications bot (Apr 25 2025 at 19:02):

alexcrichton commented on PR #10651:

Oh ignore me, I forgot that was already added!

Last updated: Dec 06 2025 at 07:03 UTC