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

Minor simplifications to typechecker #7450

Merged
merged 5 commits into from
Jan 8, 2025
Merged

Conversation

ayazhafiz
Copy link
Member

@ayazhafiz ayazhafiz commented Jan 1, 2025

Landing some simplifications ahead of a change to make named type variables where things are known to be a concrete type illegal.

See individual commits for details.

@ayazhafiz ayazhafiz changed the title Bugfix association of inferred-tag-extension variables Minor simplifications to typechecker Jan 2, 2025
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here are correct. The old version was not. foo is unconditionally called in bar.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here are correct. Because map is in a nested scope, it should not be generalized. Therefore the assignment of the lambdaset to f is valid.

@lukewilliamboswell
Copy link
Collaborator

@ayazhafiz -- something may have gone wrong with your GPG signature?

These are inferred vars, not rigids.
Remove branches on determining how let-bindings are introduced to the
scope. This is maybe a little more inefficient, but I think it is a huge
simplification.

One additional change this required was changing how fx suffixes are
checked. The current implementation would add additional constraints for
patterns in let bindings conditionally. However, this is unnecessary. I
believe it is sufficient to check the fx suffix by running the checks on
all introduced symbols after the type is well known (i.e. the body is
checked).
This is actually correct - the rigid approach is not. Lambda set
variables should be inferred in-scope.
I don't think this assert is actually accurate.
@ayazhafiz
Copy link
Member Author

Okay, resolved

@lukewilliamboswell lukewilliamboswell merged commit 99dfc55 into main Jan 8, 2025
16 of 19 checks passed
@lukewilliamboswell lukewilliamboswell deleted the ayaz/bugfix-ts branch January 8, 2025 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants