-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ungate start functions, and restrict args/returns. #297
Ungate start functions, and restrict args/returns. #297
Conversation
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 |
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. |
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? |
Ok, I've now filed #300 to just remove invariant 3, and we can consider/prioritize the rest separately. |
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.
f6bca8b
to
0a5de06
Compare
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. |
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.