Skip to content
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

feat(runtime-core) Allow dynamic signature for polymorphic host functions #1225

Merged

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Feb 17, 2020

Extracted from #1018.

⚠️ This PR targets #1217, not master! ⚠️


This patch adds a new field in Func: signature. It contains the
signature of the host function.

For non-polymorphic host functions, the signature is computed from the
Args and Rets implementation parameters at compile-time.

For polymorphic host functions though, to be fully dynamic, the
signature given to new_polymorphic is used in Func as the correct
signature.

The improvement is the following:

- Func::<i32, i32, _>::new_polymorphic(…)
+ Func::new_polymorphic(…)

with this approach, we are more dynamic, and thus more easily embeddable in dynamic languages such as Python, Ruby, PHP and so on.

…ions.

This patch adds a new field in `Func`: `signature`. It contains the
signature of the host function.

For non-polymorphic host functions, the signature is computed from the
`Args` and `Rets` implementation parameters at compile-time.

For polymorphic host functions though, to be fully dynamic, the
signature given to `new_polymorphic` is used in `Func` as the correct
signature.
Copy link
Contributor

@losfair losfair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the type parameters Args and Rets on Func, and instead place them on Func::new? Seems that with this PR we don't need the static type information any more?

@Hywan
Copy link
Contributor Author

Hywan commented Feb 17, 2020

I'm afraid it will cause a regression. Let me think about it.

@Hywan
Copy link
Contributor Author

Hywan commented Feb 19, 2020

Yeah, in our doc, we have code like let add_one: Func<u32, u32> = instance.func("add_one")?;. This code will be broken.

@losfair
Copy link
Contributor

losfair commented Feb 19, 2020

Yeah this is a breaking change.

Can we add something like ErasedFunc that provides a type-erased Func then (and maybe implement From<Func<...>> for ErasedFunc to keep the current API unchanged)? Having type parameters that do not always match the actual function type sounds confusing to me.

@Hywan
Copy link
Contributor Author

Hywan commented Feb 19, 2020

It does also sound confusing to me.
The ErasedFunc idea is good. I'll update the patch.

@losfair
Copy link
Contributor

losfair commented Feb 24, 2020

Merging this first, will continue in #1217 .

@losfair losfair merged commit 644755f into wasmerio:feature/polymorphic-v2 Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-deprecated About the deprecated crates 🧪 tests I love tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants