Consolidate large record mutation output into a single spread object literal (#8473)

* Consolidate large record update output into a single spread object literal

So it emits `{...x, a: 1}` instead of `let newrecord = {...x}; newrecord.a = 1;`.

* codex review

* changelog

Author: cometkim

CHANGELOG.md

@@ -45,6 +45,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

compiler/core/js_analyzer.ml

@@ -113,7 +113,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

compiler/core/js_dump.ml

@@ -671,6 +671,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

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)

tests/tests/src/jsx_preserve_test.mjs

@@ -54,19 +54,15 @@ let baseProps = {
   title: "foo"
 };
 
-let newrecord = {...baseProps};
-
 let _unary_element_with_spread_props = <input
-  {...newrecord} 
+  {...baseProps} 
   type={"text"}
 />;
 
-let newrecord$1 = {...baseProps};
-
 let _container_with_spread_props = <div
-  {...newrecord$1} 
-  title={"barry"}
+  {...baseProps} 
   className={"barry"}
+  title={"barry"}
 >
   {"Hello, world!"}
   <input
@@ -83,21 +79,17 @@ let baseChildren = [
   </span>
 ];
 
-let newrecord$2 = {...baseProps};
-
 let _unary_element_with_spread_props_keyed = <input
   key={"barry-key"} 
-  {...newrecord$2} 
+  {...baseProps} 
   type={"text"}
 />;
 
-let newrecord$3 = {...baseProps};
-
 let _container_with_spread_props_keyed = <div
   key={"barry-key"} 
-  {...newrecord$3} 
-  title={"barry"}
+  {...baseProps} 
   className={"barry"}
+  title={"barry"}
 >
   {"Hello, world!"}
   <input

tests/tests/src/large_record_duplication_test.mjs

@@ -35,11 +35,10 @@ let A0 = /* @__PURE__ */Primitive_exceptions.create("Large_record_duplication_te
 
 Mocha.describe("Large_record_duplication_test", () => {
   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) {

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 = {

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())));