From a644c995779fdf2065844da28723a5702314603d Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 20 Dec 2025 18:24:30 +0100 Subject: [PATCH 1/3] Fix shadowing @val bindings (rewrite using globalThis) --- compiler/core/js_pass_external_shadow.ml | 73 +++++++++++++++++++++++ compiler/core/js_pass_external_shadow.mli | 26 ++++++++ compiler/core/lam_compile_main.ml | 4 +- tests/tests/src/ExternalShadow.mjs | 21 +++++++ tests/tests/src/ExternalShadow.res | 16 +++++ 5 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 compiler/core/js_pass_external_shadow.ml create mode 100644 compiler/core/js_pass_external_shadow.mli create mode 100644 tests/tests/src/ExternalShadow.mjs create mode 100644 tests/tests/src/ExternalShadow.res diff --git a/compiler/core/js_pass_external_shadow.ml b/compiler/core/js_pass_external_shadow.ml new file mode 100644 index 0000000000..8ca3ccb31c --- /dev/null +++ b/compiler/core/js_pass_external_shadow.ml @@ -0,0 +1,73 @@ +(* Copyright (C) 2025 - Authors of ReScript + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * In addition to the permissions granted to you by the LGPL, you may combine + * or link a "work that uses the Library" with a publicly distributed version + * of this file to produce a combined library or application, then distribute + * that combined work under the terms of your choosing, with no requirement + * to comply with the obligations normally placed on you by section 4 of the + * LGPL version 3 (or the corresponding section of a later version of the LGPL + * should you choose to use a later version). + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) + +module E = Js_exp_make + +let global_this = E.js_global "globalThis" + +(* Only rewrite for lexical bindings (let-style). *) +let is_lexical_binding_kind (property : J.property) = + match property with + | Strict | StrictOpt | Alias -> true + | Variable -> false + +(* Skip JS globals/keywords and compiler-reserved JS ids. *) +let should_rewrite_binding (ident : Ident.t) = + (not (Ext_ident.is_js ident)) + && (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 super = Js_record_map.super in + let self = + { + super with + variable_declaration = + (fun self (vd : J.variable_declaration) -> + match vd with + | {ident; value = Some expr; property} + when is_lexical_binding_kind property + && should_rewrite_binding ident -> + let expr = rewrite_shadowed_global_in_expr ~name:ident.name expr in + {vd with value = Some expr} + | _ -> super.variable_declaration self vd); + } + in + self.program self js diff --git a/compiler/core/js_pass_external_shadow.mli b/compiler/core/js_pass_external_shadow.mli new file mode 100644 index 0000000000..0271e886dd --- /dev/null +++ b/compiler/core/js_pass_external_shadow.mli @@ -0,0 +1,26 @@ +(* Copyright (C) 2025 - Authors of ReScript + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * In addition to the permissions granted to you by the LGPL, you may combine + * or link a "work that uses the Library" with a publicly distributed version + * of this file to produce a combined library or application, then distribute + * that combined work under the terms of your choosing, with no requirement + * to comply with the obligations normally placed on you by section 4 of the + * LGPL version 3 (or the corresponding section of a later version of the LGPL + * should you choose to use a later version). + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) + +(* Rewrites @val/@new shadowing to avoid reading a let before it is set. *) +val program : J.program -> J.program diff --git a/compiler/core/lam_compile_main.ml b/compiler/core/lam_compile_main.ml index bd4191b0e7..5871d643eb 100644 --- a/compiler/core/lam_compile_main.ml +++ b/compiler/core/lam_compile_main.ml @@ -242,7 +242,9 @@ in js |> _j "initial" |> Js_pass_flatten.program -|> _j "flattern" +|> _j "flatten" +|> Js_pass_external_shadow.program +|> _j "external_shadow" |> Js_pass_tailcall_inline.tailcall_inline |> _j "inline_and_shake" |> Js_pass_flatten_and_mark_dead.program diff --git a/tests/tests/src/ExternalShadow.mjs b/tests/tests/src/ExternalShadow.mjs new file mode 100644 index 0000000000..fb88d8fc1d --- /dev/null +++ b/tests/tests/src/ExternalShadow.mjs @@ -0,0 +1,21 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + + +let X = {}; + +let process = globalThis.process; + +let proc = process; + +let Global = {}; + +let $$parseInt = parseInt; + +export { + X, + process, + proc, + Global, + $$parseInt, +} +/* process Not a pure module */ diff --git a/tests/tests/src/ExternalShadow.res b/tests/tests/src/ExternalShadow.res new file mode 100644 index 0000000000..01b4f412de --- /dev/null +++ b/tests/tests/src/ExternalShadow.res @@ -0,0 +1,16 @@ +/* Avoid @val shadowing: `let process = X.process` must not read itself. + The initializer should compile to `globalThis.process`, while other + uses of the external stay as plain `process`. */ +module X = { + @val external process: unknown = "process" +} + +let process = X.process /* expect `globalThis.process` */ +let proc = X.process /* expect plain `process` */ + +/* Reserved JS globals should not be rewritten to globalThis. */ +module Global = { + @val external parseInt: unknown = "parseInt" +} + +let parseInt = Global.parseInt /* expect plain `parseInt` */ From 1eea2e708e1523370b10784c084cce83b5182513 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 21 Dec 2025 09:27:11 +0100 Subject: [PATCH 2/3] Add example with @new --- tests/tests/src/ExternalShadow.mjs | 8 ++++++++ tests/tests/src/ExternalShadow.res | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/tests/tests/src/ExternalShadow.mjs b/tests/tests/src/ExternalShadow.mjs index fb88d8fc1d..8ad2d4bd8b 100644 --- a/tests/tests/src/ExternalShadow.mjs +++ b/tests/tests/src/ExternalShadow.mjs @@ -7,6 +7,12 @@ let process = globalThis.process; let proc = process; +let New = {}; + +function url(prim) { + return new URL(prim); +} + let Global = {}; let $$parseInt = parseInt; @@ -15,6 +21,8 @@ export { X, process, proc, + New, + url, Global, $$parseInt, } diff --git a/tests/tests/src/ExternalShadow.res b/tests/tests/src/ExternalShadow.res index 01b4f412de..aabe1cea27 100644 --- a/tests/tests/src/ExternalShadow.res +++ b/tests/tests/src/ExternalShadow.res @@ -8,6 +8,13 @@ module X = { let process = X.process /* expect `globalThis.process` */ let proc = X.process /* expect plain `process` */ +/* @new does not have the same shadowing issue because it uses `new URL(...)`. */ +module New = { + @new external url: string => unknown = "URL" +} + +let url = New.url /* expect `new URL(...)` in a wrapper function */ + /* Reserved JS globals should not be rewritten to globalThis. */ module Global = { @val external parseInt: unknown = "parseInt" From 7f90da14b26c19f1d927c97b69ca740ebdf05b8c Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 21 Dec 2025 09:28:39 +0100 Subject: [PATCH 3/3] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 940037e207..895cf7e0a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - Reanalyze: make optional args analysis liveness-aware, preventing false positives when functions are only called from dead code. https://github.com/rescript-lang/rescript/pull/8082 - Fix: do not warn for "editor" field in `rescript.json`. https://github.com/rescript-lang/rescript/pull/8084 +- Fix `@val` shadowing (rewrite using `globalThis`). https://github.com/rescript-lang/rescript/pull/8098 #### :memo: Documentation