diff --git a/CHANGELOG.md b/CHANGELOG.md index 070c866edc8..69131928ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ #### :nail_care: Polish +- Consolidate record mutation output into a single spread object literal. https://github.com/rescript-lang/rescript/pull/8473 - Improve default argument type mismatch errors. https://github.com/rescript-lang/rescript/pull/8389 - Resolve workspace dependencies in editor analysis. https://github.com/rescript-lang/rescript/pull/8392 - Build system: Add OpenTelemetry tracing support for cli commands. https://github.com/rescript-lang/rescript/pull/8370 diff --git a/compiler/core/js_analyzer.ml b/compiler/core/js_analyzer.ml index 25852412667..4675e29a708 100644 --- a/compiler/core/js_analyzer.ml +++ b/compiler/core/js_analyzer.ml @@ -102,7 +102,11 @@ let rec no_side_effect_expression_desc (x : J.expression_desc) = *) Ext_list.for_all xs no_side_effect | Optional_block (x, _) -> no_side_effect x - | Object (_, kvs) -> Ext_list.for_all_snd kvs no_side_effect + | Object (dup, kvs) -> + (match dup with + | Some e -> no_side_effect e + | None -> true) + && Ext_list.for_all_snd kvs no_side_effect | String_append (a, b) | Seq (a, b) -> no_side_effect a && no_side_effect b | Length (e, _) | Caml_block_tag (e, _) | Typeof e -> no_side_effect e | Bin (op, a, b) -> op <> Eq && no_side_effect a && no_side_effect b diff --git a/compiler/core/js_dump.ml b/compiler/core/js_dump.ml index 6f6da8b605c..998b5e9b340 100644 --- a/compiler/core/js_dump.ml +++ b/compiler/core/js_dump.ml @@ -618,6 +618,29 @@ and expression_desc cxt ~(level : int) f x : cxt = | [tag; ({expression_desc = J.Var _} as spread_props)] -> (* All the props are spread *) print_jsx cxt ~level ~spread_props f fn_name tag [] + | [tag; {expression_desc = J.Object (Some spread, props)}] -> + (* Spread props with overrides emitted as a single object literal: + {...base, x: 1} + *) + let fields = + List.filter_map + (fun (n, x) -> + match n with + | Js_op.Lit name -> Some (name, x) + | Symbol_name -> None) + props + in + print_jsx cxt ~level ~spread_props:spread f fn_name tag fields + | [tag; {expression_desc = J.Object (Some spread, props)}; key] -> + let fields = + List.filter_map + (fun (n, x) -> + match n with + | Js_op.Lit name -> Some (name, x) + | Symbol_name -> None) + props + in + print_jsx cxt ~level ~spread_props:spread ~key f fn_name tag fields | _ -> (* This should not happen, we fallback to the general case *) expression_desc cxt ~level f diff --git a/compiler/core/lam_compile.ml b/compiler/core/lam_compile.ml index 97f6bec84e5..dd7497c2416 100644 --- a/compiler/core/lam_compile.ml +++ b/compiler/core/lam_compile.ml @@ -1834,6 +1834,59 @@ let compile output_prefix = in Js_output.output_of_block_and_expression lambda_cxt.continuation args_code exp + and collect_dup_overrides (copy_id : Ident.t) (lam : Lam.t) + (acc : (Lam_compat.set_field_dbg_info * Lam.t) list) : + (Lam_compat.set_field_dbg_info * Lam.t) list option = + match lam with + | Lsequence + ( Lprim + {primitive = Psetfield (_, fld_info); args = [Lvar id'; value]; _}, + rest ) + when Ident.same id' copy_id -> + collect_dup_overrides copy_id rest ((fld_info, value) :: acc) + | Lvar id' when Ident.same id' copy_id -> Some acc + | _ -> None + and try_compile_record_spread (lambda_cxt : Lam_compile_context.t) + (id : Ident.t) (arg : Lam.t) (body : Lam.t) : Js_output.t option = + match arg with + | Lprim {primitive = Pduprecord; args = [init]; _} -> ( + match collect_dup_overrides id body [] with + | None -> None + | Some overrides -> + let need_value_cxt = + {lambda_cxt with continuation = NeedValue Not_tail} + in + let init_output = compile_lambda need_value_cxt init in + let init_val = + match init_output.value with + | Some v -> v + | None -> assert false + in + let blocks, props = + List.fold_left + (fun (blocks, props) + ((fld_info : Lam_compat.set_field_dbg_info), value_lam) -> + let val_output = compile_lambda need_value_cxt value_lam in + let val_val = + match val_output.value with + | Some v -> v + | None -> assert false + in + let name = + match fld_info with + | Fld_record_set name + | Fld_record_inline_set name + | Fld_record_extension_set name -> + name + in + (blocks @ val_output.block, (Js_op.Lit name, val_val) :: props)) + (init_output.block, []) (List.rev overrides) + in + Some + (Js_output.output_of_block_and_expression lambda_cxt.continuation + blocks + (E.obj ~dup:init_val props))) + | _ -> None and compile_lambda (lambda_cxt : Lam_compile_context.t) (cur_lam : Lam.t) : Js_output.t = match cur_lam with @@ -1859,14 +1912,17 @@ let compile output_prefix = } body))) | Lapply appinfo -> compile_apply appinfo lambda_cxt - | Llet (let_kind, id, arg, body) -> - (* Order matters.. see comment below in [Lletrec] *) - let args_code = - compile_lambda - {lambda_cxt with continuation = Declare (let_kind, id)} - arg - in - Js_output.append_output args_code (compile_lambda lambda_cxt body) + | Llet (let_kind, id, arg, body) -> ( + match try_compile_record_spread lambda_cxt id arg body with + | Some output -> output + | None -> + (* Order matters.. see comment below in [Lletrec] *) + let args_code = + compile_lambda + {lambda_cxt with continuation = Declare (let_kind, id)} + arg + in + Js_output.append_output args_code (compile_lambda lambda_cxt body)) | Lletrec (id_args, body) -> (* There is a bug in our current design, it requires compile args first (register that some objects are jsidentifiers) diff --git a/tests/tests/src/jsx_preserve_test.mjs b/tests/tests/src/jsx_preserve_test.mjs index bcc16a2d3ce..a839e400133 100644 --- a/tests/tests/src/jsx_preserve_test.mjs +++ b/tests/tests/src/jsx_preserve_test.mjs @@ -54,19 +54,15 @@ let baseProps = { title: "foo" }; -let newrecord = {...baseProps}; - let _unary_element_with_spread_props = ; -let newrecord$1 = {...baseProps}; - let _container_with_spread_props =
{"Hello, world!"} ]; -let newrecord$2 = {...baseProps}; - let _unary_element_with_spread_props_keyed = ; -let newrecord$3 = {...baseProps}; - let _container_with_spread_props_keyed =
{"Hello, world!"} { Mocha.test("large record spread operation", () => { - let f0 = x => { - let newrecord = {...x}; - newrecord.x0 = 1; - return newrecord; - }; + let f0 = x => ({ + ...x, + x0: 1 + }); let result = f0(v0); Test_utils.eq("File \"large_record_duplication_test.res\", line 143, characters 7-14", result.x0, 1); Test_utils.eq("File \"large_record_duplication_test.res\", line 144, characters 7-14", result.x1, 9); @@ -86,10 +85,12 @@ Mocha.describe("Large_record_duplication_test", () => { let f1 = x => { if (typeof x !== "object") { return "A1"; + } else { + return { + ...x, + x0: 1 + }; } - let newrecord = {...x}; - newrecord.x0 = 1; - return newrecord; }; Test_utils.eq("File \"large_record_duplication_test.res\", line 201, characters 7-14", get_x0(f1({ TAG: "A0", @@ -125,12 +126,14 @@ Mocha.describe("Large_record_duplication_test", () => { } }; let f2 = x => { - if (x.TAG !== "A0") { + if (x.TAG === "A0") { + return { + ...x, + x0: 1 + }; + } else { return x; } - let newrecord = {...x}; - newrecord.x0 = 1; - return newrecord; }; Test_utils.eq("File \"large_record_duplication_test.res\", line 243, characters 7-14", get_x0(f2({ TAG: "A0", @@ -161,12 +164,14 @@ Mocha.describe("Large_record_duplication_test", () => { }); Mocha.test("exception extension with large record", () => { let f3 = x => { - if (x.RE_EXN_ID !== A0) { + if (x.RE_EXN_ID === A0) { + return { + ...x, + x0: 1 + }; + } else { return x; } - let newrecord = {...x}; - newrecord.x0 = 1; - return newrecord; }; let get_x0 = x => { if (x.RE_EXN_ID === A0) { diff --git a/tests/tests/src/record_regression.mjs b/tests/tests/src/record_regression.mjs index 62cbc46ea14..9f23990ec2b 100644 --- a/tests/tests/src/record_regression.mjs +++ b/tests/tests/src/record_regression.mjs @@ -39,9 +39,10 @@ let newrecord$3 = {...v1}; newrecord$3.y1 = 22; function h11(v1) { - let newrecord = {...v1}; - newrecord.y1 = 22; - return newrecord; + return { + ...v1, + y1: 22 + }; } let po = { diff --git a/tests/tests/src/stdlib/intl/Stdlib_Intl_DateTimeFormatTest.mjs b/tests/tests/src/stdlib/intl/Stdlib_Intl_DateTimeFormatTest.mjs index c7008f5c7eb..09452136f91 100644 --- a/tests/tests/src/stdlib/intl/Stdlib_Intl_DateTimeFormatTest.mjs +++ b/tests/tests/src/stdlib/intl/Stdlib_Intl_DateTimeFormatTest.mjs @@ -46,33 +46,38 @@ let formatter$1 = new Intl.DateTimeFormat(undefined, options); console.log(formatter$1.format(new Date(Date.now()))); -let newrecord = {...options}; - -let formatter$2 = new Intl.DateTimeFormat(undefined, (newrecord.timeZoneName = "long", newrecord)); +let formatter$2 = new Intl.DateTimeFormat(undefined, { + ...options, + timeZoneName: "long" +}); console.log(formatter$2.format(new Date(Date.now()))); -let newrecord$1 = {...options}; - -let formatter$3 = new Intl.DateTimeFormat(undefined, (newrecord$1.timeZoneName = "longOffset", newrecord$1)); +let formatter$3 = new Intl.DateTimeFormat(undefined, { + ...options, + timeZoneName: "longOffset" +}); console.log(formatter$3.format(new Date(Date.now()))); -let newrecord$2 = {...options}; - -let formatter$4 = new Intl.DateTimeFormat(undefined, (newrecord$2.timeZoneName = "short", newrecord$2)); +let formatter$4 = new Intl.DateTimeFormat(undefined, { + ...options, + timeZoneName: "short" +}); console.log(formatter$4.format(new Date(Date.now()))); -let newrecord$3 = {...options}; - -let formatter$5 = new Intl.DateTimeFormat(undefined, (newrecord$3.timeZoneName = "shortGeneric", newrecord$3)); +let formatter$5 = new Intl.DateTimeFormat(undefined, { + ...options, + timeZoneName: "shortGeneric" +}); console.log(formatter$5.format(new Date(Date.now()))); -let newrecord$4 = {...options}; - -let formatter$6 = new Intl.DateTimeFormat(undefined, (newrecord$4.timeZoneName = "shortOffset", newrecord$4)); +let formatter$6 = new Intl.DateTimeFormat(undefined, { + ...options, + timeZoneName: "shortOffset" +}); console.log(formatter$6.format(new Date(Date.now())));