Turn off transformation for closures inside loops (#6480)

The scope of `var` is per-function, requiring a transformation for closures inside loops when capturing loop variables. This PR turns off the transformation.
rescript-lang · May 1, 2024 · cfed01f · cfed01f
1 parent 7bf97d9
commit cfed01f
Show file tree

Hide file tree

Showing 30 changed files with 150 additions and 398 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@
 - Fix indent for returned/thrown/wrapped in parentheses objects in generated js code. https://github.com/rescript-lang/rescript-compiler/pull/6746
 - Fix indent in generated js code. https://github.com/rescript-lang/rescript-compiler/pull/6747
 - In generated code, use `let` instead of `var`. https://github.com/rescript-lang/rescript-compiler/pull/6102
+- Turn off transformation for closures inside loops when capturing loop variables, now that `let` is emitted instead of `var`. https://github.com/rescript-lang/rescript-compiler/pull/6480
 
 # 11.1.0
 

diff --git a/jscomp/core/j.ml b/jscomp/core/j.ml
@@ -254,15 +254,14 @@ and statement_desc =
   (* Function declaration and Variable declaration  *)
   | Exp of expression
   | If of expression * block * block
-  | While of label option * expression * block * Js_closure.t
+  | While of label option * expression * block
     (* check if it contains loop mutable values, happens in nested loop *)
   | ForRange of
       for_ident_expression option
       * finish_ident_expression
       * for_ident
       * for_direction
       * block
-      * Js_closure.t
   | Continue of label
   | Break (* only used when inline a fucntion *)
   | Return of expression

diff --git a/jscomp/core/js_closure.ml b/jscomp/core/js_closure.ml
diff --git a/jscomp/core/js_closure.mli b/jscomp/core/js_closure.mli
diff --git a/jscomp/core/js_dump.ml b/jscomp/core/js_dump.ml
@@ -164,14 +164,6 @@ let exp_need_paren (e : J.expression) =
   | Await _ -> false
   | Tagged_template _ -> false
 
-let comma_idents (cxt : cxt) f ls = iter_lst cxt f ls Ext_pp_scope.ident comma
-
-let pp_paren_params (inner_cxt : cxt) (f : Ext_pp.t) (lexical : Ident.t list) :
-    unit =
-  P.string f L.lparen;
-  let (_ : cxt) = comma_idents inner_cxt f lexical in
-  P.string f L.rparen
-
 (** Print as underscore for unused vars, may not be 
     needed in the future *)
 (* let ipp_ident cxt f id (un_used : bool) =
@@ -381,10 +373,9 @@ and pp_function ~return_unit ~async ~is_method cxt (f : P.t) ~fn_state
           P.space f;
           P.brace_vgroup f 1 (fun _ -> function_body ~return_unit cxt f b)
       in
-      let lexical : Set_ident.t = Js_fun_env.get_lexical_scope env in
-      let enclose lexical =
-        let handle lexical =
-          if Set_ident.is_empty lexical then (
+      let enclose () =
+        let handle () =
+          (
             match fn_state with
             | Is_return ->
                 return_sp f;
@@ -408,46 +399,10 @@ and pp_function ~return_unit ~async ~is_method cxt (f : P.t) ~fn_state
                 P.space f;
                 ignore (Ext_pp_scope.ident inner_cxt f x : cxt);
                 param_body ())
-          else
-            (* print our closure as
-               {[(function(x,y){ return function(..){...}} (x,y))]}
-               Maybe changed to `let` in the future
-            *)
-            let lexical = Set_ident.elements lexical in
-            (match fn_state with
-            | Is_return -> return_sp f
-            | No_name _ -> ()
-            | Name_non_top name | Name_top name ->
-                ignore (pp_var_assign inner_cxt f name : cxt));
-            if async then P.string f L.await;
-            P.string f L.lparen;
-            P.string f (L.function_async ~async);
-            pp_paren_params inner_cxt f lexical;
-            P.brace_vgroup f 0 (fun _ ->
-                return_sp f;
-                P.string f (L.function_async ~async);
-                P.space f;
-                (match fn_state with
-                | Is_return | No_name _ -> ()
-                | Name_non_top x | Name_top x ->
-                    ignore (Ext_pp_scope.ident inner_cxt f x));
-                param_body ());
-            pp_paren_params inner_cxt f lexical;
-            P.string f L.rparen;
-            match fn_state with
-            | Is_return | No_name _ -> () (* expression *)
-            | _ -> semi f
-          (* has binding, a statement *)
         in
-        handle
-          (match fn_state with
-          | (Name_top name | Name_non_top name) when Set_ident.mem lexical name
-            ->
-              (*TODO: when calculating lexical we should not include itself *)
-              Set_ident.remove lexical name
-          | _ -> lexical)
+        handle ()
       in
-      enclose lexical;
+      enclose ();
       outer_cxt
 
 (* Assume the cond would not change the context,
@@ -1065,7 +1020,7 @@ and statement_desc top cxt f (s : J.statement_desc) : cxt =
           P.string f L.else_;
           P.space f;
           brace_block cxt f s2)
-  | While (label, e, s, _env) ->
+  | While (label, e, s) ->
       (*  FIXME: print scope as well *)
       (match label with
       | Some i ->
@@ -1093,7 +1048,7 @@ and statement_desc top cxt f (s : J.statement_desc) : cxt =
       let cxt = brace_block cxt f s in
       semi f;
       cxt
-  | ForRange (for_ident_expression, finish, id, direction, s, env) ->
+  | ForRange (for_ident_expression, finish, id, direction, s) ->
       let action cxt =
         P.vgroup f 0 (fun _ ->
             let cxt =
@@ -1164,24 +1119,7 @@ and statement_desc top cxt f (s : J.statement_desc) : cxt =
             in
             brace_block cxt f s)
       in
-      let lexical = Js_closure.get_lexical_scope env in
-      if Set_ident.is_empty lexical then action cxt
-      else
-        (* unlike function,
-           [print for loop] has side effect,
-           we should take it out
-        *)
-        let inner_cxt = Ext_pp_scope.merge cxt lexical in
-        let lexical = Set_ident.elements lexical in
-        P.vgroup f 0 (fun _ ->
-            P.string f L.lparen;
-            P.string f L.function_;
-            pp_paren_params inner_cxt f lexical;
-            let cxt = P.brace_vgroup f 0 (fun _ -> action inner_cxt) in
-            pp_paren_params inner_cxt f lexical;
-            P.string f L.rparen;
-            semi f;
-            cxt)
+      action cxt
   | Continue s ->
       continue f s;
       cxt

diff --git a/jscomp/core/js_fold.ml b/jscomp/core/js_fold.ml
@@ -217,12 +217,12 @@ class fold =
           let _self = _self#block _x1 in
           let _self = _self#block _x2 in
           _self
-      | While (_x0, _x1, _x2, _x3) ->
+      | While (_x0, _x1, _x2) ->
           let _self = option (fun _self -> _self#label) _self _x0 in
           let _self = _self#expression _x1 in
           let _self = _self#block _x2 in
           _self
-      | ForRange (_x0, _x1, _x2, _x3, _x4, _x5) ->
+      | ForRange (_x0, _x1, _x2, _x3, _x4) ->
           let _self =
             option (fun _self -> _self#for_ident_expression) _self _x0
           in

diff --git a/jscomp/core/js_fun_env.ml b/jscomp/core/js_fun_env.ml
@@ -44,7 +44,6 @@ type immutable_mask =
 
 type t = {
   mutable unbounded : Set_ident.t;
-  mutable bound_loop_mutable_values : Set_ident.t;
   used_mask : bool array;
   immutable_mask : immutable_mask;
 }
@@ -58,7 +57,6 @@ let make ?immutable_mask n =
       (match immutable_mask with
       | Some x -> Immutable_mask x
       | None -> All_immutable_and_no_tail_call);
-    bound_loop_mutable_values = Set_ident.empty;
   }
 
 let no_tailcall x =
@@ -92,13 +90,3 @@ let set_unbounded env v =
   (* if Set_ident.is_empty env.bound then *)
   env.unbounded <- v
 (* else assert false *)
-
-let set_lexical_scope env bound_loop_mutable_values =
-  env.bound_loop_mutable_values <- bound_loop_mutable_values
-
-let get_lexical_scope env = env.bound_loop_mutable_values
-
-(* TODO:  can be refined if it
-    only enclose toplevel variables
-*)
-(* let is_empty t = Set_ident.is_empty t.unbounded *)
diff --git a/jscomp/core/js_fun_env.mli b/jscomp/core/js_fun_env.mli
@@ -36,10 +36,6 @@ val no_tailcall : t -> bool list
 
 val set_unbounded : t -> Set_ident.t -> unit
 
-val set_lexical_scope : t -> Set_ident.t -> unit
-
-val get_lexical_scope : t -> Set_ident.t
-
 (* val to_string : t -> string *)
 
 val mark_unused : t -> int -> unit

diff --git a/jscomp/core/js_pass_scope.ml b/jscomp/core/js_pass_scope.ml
@@ -173,10 +173,6 @@ let record_scope_pass =
                due to the recursive thing
             *)
             Js_fun_env.set_unbounded env closured_idents';
-            let lexical_scopes =
-              Set_ident.(inter closured_idents' state.loop_mutable_values)
-            in
-            Js_fun_env.set_lexical_scope env lexical_scopes;
             (* tailcall , note that these varibles are used in another pass *)
             {
               state with
@@ -242,7 +238,7 @@ let record_scope_pass =
     statement =
       (fun self state x ->
         match x.statement_desc with
-        | ForRange (_, _, loop_id, _, _, a_env) ->
+        | ForRange (_, _, loop_id, _, _) ->
             (* TODO: simplify definition of For *)
             let {
               defined_idents = defined_idents';
@@ -274,8 +270,6 @@ let record_scope_pass =
                   (diff closured_idents' defined_idents')
                   state.loop_mutable_values)
             in
-            let () = Js_closure.set_lexical_scope a_env lexical_scope in
-            (* set scope *)
             {
               state with
               used_idents = Set_ident.union state.used_idents used_idents';
@@ -293,7 +287,7 @@ let record_scope_pass =
               closured_idents =
                 Set_ident.union state.closured_idents lexical_scope;
             }
-        | While (_label, pred, body, _env) ->
+        | While (_label, pred, body) ->
             with_in_loop
               (self.block self
                  (with_in_loop (self.expression self state pred) true)

diff --git a/jscomp/core/js_record_fold.ml b/jscomp/core/js_record_fold.ml
@@ -223,12 +223,12 @@ let statement_desc : 'a. ('a, statement_desc) fn =
       let st = _self.block _self st _x1 in
       let st = _self.block _self st _x2 in
       st
-  | While (_x0, _x1, _x2, _x3) ->
+  | While (_x0, _x1, _x2) ->
       let st = option label _self st _x0 in
       let st = _self.expression _self st _x1 in
       let st = _self.block _self st _x2 in
       st
-  | ForRange (_x0, _x1, _x2, _x3, _x4, _x5) ->
+  | ForRange (_x0, _x1, _x2, _x3, _x4) ->
       let st = option for_ident_expression _self st _x0 in
       let st = finish_ident_expression _self st _x1 in
       let st = _self.for_ident _self st _x2 in

diff --git a/jscomp/core/js_record_iter.ml b/jscomp/core/js_record_iter.ml
@@ -163,11 +163,11 @@ let statement_desc : statement_desc fn =
       _self.expression _self _x0;
       _self.block _self _x1;
       _self.block _self _x2
-  | While (_x0, _x1, _x2, _x3) ->
+  | While (_x0, _x1, _x2) ->
       option label _self _x0;
       _self.expression _self _x1;
       _self.block _self _x2
-  | ForRange (_x0, _x1, _x2, _x3, _x4, _x5) ->
+  | ForRange (_x0, _x1, _x2, _x3, _x4) ->
       option for_ident_expression _self _x0;
       finish_ident_expression _self _x1;
       _self.for_ident _self _x2;

diff --git a/jscomp/core/js_record_map.ml b/jscomp/core/js_record_map.ml
@@ -221,18 +221,18 @@ let statement_desc : statement_desc fn =
       let _x1 = _self.block _self _x1 in
       let _x2 = _self.block _self _x2 in
       If (_x0, _x1, _x2)
-  | While (_x0, _x1, _x2, _x3) ->
+  | While (_x0, _x1, _x2) ->
       let _x0 = option label _self _x0 in
       let _x1 = _self.expression _self _x1 in
       let _x2 = _self.block _self _x2 in
-      While (_x0, _x1, _x2, _x3)
-  | ForRange (_x0, _x1, _x2, _x3, _x4, _x5) ->
+      While (_x0, _x1, _x2)
+  | ForRange (_x0, _x1, _x2, _x3, _x4) ->
       let _x0 = option for_ident_expression _self _x0 in
       let _x1 = finish_ident_expression _self _x1 in
       let _x2 = _self.for_ident _self _x2 in
       let _x3 = for_direction _self _x3 in
       let _x4 = _self.block _self _x4 in
-      ForRange (_x0, _x1, _x2, _x3, _x4, _x5)
+      ForRange (_x0, _x1, _x2, _x3, _x4)
   | Continue _x0 ->
       let _x0 = label _self _x0 in
       Continue _x0

diff --git a/jscomp/core/js_stmt_make.ml b/jscomp/core/js_stmt_make.ml
@@ -318,17 +318,15 @@ let if_ ?comment ?declaration ?else_ (e : J.expression) (then_ : J.block) : t =
 let assign ?comment id e : t =
   { statement_desc = J.Exp (E.assign (E.var id) e); comment }
 
-let while_ ?comment ?label ?env (e : E.t) (st : J.block) : t =
-  let env = match env with None -> Js_closure.empty () | Some x -> x in
-  { statement_desc = While (label, e, st, env); comment }
+let while_ ?comment ?label (e : E.t) (st : J.block) : t =
+  { statement_desc = While (label, e, st); comment }
 
-let for_ ?comment ?env for_ident_expression finish_ident_expression id direction
+let for_ ?comment for_ident_expression finish_ident_expression id direction
     (b : J.block) : t =
-  let env = match env with None -> Js_closure.empty () | Some x -> x in
   {
     statement_desc =
       ForRange
-        (for_ident_expression, finish_ident_expression, id, direction, b, env);
+        (for_ident_expression, finish_ident_expression, id, direction, b);
     comment;
   }
 

diff --git a/jscomp/core/js_stmt_make.mli b/jscomp/core/js_stmt_make.mli
@@ -133,14 +133,12 @@ val assign : ?comment:string -> J.ident -> J.expression -> t
 val while_ :
   ?comment:string ->
   ?label:J.label ->
-  ?env:Js_closure.t ->
   J.expression ->
   J.block ->
   t
 
 val for_ :
   ?comment:string ->
-  ?env:Js_closure.t ->
   J.for_ident_expression option ->
   J.finish_ident_expression ->
   J.for_ident ->