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

ImportFunction: use FunctionType internally #607

Open
evacchi opened this issue Oct 24, 2024 · 2 comments
Open

ImportFunction: use FunctionType internally #607

evacchi opened this issue Oct 24, 2024 · 2 comments
Assignees

Comments

@evacchi
Copy link
Collaborator

evacchi commented Oct 24, 2024

ImportFunction currently takes a list of param types and a list of return types.
However, there exist a FunctionType object that represents exactly these lists; it would be natural for ImportFunction to reuse this object internally; then expose a .functionType() member

It might probably still make sense for HostFunction (at least) to expose an overloaded constructor:

new HostFunction(module, name, paramTypes, returnTypes, func)

to reduce typing for explicit construction; otherwise it will always look like:

new HostFunction(module, name, new FunctionType(paramTypes, returnTypes), func)
@evacchi evacchi self-assigned this Oct 24, 2024
@electrum
Copy link
Collaborator

electrum commented Nov 7, 2024

Related, why do we have both HostFunction and ImportFunction? It seems that we could replace all usages of HostFunction with ImportFunction and remove it.

@evacchi
Copy link
Collaborator Author

evacchi commented Nov 8, 2024

the initial idea was that the user-facing API could be unrelated to the internal API, and possibly that the naming is clearer.

The original problem was that everything was called a Host* something even when something did not originated from the host; indeed ImportFunction is less confusing, but on the other hand is less intentional (our end users will always define host functions). It also pairs well with the @HostModule concept.

I am not strongly against the idea of dropping it, if we think it's clear enough. Maybe we can take a look at the naming in other runtimes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants