Navigation Menu

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

Unification error with type aliases of conjunctive polymorphic variants #7781

Closed
vicuna opened this issue Apr 18, 2018 · 4 comments
Closed
Assignees

Comments

@vicuna
Copy link

vicuna commented Apr 18, 2018

Original bug ID: 7781
Reporter: @lpw25
Assigned to: @garrigue
Status: confirmed (set by @garrigue on 2018-04-18T14:44:25Z)
Resolution: open
Priority: normal
Severity: minor
Version: 4.06.1
Target version: later
Category: typing
Monitored by: @nojb @gasche

Bug description

The following code does not type check:

type 'a t = [< `A of int & string ] as 'a

let f (x : _ t) =
  match x with
  | `A _ -> ()

Characters 84-88:
  | `A _ -> ()
    ^^^^
Error: This pattern matches values of type [? `A of int ]
       but a pattern was expected which matches values of type
         [< `A of int & string ] t
       Types for tag `A are incompatible

However, adding what should be a spurious type annotation makes it work:

let f (x : _ t) =
  match (x : [< `A of int & string]) with
  | `A _ -> ()

val f : [< `A of int & string & 'a ] t -> unit = <fun>

The issue here is that partial copies, as used when typing pattern matching, remove the details of polymorphic variants but do not expand aliases. So in the second case we have the expected type:

[> ]

when type checking the pattern, whilst in the original case we have:

[> ] t

which unfortunately still expands out to:

[< `A of int & string ]

Of course conjunctive polymorphic variants don't matter much, so this is hardly urgent, but it is disappointing when the syntax used to express a type affects the outcome of type-checking.

@vicuna
Copy link
Author

vicuna commented Apr 18, 2018

Comment author: @garrigue

The second behavior (no error) is supposed to be the correct one.
Looks like an expansion is missing somewhere.

@vicuna
Copy link
Author

vicuna commented Apr 20, 2018

Comment author: @garrigue

Actually, having a completely symmetrical behavior would require expanding all type abbreviations whose formal argument contains a Reither beforehand.
This is rather complicated to do, as one cannot expand an abbreviation during copying (at least I think so).

So for the time being the answer is that, if an Reither is hidden by an abbreviation, one may have unexpected failures, but this can be fixed using a type annotation (positive view!)

Note of course that in most cases this is not a function you want to use anyway (it doesn't accept any input), so one could even say it's a feature :-) Unfortunately this is clearly not principal.

@github-actions
Copy link

github-actions bot commented May 7, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 7, 2020
@lpw25 lpw25 removed the Stale label May 7, 2020
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants