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

Function signatures be dropped out from recursive modules within higher order functor #5818

Closed
vicuna opened this issue Nov 10, 2012 · 10 comments

Comments

@vicuna
Copy link

vicuna commented Nov 10, 2012

Original bug ID: 5818
Reporter: tianyicui
Assigned to: @lpw25
Status: acknowledged (set by @damiendoligez on 2013-01-04T14:23:17Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.00.1
Target version: undecided
Category: typing
Tags: recmod
Related to: #4134
Monitored by: @gasche @alainfrisch

Bug description

The background can be seen from the stackoverflow question (http://stackoverflow.com/questions/13304044/implementing-okasakis-bootstrapped-heaps-in-ocaml-why-doesnt-it-compile), below is a reduced example, in which, it seems the signature of HigherOrderFunctor.Base and HigherOrderFunctor2.Base are the same, but the later will not compile.

module type ORDERED =
sig
  type t
  val compare : t -> t -> int
end

module type CARRY = sig
  module M : ORDERED
end

(* works *)
module HigherOrderFunctor
  (Make : functor (X : ORDERED) -> (CARRY with module M = X))
= struct
  module rec Base
    : (ORDERED with type t = string)
    = String
  and Other
    : (CARRY with module M = Base)
    = Make(Base)
end

(* does not work *)
module HigherOrderFunctor2
  (Make : functor (X : ORDERED) -> (CARRY with module M = X))
= struct
  module rec Base
    : sig
      (* 'compare' seems dropped from this signature *)
      type t = string
      val compare : t -> t -> int
    end
    = String
  and Other
    : (CARRY with module M = Base)
    = Make(Base)
end

File attachments

@vicuna
Copy link
Author

vicuna commented Jul 17, 2014

Comment author: @mshinwell

This bug was discussed. It is suspected that it's a shortcoming in the type checker, but that there should be work-arounds. May be re-visited for a future release.

@vicuna
Copy link
Author

vicuna commented Jul 18, 2014

Comment author: @lpw25

The issue here is that inclusion checks can fail spuriously when there are module type approximations in the environment. The attached patch fixes this by weakening inclusion checks whenever the environment contains approximations. This is safe because the checks will be run again later with the correct types, and inclusion checks don't generate any information required in the rest of type-checking.

@vicuna
Copy link
Author

vicuna commented Jul 21, 2014

Comment author: @alainfrisch

Note that Env.t has now a flags field which is a bit mask (this is currently only in the 4.02 branch, not integrated in trunk yet). This could be used to avoid adding an extra field for contains_approx.

@vicuna
Copy link
Author

vicuna commented Jul 27, 2014

Comment author: @lpw25

I think this patch also fixes #4134

@vicuna
Copy link
Author

vicuna commented Mar 15, 2017

Comment author: @garrigue

@lpw25: If you're sure that your fix doesn't break existing programs, you may go foward.
On recursive modules, I must admit that I'm not competent enough to do any kind of review.

@yallop
Copy link
Member

yallop commented Apr 5, 2019

Both HigherOrderFunctor and HigherOrderFunctor2 are rejected as of 4.08 (as a result of #1626, I think):

Line 8, characters 29-33:
8 |     : (CARRY with module M = Base)
                                 ^^^^
Error: Illegal recursive module reference

@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.

@github-actions github-actions bot added the Stale label May 15, 2020
@garrigue
Copy link
Contributor

@lpw25 Are we still waiting for a PR with the attached patch?

@lpw25
Copy link
Contributor

lpw25 commented May 15, 2020

My old patch allows ill-formed types and paths to be formed temporarily. I'm not a huge fan of that. Since I don't think people hit these cases very often, and recursive modules are incomplete in all kinds of ways, we can probably just leave this. If more people starting running into such problems it would be easy to resurrect the patch.

@github-actions github-actions bot removed the Stale label Jun 22, 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.

@github-actions github-actions bot added the Stale label Jul 21, 2021
@trefis trefis closed this as completed Jul 21, 2021
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

6 participants