From a133a38179b794cf1c0c8ed855e83bd4324f15f5 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 6 Apr 2024 08:39:14 +0200 Subject: [PATCH 1/6] Fix trailing undefined for optional parameters not omitted with @send --- jscomp/core/lam_compile_external_call.ml | 22 ++++++++++--------- ...it_trailing_undefined_in_external_calls.js | 4 +++- ...t_trailing_undefined_in_external_calls.res | 4 ++++ 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/jscomp/core/lam_compile_external_call.ml b/jscomp/core/lam_compile_external_call.ml index 9a7a92190e..23f6bfec77 100644 --- a/jscomp/core/lam_compile_external_call.ml +++ b/jscomp/core/lam_compile_external_call.ml @@ -250,6 +250,16 @@ let translate_scoped_access scopes obj = | [] -> obj | x :: xs -> Ext_list.fold_left xs (E.dot obj x) E.dot +let keep_non_undefined_args args arg_types = + let rec aux argsList (argTypes : specs) = + match (argsList, argTypes) with + | ( {J.expression_desc = Undefined {isUnit = false}; _} :: rest, + {External_arg_spec.arg_label = Arg_optional; _} :: argTypes ) -> + aux rest argTypes + | _ -> argsList + in + aux (List.rev args) (List.rev arg_types) |> List.rev + let translate_ffi (cxt : Lam_compile_context.t) arg_types (ffi : External_ffi_types.external_spec) (args : J.expression list) ~dynamic_import = match ffi with @@ -271,16 +281,7 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types else E.call ~info:{ arity = Full; call_info = Call_na } fn args) else let args, eff = assemble_args_no_splice arg_types args in - let rec keepNonUndefinedArgs argsList (argTypes : specs) = - match (argsList, argTypes) with - | ( {J.expression_desc = Undefined {isUnit = false}; _} :: rest, - {External_arg_spec.arg_label = Arg_optional; _} :: argTypes ) -> - keepNonUndefinedArgs rest argTypes - | _ -> argsList - in - let args = - keepNonUndefinedArgs (List.rev args) (List.rev arg_types) |> List.rev - in + let args = keep_non_undefined_args args arg_types in add_eff eff @@ E.call ~info:{ arity = Full; call_info = Call_na } fn args | Js_module_as_fn { external_module_name; splice } -> @@ -341,6 +342,7 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types (E.dot self name) args) else let args, eff = assemble_args_no_splice arg_types args in + let args = keep_non_undefined_args args arg_types in add_eff eff (let self = translate_scoped_access js_send_scopes self in E.call diff --git a/jscomp/test/omit_trailing_undefined_in_external_calls.js b/jscomp/test/omit_trailing_undefined_in_external_calls.js index 7a225b6f1d..fcc2df5ee4 100644 --- a/jscomp/test/omit_trailing_undefined_in_external_calls.js +++ b/jscomp/test/omit_trailing_undefined_in_external_calls.js @@ -9,7 +9,9 @@ SomeModule.formatDate(new Date(), { someOption: true }); -var x = SomeModule.formatDate(new Date(), undefined, true); +SomeModule.formatDate(new Date(), undefined, true); + +var x = (42).toString(); exports.x = x; /* Not a pure module */ diff --git a/jscomp/test/omit_trailing_undefined_in_external_calls.res b/jscomp/test/omit_trailing_undefined_in_external_calls.res index 4e4fca946d..38024b9160 100644 --- a/jscomp/test/omit_trailing_undefined_in_external_calls.res +++ b/jscomp/test/omit_trailing_undefined_in_external_calls.res @@ -9,3 +9,7 @@ external formatDate: (Js.Date.t, ~options: dateFormatOptions=?, ~done: bool=?) = let x = formatDate(Js.Date.make()) let x = formatDate(Js.Date.make(), ~options={someOption: true}) let x = formatDate(Js.Date.make(), ~done=true) + +@send external toString: (float, ~radix: int=?) => string = "toString" + +let x = toString(42.) From 3e9c4bf81678813c51d49b9af59eaf7fce522eaf Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 6 Apr 2024 08:39:32 +0200 Subject: [PATCH 2/6] release.ninja changed --- jscomp/runtime/release.ninja | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jscomp/runtime/release.ninja b/jscomp/runtime/release.ninja index 317583d49c..d21ca55b4d 100644 --- a/jscomp/runtime/release.ninja +++ b/jscomp/runtime/release.ninja @@ -25,7 +25,7 @@ o runtime/caml_exceptions.cmj : cc_cmi runtime/caml_exceptions.res | runtime/cam o runtime/caml_exceptions.cmi : cc runtime/caml_exceptions.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_float.cmj : cc_cmi runtime/caml_float.res | runtime/caml_float.cmi runtime/caml_float_extern.cmj o runtime/caml_float.cmi : cc runtime/caml_float.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj -o runtime/caml_format.cmj : cc_cmi runtime/caml_format.ml | runtime/caml_float.cmj runtime/caml_float_extern.cmj runtime/caml_format.cmi runtime/caml_int64.cmj runtime/caml_int64_extern.cmj runtime/caml_nativeint_extern.cmj runtime/caml_string_extern.cmj +o runtime/caml_format.cmj : cc_cmi runtime/caml_format.ml | runtime/caml.cmj runtime/caml_float.cmj runtime/caml_float_extern.cmj runtime/caml_format.cmi runtime/caml_int64.cmj runtime/caml_int64_extern.cmj runtime/caml_nativeint_extern.cmj runtime/caml_string_extern.cmj o runtime/caml_format.cmi : cc runtime/caml_format.mli | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_hash.cmj : cc_cmi runtime/caml_hash.res | runtime/caml_hash.cmi runtime/caml_hash_primitive.cmj runtime/caml_nativeint_extern.cmj runtime/js.cmj o runtime/caml_hash.cmi : cc runtime/caml_hash.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj @@ -41,7 +41,7 @@ o runtime/caml_md5.cmj : cc_cmi runtime/caml_md5.res | runtime/caml_array_extern o runtime/caml_md5.cmi : cc runtime/caml_md5.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_module.cmj : cc_cmi runtime/caml_module.res | runtime/caml_array_extern.cmj runtime/caml_module.cmi runtime/caml_obj.cmj o runtime/caml_module.cmi : cc runtime/caml_module.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj -o runtime/caml_obj.cmj : cc_cmi runtime/caml_obj.res | runtime/caml_array_extern.cmj runtime/caml_obj.cmi runtime/caml_option.cmj runtime/js.cmj +o runtime/caml_obj.cmj : cc_cmi runtime/caml_obj.res | runtime/caml.cmj runtime/caml_array_extern.cmj runtime/caml_obj.cmi runtime/caml_option.cmj runtime/js.cmj o runtime/caml_obj.cmi : cc runtime/caml_obj.resi | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_option.cmj : cc_cmi runtime/caml_option.res | runtime/caml_option.cmi runtime/caml_undefined_extern.cmj runtime/js.cmj o runtime/caml_option.cmi : cc runtime/caml_option.resi | runtime/bs_stdlib_mini.cmi runtime/caml_undefined_extern.cmj runtime/js.cmi runtime/js.cmj @@ -58,7 +58,7 @@ o runtime/caml_bigint_extern.cmi runtime/caml_bigint_extern.cmj : cc runtime/cam o runtime/caml_external_polyfill.cmi runtime/caml_external_polyfill.cmj : cc runtime/caml_external_polyfill.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_float_extern.cmi runtime/caml_float_extern.cmj : cc runtime/caml_float_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_int64_extern.cmi runtime/caml_int64_extern.cmj : cc runtime/caml_int64_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj -o runtime/caml_js_exceptions.cmi runtime/caml_js_exceptions.cmj : cc runtime/caml_js_exceptions.res | runtime/bs_stdlib_mini.cmi runtime/caml_exceptions.cmj runtime/js.cmi runtime/js.cmj +o runtime/caml_js_exceptions.cmi runtime/caml_js_exceptions.cmj : cc runtime/caml_js_exceptions.res | runtime/bs_stdlib_mini.cmi runtime/caml_exceptions.cmj runtime/caml_option.cmj runtime/js.cmi runtime/js.cmj o runtime/caml_nativeint_extern.cmi runtime/caml_nativeint_extern.cmj : cc runtime/caml_nativeint_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_string_extern.cmi runtime/caml_string_extern.cmj : cc runtime/caml_string_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj o runtime/caml_undefined_extern.cmi runtime/caml_undefined_extern.cmj : cc runtime/caml_undefined_extern.res | runtime/bs_stdlib_mini.cmi runtime/js.cmi runtime/js.cmj From 07c6dec9675d69e4f5d2bc817dd4070eb62e6449 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 6 Apr 2024 08:52:18 +0200 Subject: [PATCH 3/6] CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01a231c23d..2338fdb73a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,8 @@ #### :bug: Bug Fix - Improve error when using '@deriving(accessors)' on a variant with record arguments. https://github.com/rescript-lang/rescript-compiler/pull/6712 - - Stop escaping JSX prop names with hyphens. https://github.com/rescript-lang/rescript-compiler/pull/6705 +- Fix trailing undefined for optional parameters not omitted with `@send`. https://github.com/rescript-lang/rescript-compiler/pull/6716 # 11.1.0-rc.7 From 19ee61765893764e5309dfc8682707877a53708d Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 6 Apr 2024 14:49:53 +0200 Subject: [PATCH 4/6] Handle more cases of trailing undefineds --- jscomp/core/lam_compile_external_call.ml | 24 +++++++++---------- ...it_trailing_undefined_in_external_calls.js | 4 +++- ...t_trailing_undefined_in_external_calls.res | 8 +++++-- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/jscomp/core/lam_compile_external_call.ml b/jscomp/core/lam_compile_external_call.ml index 23f6bfec77..a21f2d2acc 100644 --- a/jscomp/core/lam_compile_external_call.ml +++ b/jscomp/core/lam_compile_external_call.ml @@ -151,6 +151,16 @@ type specs = External_arg_spec.params type exprs = E.t list +let keep_non_undefined_args (arg_types : specs) (args : exprs) = + let rec aux arg_types args = + match (args, arg_types) with + | ( {J.expression_desc = Undefined {isUnit = false}; _} :: args_rest, + {External_arg_spec.arg_label = Arg_optional; _} :: arg_types_rest ) -> + aux arg_types_rest args_rest + | _ -> args + in + aux (List.rev arg_types) (List.rev args) |> List.rev + (* TODO: fix splice, we need a static guarantee that it is static array construct otherwise, we should provide a good error message here, @@ -176,7 +186,7 @@ let assemble_args_no_splice (arg_types : specs) (args : exprs) : | _ :: _, [] -> assert false in let args, eff = aux arg_types args in - ( args, + ( keep_non_undefined_args arg_types args, match eff with | [] -> None | x :: xs -> @@ -250,16 +260,6 @@ let translate_scoped_access scopes obj = | [] -> obj | x :: xs -> Ext_list.fold_left xs (E.dot obj x) E.dot -let keep_non_undefined_args args arg_types = - let rec aux argsList (argTypes : specs) = - match (argsList, argTypes) with - | ( {J.expression_desc = Undefined {isUnit = false}; _} :: rest, - {External_arg_spec.arg_label = Arg_optional; _} :: argTypes ) -> - aux rest argTypes - | _ -> argsList - in - aux (List.rev args) (List.rev arg_types) |> List.rev - let translate_ffi (cxt : Lam_compile_context.t) arg_types (ffi : External_ffi_types.external_spec) (args : J.expression list) ~dynamic_import = match ffi with @@ -281,7 +281,6 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types else E.call ~info:{ arity = Full; call_info = Call_na } fn args) else let args, eff = assemble_args_no_splice arg_types args in - let args = keep_non_undefined_args args arg_types in add_eff eff @@ E.call ~info:{ arity = Full; call_info = Call_na } fn args | Js_module_as_fn { external_module_name; splice } -> @@ -342,7 +341,6 @@ let translate_ffi (cxt : Lam_compile_context.t) arg_types (E.dot self name) args) else let args, eff = assemble_args_no_splice arg_types args in - let args = keep_non_undefined_args args arg_types in add_eff eff (let self = translate_scoped_access js_send_scopes self in E.call diff --git a/jscomp/test/omit_trailing_undefined_in_external_calls.js b/jscomp/test/omit_trailing_undefined_in_external_calls.js index fcc2df5ee4..12603d50f5 100644 --- a/jscomp/test/omit_trailing_undefined_in_external_calls.js +++ b/jscomp/test/omit_trailing_undefined_in_external_calls.js @@ -11,7 +11,9 @@ SomeModule.formatDate(new Date(), { SomeModule.formatDate(new Date(), undefined, true); -var x = (42).toString(); +(42).toString(); + +var x = new RegExp("ab+c"); exports.x = x; /* Not a pure module */ diff --git a/jscomp/test/omit_trailing_undefined_in_external_calls.res b/jscomp/test/omit_trailing_undefined_in_external_calls.res index 38024b9160..07b4ac6e3e 100644 --- a/jscomp/test/omit_trailing_undefined_in_external_calls.res +++ b/jscomp/test/omit_trailing_undefined_in_external_calls.res @@ -10,6 +10,10 @@ let x = formatDate(Js.Date.make()) let x = formatDate(Js.Date.make(), ~options={someOption: true}) let x = formatDate(Js.Date.make(), ~done=true) -@send external toString: (float, ~radix: int=?) => string = "toString" +@send external floatToString: (float, ~radix: int=?) => string = "toString" -let x = toString(42.) +let x = floatToString(42.) + +@new external regExpFromString: (string, ~flags: string=?) => Js.Re.t = "RegExp" + +let x = regExpFromString("ab+c") \ No newline at end of file From a03af7218bbe0d162c7e4b17044bc5ba45c561e9 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 6 Apr 2024 17:26:47 +0200 Subject: [PATCH 5/6] CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2338fdb73a..3a0834c43c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ - Improve error when using '@deriving(accessors)' on a variant with record arguments. https://github.com/rescript-lang/rescript-compiler/pull/6712 - Stop escaping JSX prop names with hyphens. https://github.com/rescript-lang/rescript-compiler/pull/6705 -- Fix trailing undefined for optional parameters not omitted with `@send`. https://github.com/rescript-lang/rescript-compiler/pull/6716 +- Fix trailing undefined for optional parameters not omitted with `@send` and `@new`. https://github.com/rescript-lang/rescript-compiler/pull/6716 # 11.1.0-rc.7 From 208ac67765ba22b18fc410c9f26b5b8eeb6ff069 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 6 Apr 2024 19:28:17 +0200 Subject: [PATCH 6/6] Check first if there are any trailing undefined args --- jscomp/core/lam_compile_external_call.ml | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/jscomp/core/lam_compile_external_call.ml b/jscomp/core/lam_compile_external_call.ml index a21f2d2acc..77a982ccd1 100644 --- a/jscomp/core/lam_compile_external_call.ml +++ b/jscomp/core/lam_compile_external_call.ml @@ -152,14 +152,25 @@ type specs = External_arg_spec.params type exprs = E.t list let keep_non_undefined_args (arg_types : specs) (args : exprs) = + let rec has_undefined_trailing_args arg_types args = + match (arg_types, args) with + | ( [{External_arg_spec.arg_label = Arg_optional; _}], + [{J.expression_desc = Undefined {isUnit = false}; _}] ) -> + true + | ( _ :: arg_types_rest, _ :: args_rest ) -> + has_undefined_trailing_args arg_types_rest args_rest + | _ -> false + in let rec aux arg_types args = - match (args, arg_types) with - | ( {J.expression_desc = Undefined {isUnit = false}; _} :: args_rest, - {External_arg_spec.arg_label = Arg_optional; _} :: arg_types_rest ) -> + match (arg_types, args) with + | ( {External_arg_spec.arg_label = Arg_optional; _} :: arg_types_rest, + {J.expression_desc = Undefined {isUnit = false}; _} :: args_rest ) -> aux arg_types_rest args_rest | _ -> args in - aux (List.rev arg_types) (List.rev args) |> List.rev + if (has_undefined_trailing_args arg_types args) then + aux (List.rev arg_types) (List.rev args) |> List.rev + else args (* TODO: fix splice, we need a static guarantee that it is static array construct