From 8fd639e6a00f148363d9709e556560daea9c0ce1 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Fri, 16 Feb 2024 19:49:51 -0800 Subject: [PATCH 1/7] test: add failing test --- test/blackbox-tests/virtual-lib-compilation.t | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 test/blackbox-tests/virtual-lib-compilation.t diff --git a/test/blackbox-tests/virtual-lib-compilation.t b/test/blackbox-tests/virtual-lib-compilation.t new file mode 100644 index 000000000..f6fe16c84 --- /dev/null +++ b/test/blackbox-tests/virtual-lib-compilation.t @@ -0,0 +1,57 @@ + + $ . ./setup.sh + + $ cat > dune-project < (lang dune 3.9) + > (using melange 0.1) + > EOF + $ cat > dune < (melange.emit + > (target output) + > (modules x) + > (emit_stdlib false) + > (libraries vlib impl_melange)) + > EOF + + $ mkdir impl_melange vlib + + $ cat > vlib/dune < (library + > (name vlib) + > (wrapped false) + > (modes :standard melange) + > (virtual_modules virt)) + > EOF + $ cat > vlib/virt.mli < val t : string + > EOF + $ cat > vlib/vlib_impl.ml < let hello = "Hello from " ^ Virt.t + > EOF + + $ cat > impl_melange/dune < (library + > (name impl_melange) + > (modes melange) + > (implements vlib)) + > EOF + $ cat > impl_melange/virt.ml << EOF + > let t = "melange" + > EOF + + $ cat > x.ml < let () = print_endline Vlib_impl.hello + > EOF + + $ /Users/anmonteiro/projects/dune/_build/install/default/bin/dune build @melange --display=short + ocamldep vlib/.vlib.objs/virt.intf.d + ocamldep impl_melange/.impl_melange.objs/virt.impl.d + ocamldep vlib/.vlib.objs/vlib_impl.impl.d + melc vlib/.vlib.objs/melange/virt.{cmi,cmti} + melc vlib/.vlib.objs/melange/vlib_impl.{cmi,cmj,cmt} (exit 2) + File "vlib/vlib_impl.ml", line 1: + Error: Virt not found, it means either the module does not exist or it is a namespace + [1] + + $ output=_build/default/output/x.js + $ node "$output" From 4327a81c9bee3c39979af35707f4140f2407d825 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Thu, 15 Feb 2024 21:51:23 -0800 Subject: [PATCH 2/7] fix: handle case of missing `.cmj` when trying to inline external field --- jscomp/core/lam_compile.ml | 16 ++++++++++++++-- jscomp/core/mel_exception.mli | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/jscomp/core/lam_compile.ml b/jscomp/core/lam_compile.ml index a1df320a1..5bc4faf68 100644 --- a/jscomp/core/lam_compile.ml +++ b/jscomp/core/lam_compile.ml @@ -187,12 +187,24 @@ type initialization = J.block -: we should not do functor application inlining in a non-toplevel, it will explode code very quickly *) -let rec compile_external_field (* Like [List.empty]*) +let rec compile_external_field (* Like [List.empty] *) (lamba_cxt : Lam_compile_context.t) (id : Ident.t) name : Js_output.t = + (* NOTE(anmonteiro): This function checks whether there's inlining + information available for the lambda being compiled. + + The fallback case happens in 2 scenarios: + + 1. there's no inlining information available in the `.cmj` file + 2. there's no `.cmj` file available. This can happen if we're compiling + a dune virtual library where one of the modules uses a binding from + any of its virtual modules. Because we're programming against the + interface file at this point, we can emit the fallback expression + too. + *) match Lam_compile_env.query_external_id_info id name with | { persistent_closed_lambda = Some lam; _ } when Lam_util.not_function lam -> compile_lambda lamba_cxt lam - | _ -> + | _ | (exception Mel_exception.Error (Cmj_not_found _)) -> Js_output.output_of_expression lamba_cxt.continuation ~no_effects:no_effects_const (E.ml_var_dot id name) diff --git a/jscomp/core/mel_exception.mli b/jscomp/core/mel_exception.mli index fbc417e77..62a85dbb5 100644 --- a/jscomp/core/mel_exception.mli +++ b/jscomp/core/mel_exception.mli @@ -33,7 +33,7 @@ TODO: In the futrue, we should refine dependency [bsb] should not rely on such exception, it should have its own exception handling *) -(* exception Error of error *) +exception Error of error (* val report_error : Format.formatter -> error -> unit *) From 440e4dfebb3e93262977f5f7abc6dfc27ea089a3 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Thu, 15 Feb 2024 21:57:50 -0800 Subject: [PATCH 3/7] chore: add changelog entry --- Changes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Changes.md b/Changes.md index e24a52141..2cbf189b7 100644 --- a/Changes.md +++ b/Changes.md @@ -21,6 +21,11 @@ Unreleased going to be reassigned ([#1019](https://github.com/melange-re/melange/pull/1019), [#1059](https://github.com/melange-re/melange/pull/1059)). +- runtime: add minimal bindings for JS iterators + ([#1060](https://github.com/melange-re/melange/pull/1060)) +- core: handle missing `.cmj` when compiling dune virtual libraries + ([#1067](https://github.com/melange-re/melange/pull/1067), fixes + [#658](https://github.com/melange-re/melange/issues/658)) 3.0.0 2024-01-28 --------------- From e4e99d38f993a45b0553aa8498ee8f01b7c30120 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Fri, 16 Feb 2024 20:07:17 -0800 Subject: [PATCH 4/7] test: snapshot test after fix --- test/blackbox-tests/virtual-lib-compilation.t | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/blackbox-tests/virtual-lib-compilation.t b/test/blackbox-tests/virtual-lib-compilation.t index f6fe16c84..48f194441 100644 --- a/test/blackbox-tests/virtual-lib-compilation.t +++ b/test/blackbox-tests/virtual-lib-compilation.t @@ -1,6 +1,5 @@ $ . ./setup.sh - $ cat > dune-project < (lang dune 3.9) > (using melange 0.1) @@ -8,24 +7,25 @@ $ cat > dune < (melange.emit > (target output) + > (alias mel) > (modules x) > (emit_stdlib false) > (libraries vlib impl_melange)) > EOF $ mkdir impl_melange vlib - $ cat > vlib/dune < (library > (name vlib) > (wrapped false) - > (modes :standard melange) + > (modes melange) > (virtual_modules virt)) > EOF + $ cat > vlib/virt.mli < val t : string > EOF - $ cat > vlib/vlib_impl.ml < vlib/vlib.ml < let hello = "Hello from " ^ Virt.t > EOF @@ -35,23 +35,27 @@ > (modes melange) > (implements vlib)) > EOF + $ cat > impl_melange/virt.ml << EOF > let t = "melange" > EOF $ cat > x.ml < let () = print_endline Vlib_impl.hello + > let () = print_endline Vlib.hello > EOF - $ /Users/anmonteiro/projects/dune/_build/install/default/bin/dune build @melange --display=short + $ dune build @mel --display=short ocamldep vlib/.vlib.objs/virt.intf.d ocamldep impl_melange/.impl_melange.objs/virt.impl.d - ocamldep vlib/.vlib.objs/vlib_impl.impl.d + ocamldep vlib/.vlib.objs/vlib.impl.d melc vlib/.vlib.objs/melange/virt.{cmi,cmti} - melc vlib/.vlib.objs/melange/vlib_impl.{cmi,cmj,cmt} (exit 2) - File "vlib/vlib_impl.ml", line 1: - Error: Virt not found, it means either the module does not exist or it is a namespace - [1] + melc vlib/.vlib.objs/melange/vlib.{cmi,cmj,cmt} + melc impl_melange/.impl_melange.objs/melange/virt.{cmj,cmt} + melc output/vlib/vlib.js + melc .output.mobjs/melange/melange__X.{cmi,cmj,cmt} + melc output/impl_melange/virt.js + melc output/x.js $ output=_build/default/output/x.js $ node "$output" + Hello from melange From 7317746a06466a443d9f0e7aa97994517450fbfa Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Fri, 16 Feb 2024 21:05:44 -0800 Subject: [PATCH 5/7] fix: bail in more places when we can't perform cross-module inlining --- jscomp/core/lam_arity_analysis.ml | 13 +++++++------ jscomp/core/lam_compile.ml | 21 ++++++++++++++------- jscomp/core/lam_compile_env.ml | 7 ++++++- jscomp/core/lam_compile_env.mli | 10 +++++++--- jscomp/core/lam_pass_remove_alias.ml | 9 +++++++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/jscomp/core/lam_arity_analysis.ml b/jscomp/core/lam_arity_analysis.ml index 2d5be8c8e..dbd7a3e1e 100644 --- a/jscomp/core/lam_arity_analysis.ml +++ b/jscomp/core/lam_arity_analysis.ml @@ -48,9 +48,9 @@ let rec get_arity (meta : Lam_stats.t) (lam : Lam.t) : Lam_arity.t = args = [ Lglobal_module id ]; _; } -> ( - match (Lam_compile_env.query_external_id_info id name).arity with - | Single x -> x - | Submodule _ -> Lam_arity.na) + match Lam_compile_env.query_external_id_info id name with + | Some { arity = Single x; _ } -> x + | Some { arity = Submodule _; _ } | None -> Lam_arity.na) | Lprim { primitive = Pfield (m, _); @@ -65,9 +65,10 @@ let rec get_arity (meta : Lam_stats.t) (lam : Lam.t) : Lam_arity.t = ]; _; } -> ( - match (Lam_compile_env.query_external_id_info id name).arity with - | Submodule subs -> subs.(m) (* TODO: shall we store it as array?*) - | Single _ -> Lam_arity.na) + match Lam_compile_env.query_external_id_info id name with + | Some { arity = Submodule subs; _ } -> + subs.(m) (* TODO: shall we store it as array?*) + | Some { arity = Single _; _ } | None -> Lam_arity.na) (* TODO: all information except Pccall is complete, we could get more arity information *) diff --git a/jscomp/core/lam_compile.ml b/jscomp/core/lam_compile.ml index 5bc4faf68..328781c10 100644 --- a/jscomp/core/lam_compile.ml +++ b/jscomp/core/lam_compile.ml @@ -202,9 +202,10 @@ let rec compile_external_field (* Like [List.empty] *) too. *) match Lam_compile_env.query_external_id_info id name with - | { persistent_closed_lambda = Some lam; _ } when Lam_util.not_function lam -> + | Some { persistent_closed_lambda = Some lam; _ } + when Lam_util.not_function lam -> compile_lambda lamba_cxt lam - | _ | (exception Mel_exception.Error (Cmj_not_found _)) -> + | Some _ | None -> Js_output.output_of_expression lamba_cxt.continuation ~no_effects:no_effects_const (E.ml_var_dot id name) @@ -241,8 +242,9 @@ and compile_external_field_apply (appinfo : Lam.apply) (module_id : Ident.t) Lam_compile_env.query_external_id_info module_id field_name in let ap_args = appinfo.ap_args in - match ident_info.persistent_closed_lambda with - | Some (Lfunction { params; body; _ }) when List.same_length params ap_args -> + match ident_info with + | Some { persistent_closed_lambda = Some (Lfunction { params; body; _ }); _ } + when List.same_length params ap_args -> (* TODO: serialize it when exporting to save compile time *) let _, param_map = Lam_closure.is_closed_with_map Ident.Set.empty params body @@ -250,7 +252,12 @@ and compile_external_field_apply (appinfo : Lam.apply) (module_id : Ident.t) compile_lambda lambda_cxt (Lam_beta_reduce.propogate_beta_reduce_with_map lambda_cxt.meta param_map params body ap_args) - | _ -> + | Some _ | None -> + let arity = + ident_info + |> Option.map (fun (t : Js_cmj_format.keyed_cmj_value) -> t.arity) + |> Option.value ~default:Js_cmj_format.single_na + in let args_code, args = let dummy = ([], []) in if ap_args = [] then dummy @@ -265,13 +272,13 @@ and compile_external_field_apply (appinfo : Lam.apply) (module_id : Ident.t) ap_args ~init:dummy in - let fn = E.ml_var_dot module_id ident_info.name in + let fn = E.ml_var_dot module_id field_name in let expression = match appinfo.ap_info.ap_status with | (App_infer_full | App_uncurry) as ap_status -> E.call ~info:(call_info_of_ap_status ap_status) fn args | App_na -> ( - match ident_info.arity with + match arity with | Submodule _ | Single Arity_na -> E.call ~info:Js_call_info.dummy fn args | Single x -> diff --git a/jscomp/core/lam_compile_env.ml b/jscomp/core/lam_compile_env.ml index 8d1d385b9..991a2051e 100644 --- a/jscomp/core/lam_compile_env.ml +++ b/jscomp/core/lam_compile_env.ml @@ -82,7 +82,8 @@ let add_js_module (hint_name : Melange_ffi.External_ffi_types.module_bind_name) id | Some old_key -> old_key.id -let query_external_id_info (module_id : Ident.t) (name : string) : ident_info = +let query_external_id_info_exn (module_id : Ident.t) (name : string) : + ident_info = let oid = Lam_module_ident.of_ml module_id in let cmj_table = match Lam_module_ident.Hash.find_opt cached_tbl oid with @@ -95,6 +96,10 @@ let query_external_id_info (module_id : Ident.t) (name : string) : ident_info = in Js_cmj_format.query_by_name cmj_table name +let query_external_id_info module_id name = + try Some (query_external_id_info_exn module_id name) + with Mel_exception.Error (Cmj_not_found _) -> None + let get_dependency_info_from_cmj (id : Lam_module_ident.t) : Js_packages_info.t * Js_packages_info.file_case = let cmj_load_info = diff --git a/jscomp/core/lam_compile_env.mli b/jscomp/core/lam_compile_env.mli index 4ad45a9e0..b8ec67a01 100644 --- a/jscomp/core/lam_compile_env.mli +++ b/jscomp/core/lam_compile_env.mli @@ -59,11 +59,15 @@ val add_js_module : pay attention to for those modules are actually used or not *) -val query_external_id_info : Ident.t -> string -> Js_cmj_format.keyed_cmj_value +val query_external_id_info_exn : + Ident.t -> string -> Js_cmj_format.keyed_cmj_value (** [query_external_id_info id pos env found] - will raise if not found -*) + will raise if cmj not found *) + +val query_external_id_info : + Ident.t -> string -> Js_cmj_format.keyed_cmj_value option +(** [query_external_id_info id pos env found] *) val is_pure_module : Lam_module_ident.t -> bool diff --git a/jscomp/core/lam_pass_remove_alias.ml b/jscomp/core/lam_pass_remove_alias.ml index cd4e6e6b9..e557b4e60 100644 --- a/jscomp/core/lam_pass_remove_alias.ml +++ b/jscomp/core/lam_pass_remove_alias.ml @@ -133,7 +133,11 @@ let simplify_alias (meta : Lam_stats.t) (lam : Lam.t) : Lam.t = ap_info; } -> ( match Lam_compile_env.query_external_id_info ident fld_name with - | { persistent_closed_lambda = Some (Lfunction { params; body; _ }); _ } + | Some + { + persistent_closed_lambda = Some (Lfunction { params; body; _ }); + _; + } (* be more cautious when do cross module inlining *) when List.same_length params args && List.for_all @@ -146,7 +150,8 @@ let simplify_alias (meta : Lam_stats.t) (lam : Lam.t) : Lam.t = | _ -> true) args -> simpl (Lam_beta_reduce.propogate_beta_reduce meta params body args) - | _ -> Lam.apply (simpl l1) (List.map ~f:simpl args) ap_info) + | Some _ | None -> Lam.apply (simpl l1) (List.map ~f:simpl args) ap_info + ) (* Function inlining interact with other optimizations... - parameter attributes From 97bff6b35c55cb94763055e8b1540c0fcf3cbe74 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Sat, 17 Feb 2024 13:55:54 -0800 Subject: [PATCH 6/7] remove unused signature from the .mli --- jscomp/core/lam_compile_env.mli | 6 ------ 1 file changed, 6 deletions(-) diff --git a/jscomp/core/lam_compile_env.mli b/jscomp/core/lam_compile_env.mli index b8ec67a01..f9ac3a61a 100644 --- a/jscomp/core/lam_compile_env.mli +++ b/jscomp/core/lam_compile_env.mli @@ -59,12 +59,6 @@ val add_js_module : pay attention to for those modules are actually used or not *) -val query_external_id_info_exn : - Ident.t -> string -> Js_cmj_format.keyed_cmj_value -(** - [query_external_id_info id pos env found] - will raise if cmj not found *) - val query_external_id_info : Ident.t -> string -> Js_cmj_format.keyed_cmj_value option (** [query_external_id_info id pos env found] *) From f729758f22d0770fcaef06e0a581afa86c1d4962 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Sat, 17 Feb 2024 13:59:19 -0800 Subject: [PATCH 7/7] chore: move comment --- jscomp/core/lam_compile.ml | 12 ------------ jscomp/core/lam_compile_env.mli | 13 ++++++++++++- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/jscomp/core/lam_compile.ml b/jscomp/core/lam_compile.ml index 328781c10..5eb8e89e2 100644 --- a/jscomp/core/lam_compile.ml +++ b/jscomp/core/lam_compile.ml @@ -189,18 +189,6 @@ type initialization = J.block *) let rec compile_external_field (* Like [List.empty] *) (lamba_cxt : Lam_compile_context.t) (id : Ident.t) name : Js_output.t = - (* NOTE(anmonteiro): This function checks whether there's inlining - information available for the lambda being compiled. - - The fallback case happens in 2 scenarios: - - 1. there's no inlining information available in the `.cmj` file - 2. there's no `.cmj` file available. This can happen if we're compiling - a dune virtual library where one of the modules uses a binding from - any of its virtual modules. Because we're programming against the - interface file at this point, we can emit the fallback expression - too. - *) match Lam_compile_env.query_external_id_info id name with | Some { persistent_closed_lambda = Some lam; _ } when Lam_util.not_function lam -> diff --git a/jscomp/core/lam_compile_env.mli b/jscomp/core/lam_compile_env.mli index f9ac3a61a..a0e944b58 100644 --- a/jscomp/core/lam_compile_env.mli +++ b/jscomp/core/lam_compile_env.mli @@ -61,7 +61,18 @@ val add_js_module : val query_external_id_info : Ident.t -> string -> Js_cmj_format.keyed_cmj_value option -(** [query_external_id_info id pos env found] *) +(** [query_external_id_info module_id name] + + Note: This function checks whether there's inlining information available for + the lambda being compiled. + + The fallback case (deopt) happens in 2 scenarios: + + 1. there's no inlining information available in the `.cmj` file + 2. there's no `.cmj` file available. This can happen if we're compiling a + dune virtual library where one of the modules uses a binding from any of + its virtual modules. Because we're programming against the interface file + at this point, we must emit the deoptimized expression too. *) val is_pure_module : Lam_module_ident.t -> bool