Comment author: @gasche

My understanding of OCaml's recursive modules is that it looks for modules that are safe to compile even if the names it recursively depends on are not yet defined (they will not be required to evaluate the values exposed by the module itself). This criterion is approximated by the following, more restrictive one: of the modules evaluates only contain function values -- functions do not evaluate their body when they are evaluated.

The following code would therefore be accepted:

(* 1  *)   module rec M1 :
(* 2  *)   sig
(* 3  *)     module M2 :
(* 4  *)     sig
(* 5  *)       val v2 : unit -> unit
(* 6  *)     end
(* 7  *)   end = struct
(* 8  *)   let v1 () = M1.M2.v2 ()
(* 9  *)     module M2 =
(* 10 *)      struct
(* 11 *)        let v2 = fun () -> ()
(* 12 *)      end
(* 13 *)  end

It would be possible to relax this condition, but my experience is that OCaml language designers are very conservative about changes to recursive modules, that tend to make things more complex and more fragile. If you want to motivate a change on this front, you better have a highly motivating real-life example that it is clearly important to support.

(It's an interesting question to know whether Jeremy Yallop's new letrec-on-values check could be used to make the module checking more permissive. It's related to the comparison with related work in the module literature, so it is on our TODO-list.)

Comment author: kosik

That is interesting (and thus I am surprised a bit more after learning that the second snippet compiles while the former does not).

In any case, I am primarily suggesting making the error message more to the point.
(the current version is as useful as "I cannot compile your program. Guess why?")

Note that the error message looks like on 4.08:

File "./test_file.ml", line 7, characters 21-186:
 7 | .....................struct
 8 |     (* 8  *)   let v1 = M1.M2.v2
 9 |     (* 9  *)     module M2 =
10 |     (* 10 *)      struct
11 |     (* 11 *)        let v2 = ()
12 |     (* 12 *)      end
13 |     (* 13 *)  end
Error: Cannot safely evaluate the definition of the following cycle
       of recursively-defined modules: M1 -> M1.
       There are no safe modules in this cycle (see manual section 8.2).
File "./test_file.ml", line 5, characters 19-32:
5 |     (* 5  *)       val v2 : unit
                       ^^^^^^^^^^^^^
  Module M1 defines an unsafe value, v2 .

Which I believe is due to #2058.

I'll let @Octachron and @gasche decide if this is good enough to close this issue.

@Octachron will be too modest to declare his own work (improving the message) "good enough", so let me do it. It's good enough, and actually it's quite good given the restricted space budget we have.