ocaml-wasm / wasm_of_ocaml

Other
251 stars 9 forks source link

Get wasm_of_ocaml up-to-date with js_of_ocaml #47

Open vouillon opened 4 months ago

vouillon commented 4 months ago

We have a branch jsoo-tip which contains the changes we make to the Js_of_ocaml code in compiler/lib, and a branch jsoo-merge which contains some changes to the compiler/lib/wasm code to adapt to API changes in Js_of_ocaml.

OlivierNicole commented 4 months ago

Currently after applying the process above, there are only two tests failing with OCaml 5.2.0:

vouillon commented 4 months ago

Regarding compiler/tests-full/stdlib.cma.expected.js, we should keep the version from jsoo-tip. I think that the file is not up to date in jsoo-tip and that the commit "Distinguish float arrays' should be amended to fix that.

By the way, the commit "Source_map_io API changes" should also be amended with these changes:

diff --git a/compiler/lib/source_map_io.unsupported.ml b/compiler/lib/source_map
_io.unsupported.ml
index dbdb35816c..2d4ecf1cc6 100644
--- a/compiler/lib/source_map_io.unsupported.ml
+++ b/compiler/lib/source_map_io.unsupported.ml
@@ -23,6 +23,8 @@ let to_string _ = fail ()

 let of_string _ = fail ()

-let to_file _ _ = fail ()
+let of_file_no_mappings _ = fail ()
+
+let to_file ?mappings:_ _ ~file:_ = fail ()

 let enabled = false
OlivierNicole commented 4 months ago

Regarding compiler/tests-full/stdlib.cma.expected.js, we should keep the version from jsoo-tip. I think that the file is not up to date in jsoo-tip and that the commit "Distinguish float arrays' should be amended to fix that.

Done, it works!

By the way, the commit "Source_map_io API changes" should also be amended with these changes:

diff --git a/compiler/lib/source_map_io.unsupported.ml b/compiler/lib/source_map
_io.unsupported.ml
index dbdb35816c..2d4ecf1cc6 100644
--- a/compiler/lib/source_map_io.unsupported.ml
+++ b/compiler/lib/source_map_io.unsupported.ml
@@ -23,6 +23,8 @@ let to_string _ = fail ()

 let of_string _ = fail ()

-let to_file _ _ = fail ()
+let of_file_no_mappings _ = fail ()
+
+let to_file ?mappings:_ _ ~file:_ = fail ()

 let enabled = false

Done.

vouillon commented 4 months ago

The marshaling test pass for me when producing JavaScript.

It fails with dune runtest --profile wasm (after implementing two missing primitives). I think that's because Marshal.header_size has changed in OCaml 5.1.0, so we need to adjust somehow the global $caml_marshal_header_size in runtime/wasm/marshal.wat depending on the OCaml version.

OlivierNicole commented 4 months ago

The marshaling test pass for me when producing JavaScript.

Not for me. This is completely obscure to me.

OlivierNicole commented 4 months ago

Status report: I rebased jsoo-tip onto JSOO master. Since then, test compiler/tests-toplevel/test_toplevel.js has been failing with --profile dev after merging jsoo-tip into jsoo-merge. I haven’t yet found the reason but maybe related to ocsigen/js_of_ocaml#1634?

I have also pushed missing primitives to jsoo-merge, as well as cherry-picked ocsigen/js_of_ocaml#1639. Primitives for the new BLAKE2B digests are still missing, as well as caml_zstd_initialize.

hhugo commented 4 months ago

https://github.com/ocsigen/js_of_ocaml/pull/1634 adds a new test that checks that toplevel built with separate compilation works correctly. Is it possible that such setup has never worked with the wasm backend ?

OlivierNicole commented 4 months ago

Hm, what is strange is that it is test_toplevel.js (so, the whole-compilation one IIUC) that has an uncaught exception, and it is with JS and not Wasm.

OlivierNicole commented 4 months ago

The bulk of the changes needed on the Js_of_ocaml side has been submitted as PRs there, there shouldn’t be many more except to fix problems.

vouillon commented 4 months ago

Hm, what is strange is that it is test_toplevel.js (so, the whole-compilation one IIUC) that has an uncaught exception, and it is with JS and not Wasm.

There is this error:

Error: The external function "caml_float_of_int" is not available

So, it seems Generate.init is not called (it defines caml_float_of_int as an alias to %identity).

This issue is already in branch jsoo-tip, by the way.

vouillon commented 4 months ago

So, it seems Generate.init is not called (it defines caml_float_of_int as an alias to %identity).

We can add it in Driver.from_string or in lib-dynlink/js_of_ocaml_compiler_dynlink.ml. Not sure which is best...

Oh, and the merged file test_toplevel/dune seems to miss some changes. Maybe we need to cherry-pick 390e6a2833b89dac7b7e6fee42c071ded9fb1f5a on branch jsoo-merge?

hhugo commented 4 months ago

We can add it in Driver.from_string or in lib-dynlink/js_of_ocaml_compiler_dynlink.ml. Not sure which is best...

Maybe in Js_of_ocaml_compiler_dynlink.ml is best