search

grain-lang / grain

The Grain compiler toolchain and CLI. Home of the modern web staple. 🌾
https://grain-lang.org/
GNU Lesser General Public License v3.0
3.28k stars 115 forks source link

fix(compiler): Correct type approximation on recursive functions #2154

Closed spotandjake closed 2 months ago

spotandjake commented 2 months ago

This pr fixes #2151, we seem to have made an invalid assumption when creating the type here which was causing the crashes.

For anyone interested the root cause of the issue was in type_approx which is used when we are guessing types for recursive functions, we were producing the a TTyvar instead of an option(TTyvar) on default arguments.

spotandjake commented 2 months ago

I noticed the line in the image below as well which wasn't causing the problem but uses the same pattern, I'm not sure if this behaviour is incorrect at the moment but It would probably be a good idea to look into if we are handling things correctly there as well. image

Edit: Upon some furthur investigation it seems to make sense to apply this change elsewhere as generating a label newvar will cause problems in the Default case

ospencer commented 2 months ago

@spotandjake I looked into this and I don't love the solution here. Yes, default values are Option types, but I think this will just hide bugs. The Impossible case we're hitting is in fact possible, in this exact case (where the type of the function is not known beforehand).

Do you want to update that in this PR or close it and have me make the changes?

ospencer commented 2 months ago

@spotandjake and I chatted offline and decided that it's better to just handle it this way rather than handle this case in each of the other places it comes up.

spotandjake commented 2 months ago

I just added an additional test that tests for the invalid case in the original issue where the labels are missing.