c-api: Module definitions in component model linker

[MangoPeachGrape]

See previous comment #10598 (comment).
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

[alexcrichton]

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 #10598 (comment) 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)

[MangoPeachGrape]

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

[alexcrichton]

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

With tests + docs looks good to me 👍

[MangoPeachGrape]

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

What's next, values?

[alexcrichton]

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

[MangoPeachGrape]

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

[alexcrichton]

Oh ignore me, I forgot that was already added!