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

generalize internal representation of generic types #890

Merged
merged 7 commits into from
Jan 3, 2025

Conversation

hishamhm
Copy link
Member

@hishamhm hishamhm commented Jan 2, 2025

All types that have type variables are now represented as a GenericType record, which holds a non-generic Type and an array of type arguments.

This change is because originally we only cared about generic records and generic functions, but once we have the local type MyType<T> = ... syntax, other types can be generic as well (in particular, unions).

Instead of replicating generic support logic in the implementation of each type, we factor it out into a type-level term which encapsulates the application of type variables, which is something more like second-order lambda calculus.

(See https://en.wikipedia.org/wiki/System_F and the ensuing rabbit hole.)

Fixes #880.
Fixes #787.

@hishamhm hishamhm added the generics model Generics: issues caused by the old internals, implementation/regressions of the new internals label Jan 2, 2025
Copy link

github-actions bot commented Jan 2, 2025

Teal Playground URL: https://890--teal-playground-preview.netlify.app

@hishamhm
Copy link
Member Author

hishamhm commented Jan 2, 2025

I just saw two issues already when running the LuaRocks codebase through it:

Error 1) modules that return a generic type

./luarocks/search.tl:16:55: missing type arguments in generic<K> table.SortFunction<K> | Ordering<K>

https://github.com/luarocks/luarocks/blob/master/src/luarocks/search.tl#L16

Error 2) and a stricter resolution of type arguments (which I need to look closer, but might be a legitimate error that it has now caught)

./luarocks/cmd/write_rockspec.tl:415:81: argument 3: got Ordering<string>, expected table.SortFunction<number | string> | Ordering<number | string>

https://github.com/luarocks/luarocks/blob/master/src/luarocks/search.tl#L16

@hishamhm hishamhm force-pushed the next-generic-type branch 2 times, most recently from aaceca7 to 8cdb640 Compare January 3, 2025 15:14
@hishamhm
Copy link
Member Author

hishamhm commented Jan 3, 2025

Error 1 there was being misreported (it was being reported at the wrong location). It was a legitimate error, a missing <K>. Fixed here.

Error 2 is a bit trickier because it is a type check that we would like to accept (because we know that Ordering<string> is a valid Ordering<string | number>, but only because we know how the Ordering object works), but we can't, because the compiler has no way to tell that values of the type variable's type are only used for reading and not for writing... typical covariance/contravariance limitation. Currently, type variables are invariant, for simplicity. The easy fix is to change the field to Ordering<string | number> in the LuaRocks codebase.

hishamhm added a commit to luarocks/luarocks that referenced this pull request Jan 3, 2025
hishamhm added a commit to luarocks/luarocks that referenced this pull request Jan 3, 2025
hishamhm added a commit to luarocks/luarocks that referenced this pull request Jan 3, 2025
hishamhm added a commit to luarocks/luarocks that referenced this pull request Jan 3, 2025
All types that have type variables are now represented as
a GenericType record, which holds a non-generic Type and
an array of type arguments.

This change is because originally we only cared about
generic records and generic functions, but once we have
the `local type MyType<T> = ...` syntax, other types
can be generic as well (in particular, unions).

Instead of replicating generic support logic in the
implementation of each type, we factor it out into a
type-level term which encapsulates the application of
type variables, which is something more like second-order
lambda calculus.

See https://en.wikipedia.org/wiki/System_F and the ensuing
rabbit hole.
@hishamhm hishamhm merged commit 8d9a26f into master Jan 3, 2025
8 checks passed
@hishamhm hishamhm deleted the next-generic-type branch January 3, 2025 16:31
@hishamhm
Copy link
Member Author

hishamhm commented Jan 3, 2025

All right, I decided to do the brave thing and merge this already.

The fixes contained here are important enough so that it's clear that this should be the baseline moving forward. If this has introduced any further regressions, then let's fix it in the main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generics model Generics: issues caused by the old internals, implementation/regressions of the new internals
Projects
None yet
1 participant