Option optimization: do not create redundant local vars

compiler/core/lam_util.ml

@@ -38,86 +38,60 @@ let add_required_modules ( x : Ident.t list) (meta : Lam_stats.t) =
 *)
 
 
-(* 
-    It's impossible to have a case like below:
-   {[
-     (let export_f = ... in export_f)
-   ]}
-    Even so, it's still correct
-*)
-let refine_let
-    ~kind param
-    (arg : Lam.t) (l : Lam.t)  : Lam.t =
-
-  match (kind : Lam_compat.let_kind ), arg, l  with 
-  | _, _, Lvar w when Ident.same w param 
-    (* let k = xx in k
-       there is no [rec] so [k] would not appear in [xx]
-    *)
-    -> arg (* TODO: optimize here -- it's safe to do substitution here *)
-  | _, _, Lprim {primitive ; args =  [Lvar w]; loc ; _} when Ident.same w param 
-                                                          &&  (function | Lam_primitive.Pmakeblock _ -> false | _ ->  true) primitive
-    (* don't inline inside a block *)
-    ->  Lam.prim ~primitive ~args:[arg]  loc 
-  (* we can not do this substitution when capttured *)
-  (* | _, Lvar _, _ -> (\** let u = h in xxx*\) *)
-  (*     (\* assert false *\) *)
-  (*     Ext_log.err "@[substitution >> @]@."; *)
-  (*     let v= subst_lambda (Map_ident.singleton param arg ) l in *)
-  (*     Ext_log.err "@[substitution << @]@."; *)
-  (* v *)
-  | _, _, Lapply {ap_func=fn; ap_args = [Lvar w]; ap_info; ap_transformed_jsx} when
-      Ident.same w param &&
-      (not (Lam_hit.hit_variable param fn ))
-    -> 
-    (* does not work for multiple args since 
-        evaluation order unspecified, does not apply 
-        for [js] in general, since the scope of js ir is loosen
-
-        here we remove the definition of [param]
-       {[ let k = v in (body) k 
-       ]}
-        #1667 make sure body does not hit k 
-    *)
-    Lam.apply fn [arg] ap_info ~ap_transformed_jsx
-  | (Strict | StrictOpt ),
-    ( Lvar _    | Lconst  _ | 
-      Lprim {primitive = Pfield (_ , Fld_module _) ;  
-             args = [ Lglobal_module _ | Lvar _ ]; _}) , _ ->
-    (* (match arg with  *)
-    (* | Lconst _ ->  *)
-    (*     Ext_log.err "@[%a %s@]@."  *)
-    (*       Ident.print param (string_of_lambda arg) *)
-    (* | _ -> ()); *)
-    (* No side effect and does not depend on store,
-        since function evaluation is always delayed
-    *)
-    Lam.let_ Alias param arg l
-  | ( (Strict | StrictOpt ) ), (Lfunction _ ), _ ->
-    (*It can be promoted to [Alias], however, 
-        we don't want to do this, since we don't want the 
-        function to be inlined to a block, for example
-      {[
-        let f = fun _ -> 1 in
-        [0, f]
-      ]}
-        TODO: punish inliner to inline functions 
-        into a block 
-    *)
-    Lam.let_ StrictOpt  param arg l
-  (* Not the case, the block itself can have side effects 
-      we can apply [no_side_effects] pass 
-      | Some Strict, Lprim(Pmakeblock (_,_,Immutable),_) ->  
-        Llet(StrictOpt, param, arg, l) 
-  *)      
-  | Strict, _ ,_  when Lam_analysis.no_side_effects arg ->
-    Lam.let_ StrictOpt param arg l
-  | Variable, _, _ -> 
-    Lam.let_ Variable  param arg l
-  | kind, _, _ -> 
-    Lam.let_ kind  param arg l
-(* | None , _, _ -> 
-   Lam.let_ Strict param arg  l *)
+(* refine_let normalises let-bindings so we avoid redundant locals while
+   preserving evaluation semantics. *)
+let refine_let ~kind param (arg : Lam.t) (l : Lam.t) : Lam.t =
+  let is_block_constructor = function
+    | Lam_primitive.Pmakeblock _ -> true
+    | _ -> false
+  in
+  let is_alias_candidate (lam : Lam.t) =
+    match lam with
+    | Lvar _ | Lconst _ -> true
+    | Lprim { primitive = Psome_not_nest; _ } -> true
+    | Lprim { primitive = Pfield (_, Fld_module _); args = [ (Lglobal_module _ | Lvar _) ]; _ } -> true
+    | _ -> false
+  in
+  match (kind : Lam_compat.let_kind), arg, l with
+  | _, _, Lvar w when Ident.same w param ->
+      (* If the body immediately returns the binding (e.g. `{ let x = value; x }`),
+         we skip creating `x` and keep `value`. There is no `rec`, so `value`
+         cannot refer back to `x`, and we avoid generating a redundant local. *)
+      arg
+  | _, _, Lprim { primitive; args = [ Lvar w ]; loc; _ }
+      when Ident.same w param && not (is_block_constructor primitive) ->
+      (* When we immediately feed the binding into a primitive, like
+         `{ let x = value; Array.length(x) }`, we inline the primitive call
+         with `value`. This only happens for primitives that are pure and do not
+         allocate new blocks, so evaluation order and side effects stay the same. *)
+      Lam.prim ~primitive ~args:[arg] loc
+  | _, _, Lapply { ap_func = fn; ap_args = [ Lvar w ]; ap_info; ap_transformed_jsx }
+      when Ident.same w param && not (Lam_hit.hit_variable param fn) ->
+      (* For a function call such as `{ let x = value; someFn(x) }`, we can
+         rewrite to `someFn(value)` as long as the callee does not capture `x`.
+         This removes the temporary binding while preserving the call semantics. *)
+      Lam.apply fn [arg] ap_info ~ap_transformed_jsx
+  | (Strict | StrictOpt), arg, _ when is_alias_candidate arg ->
+      (* `Strict` and `StrictOpt` bindings both evaluate the RHS immediately
+         (with `StrictOpt` allowing later elimination if unused). When that RHS
+         is pure — `{ let x = Some(value); ... }`, `{ let x = 3; ... }`, or a module
+         field read — we mark it as an alias so downstream passes can inline the
+         original expression and drop the temporary. *)
+      Lam.let_ Alias param arg l
+  | Strict, Lfunction _, _ ->
+      (* If we eagerly evaluate a function binding such as
+         `{ let makeGreeting = () => "hi"; ... }`, we end up allocating the
+         closure immediately. Downgrading `Strict` to `StrictOpt` preserves the
+         original laziness while still letting later passes inline when safe. *)
+      Lam.let_ StrictOpt param arg l
+  | Strict, _, _ when Lam_analysis.no_side_effects arg ->
+      (* A strict binding whose expression has no side effects — think
+         `{ let x = computePure(); use(x); }` — can be relaxed to `StrictOpt`.
+         This keeps the original semantics yet allows downstream passes to skip
+         evaluating `x` when it turns out to be unused. *)
+      Lam.let_ StrictOpt param arg l
+  | kind, _, _ ->
+      Lam.let_ kind param arg l
 
 let alias_ident_or_global (meta : Lam_stats.t) (k:Ident.t) (v:Ident.t) 
     (v_kind : Lam_id_kind.t)  =
@@ -260,11 +234,3 @@ let is_var (lam : Lam.t) id =
      lapply (let a = 3 in let b = 4 in fun x y -> x + y) 2 3 
    ]}   
 *)
-
-
-
-
-
-
-
-

packages/@rescript/runtime/lib/es6/Belt_internalAVLset.js

@@ -741,15 +741,13 @@ function rotateWithRightChild(k1) {
 
 function doubleWithLeftChild(k3) {
   let k3l = k3.l;
-  let v = rotateWithRightChild(k3l);
-  k3.l = v;
+  k3.l = rotateWithRightChild(k3l);
   return rotateWithLeftChild(k3);
 }
 
 function doubleWithRightChild(k2) {
   let k2r = k2.r;
-  let v = rotateWithLeftChild(k2r);
-  k2.r = v;
+  k2.r = rotateWithLeftChild(k2r);
   return rotateWithRightChild(k2);
 }
 

packages/@rescript/runtime/lib/js/Belt_internalAVLset.js

@@ -741,15 +741,13 @@ function rotateWithRightChild(k1) {
 
 function doubleWithLeftChild(k3) {
   let k3l = k3.l;
-  let v = rotateWithRightChild(k3l);
-  k3.l = v;
+  k3.l = rotateWithRightChild(k3l);
   return rotateWithLeftChild(k3);
 }
 
 function doubleWithRightChild(k2) {
   let k2r = k2.r;
-  let v = rotateWithLeftChild(k2r);
-  k2.r = v;
+  k2.r = rotateWithLeftChild(k2r);
   return rotateWithRightChild(k2);
 }
 

tests/tests/src/option_optimisation.mjs

@@ -3,9 +3,7 @@
 import * as Primitive_option from "@rescript/runtime/lib/es6/Primitive_option.js";
 
 function boolean(val1, val2) {
-  let a = val1;
-  let b = val2;
-  if (b || a) {
+  if (val2 || val1) {
     return "a";
   } else {
     return "b";
@@ -33,8 +31,7 @@ function constant() {
 }
 
 function param(opt) {
-  let x = opt;
-  console.log(x);
+  console.log(opt);
 }
 
 export {

tests/tests/src/option_wrapping_test.mjs

@@ -8,6 +8,25 @@ let x6 = {
   x: 42
 };
 
+let x10 = null;
+
+let x11 = Primitive_option.some(undefined);
+
+let x1 = "hello";
+
+let x2 = 1;
+
+let x3 = {
+  TAG: "Ok",
+  _0: "hi"
+};
+
+let x4 = "polyvar";
+
+let x5 = {
+  x: 42
+};
+
 let x7 = [
   1,
   2,
@@ -16,9 +35,7 @@ let x7 = [
 
 let x8 = () => {};
 
-let x10 = null;
-
-let x11 = Primitive_option.some(undefined);
+let x12 = "test";
 
 let x20 = null;
 
@@ -68,36 +85,19 @@ let x42 = new Intl.PluralRules();
 
 let x43 = new Intl.Locale("en");
 
-let x45 = Promise.resolve(true);
-
-let x47 = {};
-
-let x48 = Stdlib_Lazy.make(() => true);
-
-let x1 = "hello";
-
-let x2 = 1;
-
-let x3 = {
-  TAG: "Ok",
-  _0: "hi"
-};
-
-let x4 = "polyvar";
-
-let x5 = {
-  x: 42
-};
-
-let x12 = "test";
-
 let x44 = [
   1,
   2
 ];
 
+let x45 = Promise.resolve(true);
+
 let x46 = /* [] */0;
 
+let x47 = {};
+
+let x48 = Stdlib_Lazy.make(() => true);
+
 export {
   x1,
   x2,

tests/tests/src/reasonReact.mjs

@@ -92,17 +92,6 @@ function wrapReasonForJs(component, jsPropsToReason) {
 let dummyInteropComponent = basicComponent("interop");
 
 function wrapJsForReason(reactClass, props, children) {
-  let jsElementWrapped = (extra, extra$1) => {
-    let props$1 = Object.assign(Object.assign({}, props), {
-      ref: extra$1,
-      key: extra
-    });
-    let varargs = Js_array.concat(children, [
-      reactClass,
-      props$1
-    ]);
-    return React.createElement.apply(null, varargs);
-  };
   return {
     debugName: dummyInteropComponent.debugName,
     reactClassInternal: dummyInteropComponent.reactClassInternal,
@@ -117,7 +106,17 @@ function wrapJsForReason(reactClass, props, children) {
     initialState: dummyInteropComponent.initialState,
     retainedProps: dummyInteropComponent.retainedProps,
     reducer: dummyInteropComponent.reducer,
-    jsElementWrapped: jsElementWrapped
+    jsElementWrapped: (extra, extra$1) => {
+      let props$1 = Object.assign(Object.assign({}, props), {
+        ref: extra$1,
+        key: extra
+      });
+      let varargs = Js_array.concat(children, [
+        reactClass,
+        props$1
+      ]);
+      return React.createElement.apply(null, varargs);
+    }
   };
 }