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

Unexpected GADT error #7618

Closed
vicuna opened this issue Sep 1, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Sep 1, 2017

Original bug ID: 7618
Reporter: @trefis
Assigned to: @garrigue
Status: resolved (set by @garrigue on 2017-09-12T23:34:09Z)
Resolution: fixed
Priority: normal
Severity: minor
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: typing
Child of: #7617
Monitored by: @gasche

Bug description

Currently the following code is rejected by the typechecker:

# type _ t = I : int t
type _ t = I : int t
# let f (type a) (x : a t) (y : int) =
    match x, y with
    | I, (_:a) -> ()
  ;;
Error: This pattern matches values of type a t * a
       but a pattern was expected which matches values of type a t * int
       Type a is not compatible with type int

While this one is accepted:

# let g (type a) (x : a t) (y : a) =
    match x, y with
    | I, (0:int) -> ()
  ;;
val g : 'a t -> 'a -> unit

I believe that both of these function suffer from the issue raised in #7617,
i.e. the typechecker allows ambiguous types to escape, arbitrarily choosing one
of the instances as the type of the pattern (or rather, always choosing the
locally abstract type).

[f] later fails to typecheck because we try to unify the type of the pattern
with the type of the argument outside the scope of the equation introduced by
[I].
This happens on line 3887 of typecore.ml, with the call to
[unify_pats (instance env ty_arg)].

Once #7617 is fixed, everything else being unchanged, these functions should
both be rejected.
However Leo suggests they could be accepted that the typechecker if we decreased
the sharing between the type of the scrutiny and the expected type of each
pattern (eg. by using generalize_structure).

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 5, 2017

Comment author: @garrigue

Leo is right. The fix should be the same as 7617.

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 12, 2017

Comment author: @garrigue

Fixed in trunk by commits 19b37dc and 5c174df.
Do not lower the level when unifying branches of pattern-matching together, and use local environment for unification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.