Option optimization: do not create redundant local vars

CHANGELOG.md

@@ -25,6 +25,8 @@
 #### :nail_care: Polish
 
 - Keep track of compiler info during build. https://github.com/rescript-lang/rescript/pull/7889
+- Improve option optimization for constants. https://github.com/rescript-lang/rescript/pull/7913
+- Option optimization: do not create redundant local vars. https://github.com/rescript-lang/rescript/pull/7915
 
 #### :house: Internal
 
@@ -48,7 +50,6 @@
 - Add (dev-)dependencies to build schema. https://github.com/rescript-lang/rescript/pull/7892
 - Dedicated error for dict literal spreads. https://github.com/rescript-lang/rescript/pull/7901
 - Dedicated error message for when mixing up `:` and `=` in various positions. https://github.com/rescript-lang/rescript/pull/7900
-- Improve option optimization for constants. https://github.com/rescript-lang/rescript/pull/7913
 
 # 12.0.0-beta.11
 

compiler/core/lam_util.ml

@@ -38,86 +38,104 @@ 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 the semantics encoded by Lambda's let_kind.  Downstream passes at
+   the JS backend interpret the k-tag as the shape of code they are allowed to
+   emit:
+     Strict     --> emit `const x = e; body`, with `e` evaluated exactly once.
+                    Reordering `e` or duplicating it would be incorrect.
+     StrictOpt  --> emit either `const x = e; body` (when `x` is used) or drop
+                    the declaration entirely (when DCE prunes `x`).  Duplicating
+                    `e` remains forbidden.
+     Alias      --> emit `const x = e; body` or substitute `e` directly at each
+                    use site, removing the binding if convenient.
+     Variable   --> emit a thunked shape like `function() { return e; }` or keep
+                    the original `let` without forcing; evaluation must stay
+                    deferred.
+
+   The function implements this contract through ordered rewrite clauses:
+   - (Return)      [let[k] x = e in x]               ⟶ e
+   - (Prim)        [let[k] x = e in prim p x]        ⟶ prim p e   (p ≠ makeblock)
+   - (Call)        [let[k] x = e in f x]             ⟶ f e        (x not captured in f)
+   - (Alias)       [let[k] x = e in body]            ⟶ let[Alias] x = e in body
+                     when k ∈ {Strict, StrictOpt} and SafeAlias(e)
+   - (Strict λ)    [let[Strict] x = fn in body]      ⟶ let[StrictOpt] x = fn in body
+   - (Strict Pure) [let[Strict] x = e in body]       ⟶ let[StrictOpt] x = e in body
+                     when no_side_effects(e)
+   Falling through keeps the original binding.  Only the Alias clause changes
+   evaluation strategy downstream, so we keep its predicate intentionally
+   syntactic and narrow. *)
+  let refine_let ~kind param (arg : Lam.t) (l : Lam.t) : Lam.t =
+  let is_block_constructor = function
+    | Lam_primitive.Pmakeblock _ -> true
+    | _ -> false
+  in
+  (* SafeAlias is the predicate that justifies the (Alias) rewrite
+       let[k] x = e in body  -->  let[Alias] x = e in body
+     for strict bindings.  Turning a binding into [Alias] authorises JS codegen
+     to inline [e] at every use site or drop `const x = e` entirely, so every
+     clause below must ensure that duplicate evaluation of [e] is equivalent to
+     the single eager evaluation promised by [Strict]/[StrictOpt]. *)
+  let rec is_safe_to_alias (lam : Lam.t) =
+    match lam with
+    | Lvar _ | Lconst _ ->
+        (* var/const --> emitting multiple `const` reads is identical to the
+           original eager evaluation, so codegen may inline them freely. *)
+        true
+    | Lprim { primitive = Pfield (_, Fld_module _); args = [ (Lglobal_module _ | Lvar _) ]; _ } ->
+        (* field read --> access hits an immutable module block; inlining emits
+           the same read the eager binding would have performed once. *)
+        true
+    | Lprim { primitive = Psome_not_nest; args = [inner]; _ } ->
+        (* some_not_nest(inner) --> expands to two explicit rewrites:
+             let[k] x = inner  --> let[Alias] x = inner
+             let[Alias] x = inner --> let[Alias] x = Some(inner)
+           The recursive call discharges the first arrow; the constructor wrap is
+           allocation-free in JS, so the second arrow preserves the single eager
+           evaluation promised by Strict/StrictOpt. *)
+        is_safe_to_alias inner
+    | _ -> 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_safe_to_alias 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 +278,3 @@ let is_var (lam : Lam.t) id =
      lapply (let a = 3 in let b = 4 in fun x y -> x + y) 2 3 
    ]}   
 *)
-
-
-
-
-
-
-
-

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

@@ -58,8 +58,6 @@ let x37 = new Intl.DateTimeFormat();
 
 let x38 = new Intl.NumberFormat();
 
-let x39 = true;
-
 let x40 = new Intl.Collator();
 
 let x41 = new Intl.RelativeTimeFormat();
@@ -91,6 +89,8 @@ let x5 = {
 
 let x12 = "test";
 
+let x39 = true;
+
 let x44 = [
   1,
   2