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

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

Merged
merged 4 commits into from
Aug 31, 2024

Conversation

spotandjake
Copy link
Member

@spotandjake spotandjake commented Aug 29, 2024

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

spotandjake commented Aug 29, 2024

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

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

@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.

@ospencer ospencer added this pull request to the merge queue Aug 31, 2024
@spotandjake spotandjake removed this pull request from the merge queue due to a manual request Aug 31, 2024
@spotandjake
Copy link
Member Author

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

@ospencer ospencer changed the title fix(compiler): Correct type approximation on recursive functions. fix(compiler): Correct type approximation on recursive functions Aug 31, 2024
@ospencer ospencer added this pull request to the merge queue Aug 31, 2024
Merged via the queue into grain-lang:main with commit b0fb040 Aug 31, 2024
12 checks passed
@github-actions github-actions bot mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default argument in recursive functions crashes compiler
2 participants