Fix sum type declaration collision #1277
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shonfeder
force-pushed
the
1275/sum-type-collision
branch
2 times, most recently
from
c0815ea to
6731617
Compare
shonfeder
changed the title
shonfeder
force-pushed
the
1275/sum-type-collision
branch
from
3f666f9 to
607af5c
Compare
bugarela
reviewed
Nov 30, 2023
added 9 commits
November 30, 2023 09:48
Our parser and IR has the convention that the declaration def f(x) = y Gets represented as def f = (x) => y And in the IR, we expect both the representation of the declaration and the representation of the lambda to have the same source ID. We had not been following this convention in the generation of variant constructors, and that is corrected here.
Closes #1275 Since we don't do any backtracking in our constraint solving, the order in which the are solved is important. In particular, if we solve `isDefined` constraints before solving equality constraints, we can end up unifying a free variable with a variable in the first discovered defined type, rather than finding the most specific type that can unify with the given schema. This change defines an order over constraints, based on their kind, and uses that to sort the constraints before they are solved.
This reverts commit 3158e29. Apparently the example in the IR I was following is incorrect :)
This reverts commit dc4eb2a.
shonfeder
force-pushed
the
1275/sum-type-collision
branch
from
607af5c to
d95b299
Compare
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1277
The implementation of the
isDefinedconstraint was producing invalid type inferences in the presence of row types with the same labels coordinated with incompatible types.The problem was that the
isDefinedconstraint was being solved before other constraints that would have narrowed the type thru substitutions that would have prevented the ambiguity (e.g., by unifying thru the equality constraints induced by operator annotations). This fix takes the very simple course of introducing an ordering over constraints that placesisDefinedconstraints lasts, and sorts constraints prior to solving.A more robust solution should be considered if we encounter similar problems with constraint order again. Alternatives might include:
isDefinedconstraint.isDefinedusing it.Review by commit is probably helpful here, as some incidental cleanup is included and explained in the commit messages.
CHANGELOG.mdfor any new functionalityREADME.mdupdated for any listed functionality