Per module inference context + substitute type variables

[gertvv]

For issue #6.

This is a draft PR. The compiler is currently broken at codegen stage (something to do with tuples and/or closures) but is able to complete type inference, lowering, and monomorphization for itself. I will try to identify the issue later, but pointers are very welcome!

This implements your suggestion to move resolving type references into typed_ast, and core has been simplified as a result. I think there is probably a small performance penalty to the current implementation. Once it is full working I have additional refactoring in mind that should resolve this.

Resolving globals now cares whether they're public, so I had to update some of the patch files.

[gertvv]

The offending snippet is generated from gig/graph.strongly_connected_components:

  list.fold(l, [], fn(a, i) { assign(r, a, i, i) })
  |> list.chunk(fn(i) { i.1 })
  |> list.map(list.map(_, fn(x: #(a, a)) { x.0 }))

The closures for the anonymous functions list.map(_, fn(x: #(a, a)) { x.0}) look like this:

List_String Closure_380(List_Tuple2_String_String T0) {
  Closure T0;
  T0 = create_function(Closure_379);
  return gleam_list_map_Tuple2_String_String_String(T0, T0);
}

String Closure_379(Tuple2_String_String x) {
  return x.field0;
}

I don't yet see the connection between my changes and the aliasing that's occurring between the list argument and the nested closure.

Update: I pushed a fix for this that works but that I don't fully understand. It seems like it may have been working previously because temp_uid only started at 0 for the prelude and then ticked up. Starting it at 1 for every module prevents the clash.

[schurhammer]

I think you were going a bit in the wrong direction with the TypeVarId.
It's a bit hard to explain but I've pushed what is the right way to do it imo.

[gertvv]

I think you were going a bit in the wrong direction with the TypeVarId. It's a bit hard to explain but I've pushed what is the right way to do it imo.

My idea was to make sure type variable ID assignment doesn't need to depend on what modules have been checked before it (so that type inference is not order-dependent beyond what is implied by the import graph). Maybe your version also achieves that? I've only had a quick look.

[gertvv]

Can you explain why it works to just reset the type_uid counter to 0 for each module? It seems like they should clash...

[schurhammer]

Can you explain why it works to just reset the type_uid counter to 0 for each module? It seems like they should clash...

This is because types from other modules are generalised and instantiated.
e.g. whenever list.map is used somewhere, we don't just keep the original type variables. Instead, we replace the type variables (poly.vars) with "fresh" ones.
This is what allows functions to be polymorphic since without instantiation all list.map in the program would refer to the same type variable and therefore have to operate on the exact same type.
It also avoids the clashes, since the IDs from other modules get replaced before interacting with the current module.

[gertvv]

This is because types from other modules are generalised and instantiated. e.g. whenever list.map is used somewhere, we don't just keep the original type variables. Instead, we replace the type variables (poly.vars) with "fresh" ones.

Oh, right! Apparently forgot about that part from when I implemented this myself... To be honest I found the whole process difficult to keep in my head even then. There were some errors due to types from other modules while I was working through the refactor that set me down the wrong path.

My remaining open question is: does type variable substitution need to be done to the "annotations" as well?

[schurhammer]

does type variable substitution need to be done to the "annotations" as well?

I'm not sure but probably yes

[schurhammer]

Looks good but lets add Annotation to the substitute functions

[gertvv]

Sure, I can do that.