Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn off transformation for closures inside loops. #6480

Merged
merged 1 commit into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions jscomp/core/j.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 0 additions & 31 deletions jscomp/core/js_closure.ml

This file was deleted.

35 changes: 0 additions & 35 deletions jscomp/core/js_closure.mli

This file was deleted.

78 changes: 8 additions & 70 deletions jscomp/core/js_dump.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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 ->
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/js_fold.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions jscomp/core/js_fun_env.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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 =
Expand Down Expand Up @@ -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 *)
4 changes: 0 additions & 4 deletions jscomp/core/js_fun_env.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions jscomp/core/js_pass_scope.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/js_record_fold.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/js_record_iter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions jscomp/core/js_record_map.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions jscomp/core/js_stmt_make.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 0 additions & 2 deletions jscomp/core/js_stmt_make.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
Loading
Loading