teal-language / tl

The compiler for Teal, a typed dialect of Lua
MIT License
2.11k stars 109 forks source link

Cannot create generic methods that return arrays of the generic type #512

Closed svermeulen closed 1 year ago

svermeulen commented 2 years ago

This builds:

function get_foo<T>():T
   return nil
end

local foo:integer = get_foo()
print(foo)

However, the following does not:

function get_foos<T>():{T}
   return {}
end

local foos:{integer} = get_foos()
print(foos)

With error: in local declaration: foos: got {<invalid type>}, expected {integer}

Is this a known limitation or is there a better way to handle this case?

For now I'm doing this:

function get_foos():{any}
   return {}
end

local foos = get_foos() as {integer}
print(foos)
hishamhm commented 2 years ago

Is this a known limitation or is there a better way to handle this case?

Yes, it is a known issue at this time, because generics for a function are currently resolved only on their arguments, not on return types. We'd need a smarter bidirectional type inference to do that.

We've had another issue related to inference of generics but I couldn't find it quickly now, and the conclusion I'm getting to is that it's probably best to allow the user to specify the type values explicitly instead of trying to be too smart in inference — that is, for the case you describede, instead of local foos: {integer} = get_foos(), to support something like local foos = get_foos<integer>() (notwithstanding the syntax ambiguity issues...)

lenscas commented 2 years ago

Maybe we can copy Rust's syntax for specifying generics (local foos = get_foos::<integer>()) ?

Though, for teal local foos = geet_foos:<integer>() is probably nicer looking, as : already has a meaning.

svermeulen commented 2 years ago

This could be unrelated, but on the topic of type inference, it would be really neat if we could do things like this:

local function perform_action(action:function(integer):string):string
   local result = action(12)
   print(result)
end

perform_action(function(value) return tostring(value + 1) end)

That is - if teal could infer the entire type values for a function given as an inline argument to a function.

Currently you have to do this:

perform_action(function(value:integer):string return tostring(value + 1) end)
hishamhm commented 1 year ago

I have improved flow checking of function return values. The example in this issue now type checks:

local function get_foos<T>():{T}
   return {}
end

local foos:{integer} = get_foos()
print(foos)

Teal is now able to make {integer} flow into the return value of the get_foos() function call and use it to resolve T.

Thank you for the report, @svermeulen !

svermeulen commented 1 year ago

Brilliant. Thank you!

hishamhm commented 1 year ago

This involved some pretty serious surgery to the function call checking implementation! Please give it a spin on your codebase and let me know if the code in the latest master has caused any regressions!

svermeulen commented 1 year ago

Ok I updated teal to master and did a type check of the 344 tl files in my project. It found a bunch of new errors, all of which were valid (which means that the previous version I had missed those somehow) but there was one in particular that seems like a possible regression. I've reported it here: https://github.com/teal-language/tl/issues/582

hishamhm commented 1 year ago

It found a bunch of new errors, all of which were valid (which means that the previous version I had missed those somehow)

Good! That means I'm making the type checker smarter! By any chance do you have a way to isolate any of those errors that are now caught? I'd love to add them to the test suite, if they're new patterns I don't happen to have covered in tests (I didn't manage to get many new tests in).

but there was one in particular that seems like a possible regression. I've reported it here: https://github.com/teal-language/tl/issues/582

Yes, it's definitely a regression. I have a general idea of what's going on, I think it should be possible to get to a fix with your testcase in #582. Thanks again!