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

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

Closed
svermeulen opened this issue Apr 4, 2022 · 8 comments
Closed
Labels
feature request New feature or request semantics Unexpected or unsound behaviors

Comments

@svermeulen
Copy link
Contributor

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
Copy link
Member

hishamhm commented Apr 7, 2022

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
Copy link
Contributor

lenscas commented Apr 7, 2022

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
Copy link
Contributor Author

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 hishamhm added feature request New feature or request semantics Unexpected or unsound behaviors labels Aug 9, 2022
@hishamhm
Copy link
Member

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
Copy link
Contributor Author

Brilliant. Thank you!

@hishamhm
Copy link
Member

hishamhm commented Nov 12, 2022 via email

@svermeulen
Copy link
Contributor Author

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: #582

@hishamhm
Copy link
Member

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: #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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request semantics Unexpected or unsound behaviors
Projects
None yet
Development

No branches or pull requests

3 participants