WebAssembly / component-model

Repository for design and specification of the Component Model
Other
899 stars 75 forks source link

Ungate start functions, and restrict args/returns. #297

Closed sunfishcode closed 2 months ago

sunfishcode commented 5 months ago

Remove the restriction on component start functions calling imports. This allows start functions to run artbitrary user code.

And, adjust the 🪙 to include start functions in the MVP, and add a restriction that start functions cannot currently have arguments or return values, for now.

And add some words describing the order that start functions are called in.

lukewagner commented 5 months ago

This makes sense to me. Originally, I had thought the restriction enabled new kinds of AOT snapshotting and avoided some weird initialization-time circularities regarding parent/child initialization but, with the benefit of the intervening two years and tons more experience, I don't think it's actually buying us anything.

The only question is: independent of removing the trap, do we want to go ahead and add the component-level start function now? If Preview 2 implementations are happy to add it Real Soon, that sounds good to me.

sunfishcode commented 5 months ago

This is also aligning with existing implementation in Wasmtime, where it's already possible to have an inner core module with a core-wasm start function, which runs at instantiation time, and it can call imports at that time.

lukewagner commented 5 months ago

So for next steps: I think we're all agreed on simply deleting runtime invariant 3 (bringing the spec in-line with current implementations); if there was a PR with just that, I think we could merge it today. But it's still a question of whether we want or need to un-gate (zero-arity) component-level start functions. For example, maybe we should consider that a feature of 0.2.1. Also, iiuc, toolchain supports exists in various places to achieve the effect we need using just the core wasm start function (which is also nullary). So my suggestion is: maybe we scope this PR down to just deleting runtime invariant 3 and consider/prioritize component-level start functions separately?

sunfishcode commented 5 months ago

Ok, I've now filed https://github.com/WebAssembly/component-model/pull/300 to just remove invariant 3, and we can consider/prioritize the rest separately.

lukewagner commented 2 months ago

I think the criteria for this being ungated is the feature being implemented and included in some release, so I'll close this PR for now (since we don't have PRs open for all gated features). When it is time to ungate, we can reopen (and rebase, since #336 will add more cases) or just file a new PR since deleting emojis is easy.