-
Notifications
You must be signed in to change notification settings - Fork 463
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
Coerce polyvariant to variant #6981
Conversation
@@ -3701,6 +3701,14 @@ let rec subtype_rec env trace t1 t2 cstrs = | |||
with Exit -> | |||
(trace, t1, t2, !univar_pairs)::cstrs | |||
end | |||
| (Tvariant {row_closed=true; row_fields}, Tconstr (_, [], _)) | |||
when extract_concrete_typedecl_opt env t2 |> Variant_coercion.type_is_variant -> |
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.
I wonder if there's a better way to do this. I had to check that it's an actual variant, or the case under would be blocked, which broke a coercion test.
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.
@cristianoc anything else we should check for here? Private?
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.
Private does not change the internal representation, so I would not bother.
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.
Looks good so far
e1ddefd
to
40a31f1
Compare
40a31f1
to
0eb44ef
Compare
@cristianoc ready for review. |
@@ -3701,6 +3701,14 @@ let rec subtype_rec env trace t1 t2 cstrs = | |||
with Exit -> | |||
(trace, t1, t2, !univar_pairs)::cstrs | |||
end | |||
| (Tvariant {row_closed=true; row_fields}, Tconstr (_, [], _)) | |||
when extract_concrete_typedecl_opt env t2 |> Variant_coercion.type_is_variant -> |
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.
Private does not change the internal representation, so I would not bother.
This makes it possible to coerce from polyvariant to variant when we can guarantee the runtime representations will match.