Rewrite shadowed globals in a single walk

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

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,13 +50,20 @@ 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 =
     {
       super with
       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

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 */

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