Fix Recursive Modules on ocaml < 4.13

[mlasson]

Description

The CI of gen_js_api is failing for version of ocaml < 4.13 on some tests involving recursive module.

This can basically be reproduced by running on ocaml 4.12.1:

module rec Even : sig val is: int -> bool end = struct
  let is n =
    match n with
    | 0 -> true
    | 1 -> false
    | n -> Odd.is (n - 1)

end and Odd : sig val is: int -> bool end = struct
      let is n =
        match n with
        | 0 -> false
        | 1 -> true
        | n -> Even.is (n - 1)
    end

let () =
  Jsoo_runtime.Js.debugger ();
  assert (not (Even.is 1))

The "Even.is" is not even a function (as shown by a debugger); it is an object with a single fun property and it is not callable.
Since PR #1389, caml_call_gen is no longer checking for the presence of ".fun".

Fix Proposal

We could use caml_alloc_dummy_infix to allocate the dummy function.

[hhugo]

I try to keep the directory tests-ocaml as close as possible to the testsuite/tests directory from ocaml repo.

Can you move the test in compiler/tests-jsoo/test_rec_mod.ml ?

I'll try to document that the purpose of the various test directories

[hhugo]

Running the test in compiler/tests-jsoo/test_rec_mod.ml
We might want this additional fix

 //Provides: caml_update_dummy
 function caml_update_dummy (x, y) {
-  if( typeof y==="function" ) { x.fun = y; return 0; }
   if( y.fun ) { x.fun = y.fun; return 0; }
+  if( typeof y==="function" ) { x.fun = y; return 0; }
   var i = y.length; while (i--) x[i] = y[i]; return 0;
 }

[hhugo]

LGMT, I left small comments.
Thanks you very much for your contribution

[hhugo]

Here is a commit allowing to run some test with ocaml 4.12.
7dc2ae6

There are already showing the issue you're fixing. Fill tree to include this commit in your branch.

 |let%expect_test _ =
 |  (try print_int (M1.f 3) with e -> print_endline @@ Printexc.to_string e);
-|  [%expect {| 2 |}];
+|  [%expect {|
+|    function(){
+|                var
+|                 extra_args = arguments.length == 0 ? 1 : arguments.length,
+|                 nargs = new Array(args.length + extra_args);
+|                for(var i = 0; i < args.length; i++) nargs[i] = args[i];
+|                for(var i = 0; i < arguments.length; i++)
+|                 nargs[args.length + i] = arguments[i];
+|                return caml_call_gen(f, nargs);
+|               } |}];
-|  (try print_int (M1.g 3) with e -> print_endline @@ Printexc.to_string e);
-|  [%expect {| 2 |}];
+|  (try print_int (M1.g 3) with e -> print_endline @@ Printexc.to_string e);
+|  [%expect {|
+|    function(){
+|                var
+|                 extra_args = arguments.length == 0 ? 1 : arguments.length,
+|                 nargs = new Array(args.length + extra_args);
+|                for(var i = 0; i < args.length; i++) nargs[i] = args[i];
+|                for(var i = 0; i < arguments.length; i++)
+|                 nargs[args.length + i] = arguments[i];
+|                return caml_call_gen(f, nargs);
+|               } |}];
-|  (try print_int (M1.h 3) with e -> print_endline @@ Printexc.to_string e);
-|  [%expect {| 2 |}]
+|  (try print_int (M1.h 3) with e -> print_endline @@ Printexc.to_string e);
+|  [%expect {|
+|    function(){
+|                var
+|                 extra_args = arguments.length == 0 ? 1 : arguments.length,
+|                 nargs = new Array(args.length + extra_args);
+|                for(var i = 0; i < args.length; i++) nargs[i] = args[i];
+|                for(var i = 0; i < arguments.length; i++)
+|                 nargs[args.length + i] = arguments[i];
+|                return caml_call_gen(f, nargs);
+|               } |}]
 |
 |module rec Odd : sig
 |  val odd : int -> bool
 |end = struct
 |  let odd x = if x = 0 then false else Even.even (pred x)
 |end
 |
 |and Even : sig
 |  val even : int -> bool
 |end = struct
 |  let even x = if x = 0 then true else Odd.odd (pred x)
 |end
 |
 |let%expect_test _ =
 |  Printf.printf "%b" (Even.even 1000);
 |  [%expect {| true |}];
 |  Printf.printf "%b" (Odd.odd 1000);
-|  [%expect {| false |}]
+|  [%expect {| true |}]

[mlasson]

I still have:

--- a/_build/default/compiler/tests-jsoo/test_rec_mod.ml
+++ b/_build/.sandbox/0a6bc1e217d6e38b1adcf391b7816958/default/compiler/tests-jsoo/test_rec_mod.ml.corrected
@@ -59,11 +59,11 @@ end

 let%expect_test _ =
   (try print_int (M1.f 3) with e -> print_endline @@ Printexc.to_string e);
-  [%expect {| 2 |}];
+  [%expect {| File "compiler/tests-jsoo/test_rec_mod.ml", line 52, characters 6-12: Undefined recursive module |}];
   (try print_int (M1.g 3) with e -> print_endline @@ Printexc.to_string e);
-  [%expect {| 2 |}];
+  [%expect {| File "compiler/tests-jsoo/test_rec_mod.ml", line 52, characters 6-12: Undefined recursive module |}];
   (try print_int (M1.h 3) with e -> print_endline @@ Printexc.to_string e);
-  [%expect {| 2 |}]
+  [%expect {| File "compiler/tests-jsoo/test_rec_mod.ml", line 52, characters 6-12: Undefined recursive module |}]

But I guess this is expected on 4.12, right ?

[hhugo]

I still have:

--- a/_build/default/compiler/tests-jsoo/test_rec_mod.ml
+++ b/_build/.sandbox/0a6bc1e217d6e38b1adcf391b7816958/default/compiler/tests-jsoo/test_rec_mod.ml.corrected
@@ -59,11 +59,11 @@ end

 let%expect_test _ =
   (try print_int (M1.f 3) with e -> print_endline @@ Printexc.to_string e);
-  [%expect {| 2 |}];
+  [%expect {| File "compiler/tests-jsoo/test_rec_mod.ml", line 52, characters 6-12: Undefined recursive module |}];
   (try print_int (M1.g 3) with e -> print_endline @@ Printexc.to_string e);
-  [%expect {| 2 |}];
+  [%expect {| File "compiler/tests-jsoo/test_rec_mod.ml", line 52, characters 6-12: Undefined recursive module |}];
   (try print_int (M1.h 3) with e -> print_endline @@ Printexc.to_string e);
-  [%expect {| 2 |}]
+  [%expect {| File "compiler/tests-jsoo/test_rec_mod.ml", line 52, characters 6-12: Undefined recursive module |}]

But I guess this is expected on 4.12, right ?

Yes, this is expected.

[hhugo]

Merged, thanks again

[mlasson]

Thank you !