diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a5ad9f2ea2..8eb31546ca9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ - Compile dict spreads to object spread syntax instead of `Object.assign`. https://github.com/rescript-lang/rescript/pull/8747 - Make compilation slightly faster and stop rewatch from copying files twice for modules with an interface. https://github.com/rescript-lang/rescript/pull/8769 - Speed up inlining decisions for large functions. https://github.com/rescript-lang/rescript/pull/8771 +- Speed up the rewriting of JS globals shadowed by local bindings in nested functions. https://github.com/rescript-lang/rescript/pull/8772 - Represent explicit expression braces as `Pexp_braces` in parsetree v1 and format `else` branches consistently with `if` branches. https://github.com/rescript-lang/rescript/pull/8678 - Omit redundant braces around multi-statement switch case bodies when formatting. https://github.com/rescript-lang/rescript/pull/8677 - Avoid running `rescript-schema-ppx` and `sury-ppx` on source files without an `@schema` annotation. https://github.com/rescript-lang/rescript/pull/8662 diff --git a/compiler/core/js_pass_external_shadow.ml b/compiler/core/js_pass_external_shadow.ml index 1fe994506cb..ccbdc5ac8f7 100644 --- a/compiler/core/js_pass_external_shadow.ml +++ b/compiler/core/js_pass_external_shadow.ml @@ -40,22 +40,6 @@ let should_rewrite_binding (ident : Ident.t) = && (not (Js_reserved_map.is_js_keyword ident.name)) && not (Js_reserved_map.is_js_global ident.name) -let rewrite_shadowed_global_in_expr ~(name : string) (expr : J.expression) : - J.expression = - let super = Js_record_map.super in - let self = - { - super with - expression = - (fun self expr -> - match expr.expression_desc with - | Var (Id id) when Ext_ident.is_js id && String.equal id.name name -> - E.dot global_this name - | _ -> super.expression self expr); - } - in - self.expression self expr - let program (js : J.program) : J.program = let shadowed_globals = Ext_list.fold_left js.block String_set.empty (fun acc (st : J.statement) -> @@ -66,6 +50,9 @@ let program (js : J.program) : J.program = String_set.add ident.name acc | _ -> acc) in + (* Names of the lexical bindings whose initializer is being traversed: a JS + global with such a name can't be referenced directly there. *) + let enclosing_bindings = ref String_set.empty in let super = Js_record_map.super in let self = { @@ -73,6 +60,10 @@ let program (js : J.program) : J.program = expression = (fun self expr -> match expr.expression_desc with + | Var (Id id) + when Ext_ident.is_js id + && String_set.mem id.name !enclosing_bindings -> + E.dot global_this id.name | Static_index (obj, field, pos) -> let obj = self.expression self obj in let obj = @@ -88,11 +79,14 @@ let program (js : J.program) : J.program = variable_declaration = (fun self (vd : J.variable_declaration) -> match vd with - | {ident; value = Some expr; property} + | {ident; value = Some _; property} when is_lexical_binding_kind property && should_rewrite_binding ident -> - let expr = rewrite_shadowed_global_in_expr ~name:ident.name expr in - super.variable_declaration self {vd with value = Some expr} + let outer = !enclosing_bindings in + enclosing_bindings := String_set.add ident.name outer; + let vd = super.variable_declaration self vd in + enclosing_bindings := outer; + vd | _ -> super.variable_declaration self vd); } in diff --git a/tests/tests/src/ExternalShadow.mjs b/tests/tests/src/ExternalShadow.mjs index 8ad2d4bd8b4..9a14967e3f8 100644 --- a/tests/tests/src/ExternalShadow.mjs +++ b/tests/tests/src/ExternalShadow.mjs @@ -17,6 +17,21 @@ let Global = {}; let $$parseInt = parseInt; +let Y = {}; + +function myGlobalA(n) { + let inner = m => { + let myGlobalB = (((globalThis.myGlobalB * m | 0) + (globalThis.myGlobalA * n | 0) | 0) + ((m * n | 0) * 3 | 0) | 0) + 7 | 0; + return (myGlobalB * myGlobalB | 0) + m | 0; + }; + let sibling = m => (((myGlobalB * m | 0) + (globalThis.myGlobalA * n | 0) | 0) + ((m * n | 0) * 5 | 0) | 0) + 11 | 0; + return (((inner(n) + inner(n + 1 | 0) | 0) + sibling(n) | 0) + sibling(n + 1 | 0) | 0) + globalThis.myGlobalA | 0; +} + +function useB(n) { + return ((myGlobalB * n | 0) + ((n * n | 0) * 7 | 0) | 0) + 13 | 0; +} + export { X, process, @@ -25,5 +40,8 @@ export { url, Global, $$parseInt, + Y, + myGlobalA, + useB, } /* process Not a pure module */ diff --git a/tests/tests/src/ExternalShadow.res b/tests/tests/src/ExternalShadow.res index aabe1cea279..3317c8b7785 100644 --- a/tests/tests/src/ExternalShadow.res +++ b/tests/tests/src/ExternalShadow.res @@ -21,3 +21,24 @@ module Global = { } let parseInt = Global.parseInt /* expect plain `parseInt` */ + +/* Nested lexical bindings: a reference is rewritten while it is inside the + initializer of any binding with its name, and only there. */ +module Y = { + @val external myGlobalA: int = "myGlobalA" + @val external myGlobalB: int = "myGlobalB" +} + +let myGlobalA = (n: int) => { + let inner = (m: int) => { + /* inside both initializers: both rewritten */ + let myGlobalB = Y.myGlobalB * m + Y.myGlobalA * n + m * n * 3 + 7 + myGlobalB * myGlobalB + m + } + /* back in myGlobalA's initializer only: myGlobalB stays plain */ + let sibling = (m: int) => Y.myGlobalB * m + Y.myGlobalA * n + m * n * 5 + 11 + inner(n) + inner(n + 1) + sibling(n) + sibling(n + 1) + Y.myGlobalA +} + +/* outside both initializers: not rewritten */ +let useB = (n: int) => Y.myGlobalB * n + n * n * 7 + 13