rhaiscript / lsp

Language server for Rhai.
Apache License 2.0
43 stars 4 forks source link

Properly handle function signatures #82

Open schungx opened 2 years ago

schungx commented 2 years ago

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 commented 2 years ago

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:

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.

schungx commented 2 years ago

I am now aware why some language designers hate overloading.

Probably why Rust doesn't have it!

schungx commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.