-
-
Notifications
You must be signed in to change notification settings - Fork 647
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
Constrained monomorphs #9549
Constrained monomorphs #9549
Conversation
the "Constraint check failure for test.T" is missing, but that's a separate problem. I think it gets eaten by the BetterErrors transformation.
# Conflicts: # src/context/typecore.ml # src/typing/calls.ml
# Conflicts: # src/context/typecore.ml # src/typing/calls.ml
| TAnon _ when List.mem MOpenStructure m.tm_constraints -> | ||
(* If we assign an open structure monomorph to another structure, the semantics want us to merge the | ||
fields. This is kinda weird, but that's how it has always worked. *) | ||
constrain_to_type m None t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also check if anon's fields don't conflict with current constraints here?
I didn't find such a check down the constraint_to_type
calls path, and Monomorph.bind
is called from various places other than unification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think a check is missing here. I think we can do this in classify_constraints
, where we're building the fields map anyway. This should avoid any problems with too early unification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question for other branches of this match
monos | ||
|
||
let safe_mono_close ctx m p = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return value of this function is never used.
It should be changed to unit
probably. And Monomorph.close
too.
I 100% agree. Let's merge this and deal with the issues should they arise. |
I'll deal with field constraint conflicts later. That requires some proper handling to avoid getting |
This PR adds support for constraints on monomorphs by introducing the
tm_constraints
field ontmono
. When a monomorph is bound viaMonomorph.bind
, the compiler checks if the binding is valid incheck_constraints
.There are currently two sources of constraints:
spawn_constrained_monos
, which creates the monomorph and adds the constraints from type parameters, if exists.MOpenStructure
flag, a structural constraint for that field is added to the monomorph.Furthermore, if we have an unbound monomorph after typing a function, that monomorph is
Monomorph.close
d by using its constraints to find a type.Some information about related issues:
For now, my goal here is to "not break anything". Afterwards, we can look into what further possibilities this opens. This includes:
@:multiType
Opened
flag for structuresI'm not 100% confident in the changes made here, but we have to start somewhere.