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

Object type in recursive module's `with` annotation raises Ctype.Unify(_) #7082

Closed
vicuna opened this issue Dec 10, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Dec 10, 2015

Original bug ID: 7082
Reporter: labichn
Assigned to: @garrigue
Status: closed (set by @xavierleroy on 2017-02-16T14:18:10Z)
Resolution: fixed
Priority: normal
Severity: crash
Platform: x86_64
OS: GNU/Linux
OS Version: 4.2.5-1
Version: 4.02.3
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: typing

Bug description

See steps to reproduce for a minimal example.

The compiler throws a Ctype.Unify(_) exception when a recursive module A's with annotation specializes an abstract type to an object type containing a field of some type from a module B (mutually recursive with, but not necessarily referencing A).

Syntactic expansion (wrapping A's signature with sig include ... end) is a workaround, but shouldn't be necessary.

The error occurs with ocamlc versions 3.11.2, 4.02.3, and today's 4.03.0+trunk.

Steps to reproduce

cat bug.ml
module type FOO = sig type t end
module type BAR =
sig
(* Works: module rec A : (sig include FOO with type t = < b:B.t > end) *)
module rec A : (FOO with type t = < b:B.t >)
and B : FOO
end
ocamlc bug.ml
Fatal error: exception Ctype.Unify(_)

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 11, 2015

Comment author: @alainfrisch

Confirmed with trunk as of today.

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 11, 2015

Comment author: @alainfrisch

I've learned that such bugs (#6981, #6513) are fixed by adding some "Ctype.init_def(Ident.current_time())", so I've given it a try. See attached patch which fixes the problem, but I've no idea if it's correct.

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 12, 2015

Comment author: @garrigue

Good idea.
But this is not a very natural place to add this init_def: usually one does it after creating new ids.
So here the natural place would be inside transl_recmodule_modtypes, just after the creation of the new ids. Cf my alternative patch. The result is identical.

I'm starting to think that we should have a more uniform way to do that...

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 14, 2015

Comment author: @garrigue

Fixed in trunk, commit 7bcfced.

@vicuna vicuna closed this Feb 16, 2017

@vicuna vicuna added the typing label Mar 14, 2019

@vicuna vicuna added this to the 4.03.0 milestone Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

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.