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

Properly handle function signatures #82

Open
schungx opened this issue Aug 8, 2022 · 9 comments
Open

Properly handle function signatures #82

schungx opened this issue Aug 8, 2022 · 9 comments
Labels
A-HIR Area: HIR E-Hard P-Medium Priority: Medium T-Types Related to the type system

Comments

@schungx
Copy link
Collaborator

schungx commented Aug 8, 2022

Right now, functions are recognized seemingly randomly.

For example:

image

It seems to just show a random choice of function that has the appropriate name. I believe arity should also be checked, which should restrict the list of choices significantly.

@tamasfe
Copy link
Member

tamasfe commented Aug 8, 2022

I am now aware why some language designers hate overloading.

The proper way to handle different function signatures will probably involve multiple phases for type-checking:

  • First we try to create a signature at the call site with the known types (e.g. for every call like fn_name("hi", 3) we deduce a signature such as fn_name(String, int)).
  • Then look for function definitions that:
    1. have the same name and an exact type signature match
    2. have the same name and amount of parameters
    3. have the same name

The main issue is that we cannot construct call-site signatures without knowing the types but we cannot know the types without constructing and matching signatures, so type inference will probably need to be a lot more complex than it is now.

I will probably look into how typescript does this but last time I checked that codebase it was not a great learning material.

Also this is pretty much #63. Closing that in favor of this.

@tamasfe tamasfe changed the title Function arity Properly handle function signatures Aug 8, 2022
@tamasfe tamasfe added E-Hard A-HIR Area: HIR P-Medium Priority: Medium T-Types Related to the type system labels Aug 8, 2022
@schungx
Copy link
Collaborator Author

schungx commented Aug 8, 2022

I am now aware why some language designers hate overloading.

Probably why Rust doesn't have it!

@schungx
Copy link
Collaborator Author

schungx commented Aug 8, 2022

Then look for function definitions that:

1. have the same name and an exact type signature match
2. have the same name and amount of parameters
3. have the same name

I believe you can use simply include arity as part of the function name (e.g. function foo(x, y) will be foo#2). So essentially, you're only handling 2, by-passing 1 and 3.

3 is not really required as, in Rhai, a function will never match something with the wrong number of parameters, so it really is not necessary to search for just the name alone.

In the actual function resolution code, Rhai at least hashes the namespace, function name and number of parameters together. There is no hash that contains only the function name, so always these three things are searched together. Parameter types, however, are added later on, as script-defined functions can take any parameter types.

1 is not needed for script-defined functions because every parameter has a known type: Dynamic.

@schungx
Copy link
Collaborator Author

schungx commented Aug 8, 2022

I believe you can use simply include arity as part of the function name (e.g. function foo(x, y) will be foo#2). So essentially, you're only handling 2, by-passing 1 and 3.

Come to think of it... I hope we haven't re-invented function name mangling!

@tamasfe
Copy link
Member

tamasfe commented Aug 8, 2022

1 is not needed for script-defined functions because every parameter has a known type: Dynamic.

Unknown types ? will match any other type so that's not an issue, script-defined functions have parameters that are all of unknown (same as Dynamic) type. We will eventually need to match function definitions to script-defined functions though, this is currently not done.

3 is not really required as, in Rhai, a function will never match something with the wrong number of parameters

That's great at runtime when you want fast efficient lookups, however here we need to show errors and hints if a function name is found but the signatures do not match instead of just not finding the function. I don't think hashing functions will help with any of the issues here.

@schungx
Copy link
Collaborator Author

schungx commented Aug 8, 2022

We will eventually need to match function definitions to script-defined functions though, this is currently not done.

Well, scripted functions always "trump" Rust functions, so if 2 already gives you a match, you won't need to search for 1.

we need to show errors and hints if a function name is found but the signatures do not match

Ah, I understand your point now.

@tamasfe
Copy link
Member

tamasfe commented Aug 8, 2022

however here we need to show errors and hints

This is the reason for the 3 filters during lookups I mentioned, if the type signature is good, everything is fine, if the types are different, we need to signal with a type error for the specific parameters, and at last it's yet a different error if the function does not even get the number of parameters we expect.

Well, scripted functions always "trump" Rust functions

I'm thinking of documenting rhai script code by creating foo.rhai and foo.d.rhai where the latter contains type definitions for functions in the former.

@schungx
Copy link
Collaborator Author

schungx commented Aug 8, 2022

I'm thinking of documenting rhai script code by creating foo.rhai and foo.d.rhai where the latter contains type definitions for functions in the former.

Well, I believe script code should be self-contained as much as possible. Users dislike having to constantly write definition files, so they should be required for native functions only as much as possible...

If there is a feature missing for script functions, I can modify Rhai to add it in.

@tamasfe
Copy link
Member

tamasfe commented Aug 8, 2022

This is partly because we don't have inline type hints, but typescript even does this for overloads:

// Type definitions.
function foo(a: number): number;
function foo(a: number, b: number): number;

// The implementation.
function foo(a, b) {
  if (typeof b !== "undefined") {
    return b;
  }
  return a;
}

If there is a feature missing for script functions, I can modify Rhai to add it in.

We'd need inline type hints but I'd personally push back on this as much as possible, I don't have the capacity to thoroughly design something like this and getting something half-baked will only cause pain down the line (e.g. potential parsing performance issues, ambiguities, bloat). I'd rather not touch the Rhai implementation unless we absolutely need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-HIR Area: HIR E-Hard P-Medium Priority: Medium T-Types Related to the type system
Projects
None yet
Development

No branches or pull requests

2 participants