Fix import of signatures with mutually recursive types

[thierry-martinez]

Import of signatures with mutually recursive types was broken.

If a signature with mutually recursive types was defined in a.ml:

module type S = sig
  type t = A of u
  and u = B of t
end

then the compilation of the following compilation unit b.ml fails:

module type S = [%import: (module A.S)]

The reason is that the following pattern matching branch was broken in two ways:

| (Sig_type (_, _, Trec_first) | _) :: _ when trec <> [] ->

1. _ is a catch all, so it matches in particular Sig_type with rec_flag other than Trec_first, i.e. the left hand side branch of pattern disjunction is useless;

2. trec should be appended to the result in the final case of the empty list (tsig = []).

[ejgallego]

Should a test-case be added?

[whitequark]

Ideally, yes.

[gasche]

This is a very nice catch, but I would like the new code to be more readable than your proposal. Before reading your patch, I had no idea that | _ -> true -> was valid OCaml code. Some ideas to make things more readable:

1. Avoid when, which is generally a tool to make code less readable.
2. More ambitious change, have a recursive function psig_of_type_block or psig_of_type_next dedicated to parsing recursive blocks of type declarations, mutually-recursive with psig_of_tsig. This lets you remove the optional paramteer of psig_of_tsig, which also contributes in making this code more complex than it could be.

[thierry-martinez]

@ejgallego Test-case added, thanks!

@gasche Thanks for your suggestion! Indeed, it could be better written: I was misguided by the idea of making as little change as possible. I did not follow your suggestion of two mutually recursive functions as I did not know how not to repeat the call to ptype_decl_of_ttype_decl twice (one time for Trec_first and another time for the other items). I think that my proposal of gathering mutually recursive definitions before transforming them is quite natural at least.