Skip to content

False detection of self cycle error with an explicit module type #5368

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

Closed
cannorin opened this issue Apr 5, 2022 · 6 comments · Fixed by #5393
Closed

False detection of self cycle error with an explicit module type #5368

cannorin opened this issue Apr 5, 2022 · 6 comments · Fixed by #5393

Comments

@cannorin
Copy link
Contributor

cannorin commented Apr 5, 2022

Version: 9.1.4

Assume we have the following code of name b.res:

module Types : {
  module B : {
    type t = private int
  }
} = {
  module B = {
    type t = private int
  }
}

open Types

module B = {
  open B
  let t : t = Obj.magic(42)
}

This fails with a FAILED: B has a self cycle error despite that this does not contain any dependency to itself.
The open B actually refers to Types.B, but the compiler falsely detects it as a self-reference.

Interestingly, when you remove the explicit module type of the Types module, it compiles correctly:

module Types = {
  module B = {
    type t = private int
  }
}

open Types

module B = {
  open B
  let t : t = Obj.magic(42)
}
@cannorin
Copy link
Contributor Author

cannorin commented Apr 6, 2022

I believe this is a remnant of an old bug of ocamldep: ocaml/ocaml#4618

This seems to be considered by the community as one of the many limitations of ocamldep, as this and many other ocamldep bugs are marked either stale, suspended, or not fixable.

I think self-cycles should not be considered as fatal error, as the result reported from jscomp/ml/Depend.ml is unreliable. For example, dune just ignores any self-reference reported from ocamldep: https://github.com/ocaml/dune/blob/574f0f7de27c2391d660394e1dc72e9d70b0531e/boot/duneboot.ml#L875-L876

@cannorin
Copy link
Contributor Author

Can I open a PR to disable self-reference errors? I believe it's not very good to reject codes which should be valid. If a source file does indeed contain a self-reference, it would fail to typecheck & compile anyway.

@cristianoc
Copy link
Collaborator

Would you know where to look to check where this behaviour originates.
Notice the compiler has its own build system, so presumably it would be somewhere in there.

@cannorin
Copy link
Contributor Author

cannorin commented May 26, 2022

@cristianoc

@cristianoc
Copy link
Collaborator

Great!
If you know where the fix goes go ahead.

@cannorin
Copy link
Contributor Author

PR opened: #5393

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

Successfully merging a pull request may close this issue.

2 participants