Skip to content

Fix making the static import for the dynamic import of external ffi #6664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

# 11.1.0-rc.4 (Unreleased)

#### :bug: Bug Fix

- Fix making the static import for the dynamic import of external ffi https://github.com/rescript-lang/rescript-compiler/pull/6664

#### :nail-care: Polish
- Omit `undefined` in external function calls for trailing optional arguments when not supplied. https://github.com/rescript-lang/rescript-compiler/pull/6653

Expand Down
6 changes: 3 additions & 3 deletions jscomp/core/lam.ml
Original file line number Diff line number Diff line change
Expand Up @@ -788,13 +788,13 @@ let rec transform_uncurried_arg_type loc (arg_types : External_arg_spec.params)
| ([], [] | _ :: _, [] | [], _ :: _) as ok -> ok

let handle_bs_non_obj_ffi (arg_types : External_arg_spec.params)
(result_type : External_ffi_types.return_wrapper) ffi args loc prim_name =
(result_type : External_ffi_types.return_wrapper) ffi args loc prim_name ~dynamic_import =
if no_auto_uncurried_arg_types arg_types then
result_wrap loc result_type
(prim ~primitive:(Pjs_call { prim_name; arg_types; ffi }) ~args loc)
(prim ~primitive:(Pjs_call { prim_name; arg_types; ffi; dynamic_import }) ~args loc)
else
let n_arg_types, n_args = transform_uncurried_arg_type loc arg_types args in
result_wrap loc result_type
(prim
~primitive:(Pjs_call { prim_name; arg_types = n_arg_types; ffi })
~primitive:(Pjs_call { prim_name; arg_types = n_arg_types; ffi; dynamic_import })
~args:n_args loc)
1 change: 1 addition & 0 deletions jscomp/core/lam.mli
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ val handle_bs_non_obj_ffi :
t list ->
Location.t ->
string ->
dynamic_import: bool ->
t

(**************************************************************)
Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/lam_compile_env.ml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ let reset () =
be escaped quite ugly
*)
let add_js_module ?import_attributes (hint_name : External_ffi_types.module_bind_name)
(module_name : string) default : Ident.t =
(module_name : string) default ~dynamic_import : Ident.t =
let id =
Ident.create
(match hint_name with
Expand All @@ -71,7 +71,7 @@ let add_js_module ?import_attributes (hint_name : External_ffi_types.module_bind
| Phint_nothing -> Ext_modulename.js_id_name_of_hint_name module_name)
in
let lam_module_ident : J.module_id =
{ id; kind = External { name = module_name; default; import_attributes }; dynamic_import = false }
{ id; kind = External { name = module_name; default; import_attributes }; dynamic_import }
in
match Lam_module_ident.Hash.find_key_opt cached_tbl lam_module_ident with
| None ->
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/lam_compile_env.mli
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
val reset : unit -> unit

val add_js_module :
?import_attributes:External_ffi_types.import_attributes -> External_ffi_types.module_bind_name -> string -> bool -> Ident.t
?import_attributes:External_ffi_types.import_attributes -> External_ffi_types.module_bind_name -> string -> bool -> dynamic_import:bool -> Ident.t
(**
[add_js_module hint_name module_name]
Given a js module name and hint name, assign an id to it
Expand Down
29 changes: 15 additions & 14 deletions jscomp/core/lam_compile_external_call.ml
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ let splice_obj_apply obj name args =
bundle *)

let external_var
({ bundle; module_bind_name; import_attributes } : External_ffi_types.external_module_name) =
let id = Lam_compile_env.add_js_module ?import_attributes module_bind_name bundle false in
({ bundle; module_bind_name; import_attributes } : External_ffi_types.external_module_name) ~dynamic_import =
let id = Lam_compile_env.add_js_module ?import_attributes module_bind_name bundle false ~dynamic_import in
E.external_var ?import_attributes ~external_name:bundle id

(* let handle_external_opt
Expand Down Expand Up @@ -216,21 +216,22 @@ let assemble_args_has_splice (arg_types : specs) (args : exprs) :

let translate_scoped_module_val
(module_name : External_ffi_types.external_module_name option) (fn : string)
(scopes : string list) =
(scopes : string list)
~dynamic_import =
match module_name with
| Some { bundle; module_bind_name; import_attributes } -> (
match scopes with
| [] ->
let default = fn = "default" in
let id =
Lam_compile_env.add_js_module ?import_attributes module_bind_name bundle default
Lam_compile_env.add_js_module ?import_attributes module_bind_name bundle default ~dynamic_import
in
E.external_var_field ?import_attributes ~external_name:bundle ~field:fn ~default id
| x :: rest ->
(* TODO: what happens when scope contains "default" ?*)
let default = false in
let id =
Lam_compile_env.add_js_module ?import_attributes module_bind_name bundle default
Lam_compile_env.add_js_module ?import_attributes module_bind_name bundle default ~dynamic_import
in
let start =
E.external_var_field ?import_attributes ~external_name:bundle ~field:x ~default id
Expand All @@ -250,10 +251,10 @@ let translate_scoped_access scopes obj =
| x :: xs -> Ext_list.fold_left xs (E.dot obj x) E.dot

let translate_ffi (cxt : Lam_compile_context.t) arg_types
(ffi : External_ffi_types.external_spec) (args : J.expression list) =
(ffi : External_ffi_types.external_spec) (args : J.expression list) ~dynamic_import =
match ffi with
| Js_call { external_module_name; name; splice: _; scopes; tagged_template = true } ->
let fn = translate_scoped_module_val external_module_name name scopes in
let fn = translate_scoped_module_val external_module_name name scopes ~dynamic_import in
(match args with
| [ {expression_desc = Array (strings, _); _}; {expression_desc = Array (values, _); _} ] ->
E.tagged_template fn strings values
Expand All @@ -262,7 +263,7 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types
(if dynamic then splice_apply fn args
else E.call ~info:{ arity = Full; call_info = Call_na } fn args))
| Js_call { external_module_name = module_name; name = fn; splice; scopes; tagged_template = false } ->
let fn = translate_scoped_module_val module_name fn scopes in
let fn = translate_scoped_module_val module_name fn scopes ~dynamic_import in
if splice then
let args, eff, dynamic = assemble_args_has_splice arg_types args in
add_eff eff
Expand All @@ -283,7 +284,7 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types
add_eff eff
@@ E.call ~info:{ arity = Full; call_info = Call_na } fn args
| Js_module_as_fn { external_module_name; splice } ->
let fn = external_var external_module_name in
let fn = external_var external_module_name ~dynamic_import in
if splice then
let args, eff, dynamic = assemble_args_has_splice arg_types args in
(* TODO: fix in rest calling convention *)
Expand Down Expand Up @@ -313,14 +314,14 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types
in
if splice then
let args, eff, dynamic = assemble_args_has_splice arg_types args in
let fn = translate_scoped_module_val module_name fn scopes in
let fn = translate_scoped_module_val module_name fn scopes ~dynamic_import in
add_eff eff
(mark ();
if dynamic then splice_new_apply fn args
else E.new_ fn args)
else
let args, eff = assemble_args_no_splice arg_types args in
let fn = translate_scoped_module_val module_name fn scopes in
let fn = translate_scoped_module_val module_name fn scopes ~dynamic_import in
add_eff eff
(mark (); E.new_ fn args)
| Js_send { splice; name; js_send_scopes } -> (
Expand All @@ -346,16 +347,16 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types
~info:{ arity = Full; call_info = Call_na }
(E.dot self name) args)
| _ -> assert false)
| Js_module_as_var module_name -> external_var module_name
| Js_module_as_var module_name -> external_var module_name ~dynamic_import
| Js_var { name; external_module_name; scopes } ->
(* TODO #11
1. check args -- error checking
2. support [@@scope "window"]
we need know whether we should call [add_js_module] or not
*)
translate_scoped_module_val external_module_name name scopes
translate_scoped_module_val external_module_name name scopes ~dynamic_import
| Js_module_as_class module_name ->
let fn = external_var module_name in
let fn = external_var module_name ~dynamic_import in
let args, eff = assemble_args_no_splice arg_types args in
(* TODO: fix in rest calling convention *)
add_eff eff
Expand Down
1 change: 1 addition & 0 deletions jscomp/core/lam_compile_external_call.mli
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ val translate_ffi :
External_arg_spec.params ->
External_ffi_types.external_spec ->
J.expression list ->
dynamic_import:bool ->
J.expression

(** TODO: document supported attributes
Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/lam_compile_primitive.ml
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,8 @@ let translate output_prefix loc (cxt : Lam_compile_context.t)
(* Lam_compile_external_call.translate loc cxt prim args *)
(* Test if the argument is a block or an immediate integer *)
| Pjs_object_create _ -> assert false
| Pjs_call { arg_types; ffi } ->
Lam_compile_external_call.translate_ffi cxt arg_types ffi args
| Pjs_call { arg_types; ffi; dynamic_import } ->
Lam_compile_external_call.translate_ffi cxt arg_types ffi args ~dynamic_import
(* FIXME, this can be removed later *)
| Pisint -> E.is_type_number (Ext_list.singleton_exn args)
| Pis_poly_var_block -> E.is_type_object (Ext_list.singleton_exn args)
Expand Down
6 changes: 3 additions & 3 deletions jscomp/core/lam_convert.ml
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ let convert (exports : Set_ident.t) (lam : Lambda.lambda) :
let may_depends = Lam_module_ident.Hash_set.create 0 in

let rec convert_ccall (a_prim : Primitive.description)
(args : Lambda.lambda list) loc : Lam.t =
(args : Lambda.lambda list) loc ~dynamic_import : Lam.t =
let prim_name = a_prim.prim_name in
let prim_name_len = String.length prim_name in
match External_ffi_types.from_string a_prim.prim_native_name with
Expand All @@ -400,7 +400,7 @@ let convert (exports : Set_ident.t) (lam : Lambda.lambda) :
| Param_number i -> Ext_list.init i (fun _ -> External_arg_spec.dummy)
in
let args = Ext_list.map args convert_aux in
Lam.handle_bs_non_obj_ffi arg_types result_type ffi args loc prim_name
Lam.handle_bs_non_obj_ffi arg_types result_type ffi args loc prim_name ~dynamic_import
| Ffi_inline_const i -> Lam.const i
and convert_js_primitive (p : Primitive.description)
(args : Lambda.lambda list) loc : Lam.t =
Expand Down Expand Up @@ -542,7 +542,7 @@ let convert (exports : Set_ident.t) (lam : Lambda.lambda) :
convert_pipe f x outer_loc
| Lprim (Prevapply, _, _) -> assert false
| Lprim (Pdirapply, _, _) -> assert false
| Lprim (Pccall a, args, loc) -> convert_ccall a args loc
| Lprim (Pccall a, args, loc) -> convert_ccall a args loc ~dynamic_import
| Lprim (Pgetglobal id, args, _) ->
let args = Ext_list.map args convert_aux in
if Ident.is_predef_exn id then
Expand Down
5 changes: 3 additions & 2 deletions jscomp/core/lam_primitive.ml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type t =
prim_name : string;
arg_types : External_arg_spec.params;
ffi : External_ffi_types.external_spec;
dynamic_import: bool;
}
| Pjs_object_create of External_arg_spec.obj_params
(* Exceptions *)
Expand Down Expand Up @@ -249,11 +250,11 @@ let eq_primitive_approx (lhs : t) (rhs : t) =
i0 = i1 && flag0 = flag1 && eq_tag_info info0 info1
| _ -> false)
| Pduprecord -> rhs = Pduprecord
| Pjs_call { prim_name; arg_types; ffi } -> (
| Pjs_call { prim_name; arg_types; ffi; dynamic_import } -> (
match rhs with
| Pjs_call rhs ->
prim_name = rhs.prim_name && arg_types = rhs.arg_types
&& ffi = rhs.ffi
&& ffi = rhs.ffi && dynamic_import = rhs.dynamic_import
| _ -> false)
| Pjs_object_create obj_create -> (
match rhs with
Expand Down
1 change: 1 addition & 0 deletions jscomp/core/lam_primitive.mli
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type t =
prim_name : string;
arg_types : External_arg_spec.params;
ffi : External_ffi_types.external_spec;
dynamic_import: bool;
}
| Pjs_object_create of External_arg_spec.obj_params
| Praise
Expand Down
4 changes: 3 additions & 1 deletion jscomp/test/build.ninja

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions jscomp/test/import2.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions jscomp/test/import2.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
let a = Js.import(Import_external.makeA)
let b = Import_external.makeA
10 changes: 10 additions & 0 deletions jscomp/test/import_external.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions jscomp/test/import_external.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@module("a")
external makeA: string = "default"

let f8 = Js.import(makeA)