fix(transform): stop cross-module inlining bundling a separately exported sibling by value

changelog.d/10630-fn-identity-own-module.md

@@ -0,0 +1,37 @@
+### Fixed
+
+- **A function referenced inside its own module is a different object from
+  the same function imported elsewhere (#10554).** `function f(){}; export
+  function isSame(x){ return x === f; }; export { f };` gave `isSame(f)`
+  `false` for an importer's own `f` — identity checks, registries/caches
+  keyed by function, `removeEventListener`/`off(fn)`, and memoization all
+  silently took the wrong branch.
+
+  Root cause: the cross-module *function* inliner in
+  `perry-transform`'s `inline/cross_module.rs` harvests an exported
+  function's whole value-dependency graph — every function it transitively
+  references, including by value (`x === f`), not just as a call target —
+  and clones the entire graph into every importing module under fresh
+  `__perry_xmod_inline_<id>_<name>` symbols. When the referenced function
+  (`f`) is *also* independently exported, it got cloned alongside the
+  candidate instead of resolved through its own canonical wrapper. Every
+  function value materializes into a heap closure keyed by its wrapper
+  *symbol* (`js_closure_alloc_singleton`), so the clone's `f` and the
+  canonical `f` every importer resolves through produced two distinct
+  closures — an in-module identity check comparing them disagreed with
+  every importer's own view.
+
+  Fix: `gather_cross_module_functions` now refuses a candidate whose
+  dependency graph would need to bundle a *separately exported* sibling
+  function referenced by value — it falls back to an ordinary cross-module
+  call instead, which resolves through the shared canonical wrapper.
+  Self-recursion is unaffected. The directly-affected shape actually gets
+  **faster**, not slower: the unsound inline was paying for an extra
+  closure materialization on every call.
+
+  Validation: new `test_gap_10554_fn_identity_own_module` (function
+  declaration, function expression, arrow-in-const, named export, a barrel
+  re-export, a default export referencing an exported sibling, `Set`
+  membership, both identity directions) fails on the baseline and matches
+  Node on the fix; the existing `test_gap_10434`/export/import/cross-module/
+  inline/module gap-test families (16 tests) are unaffected.

crates/perry-transform/src/inline/cross_module.rs

@@ -163,6 +163,25 @@ pub fn gather_cross_module_functions(module: &Module) -> HashMap<String, Functio
         .collect();
     source_class_names.extend(imported_binding_names(module));
 
+    // #10554: a function referenced BY VALUE (`Expr::FuncRef`, not just
+    // called) from a candidate's dependency graph must resolve to the SAME
+    // closure singleton every importer sees. Bundling a clone of a
+    // SEPARATELY EXPORTED sibling breaks that -- the clone gets its own
+    // `__perry_xmod_inline_<id>_<name>` wrapper symbol, a different
+    // `js_closure_alloc_singleton` key than the sibling's own canonical
+    // `__perry_wrap_perry_fn_<src>__<name>`, so `x === exportedSibling`
+    // inside the inlined body silently disagrees with every importer's view
+    // of `exportedSibling`. `exported_ids` gates `collect_function_graph`'s
+    // dependency walk below: a graph that would need to bundle a
+    // separately-exported function is refused entirely (no candidate),
+    // falling back to the ordinary cross-module call, which shares the
+    // source module's own canonical wrapper.
+    let exported_ids: HashSet<FuncId> = module
+        .exported_functions
+        .iter()
+        .map(|(_, id)| *id)
+        .collect();
+
     let mut out = HashMap::new();
     for (exported_name, root_id) in &module.exported_functions {
         let mut visiting = HashSet::new();
@@ -174,6 +193,7 @@ pub fn gather_cross_module_functions(module: &Module) -> HashMap<String, Functio
             &mut visiting,
             &mut visited,
             &mut graph_ids,
+            &exported_ids,
         ) {
             continue;
         }
@@ -244,6 +264,7 @@ fn collect_function_graph(
     visiting: &mut HashSet<FuncId>,
     visited: &mut HashSet<FuncId>,
     out: &mut Vec<FuncId>,
+    exported_ids: &HashSet<FuncId>,
 ) -> bool {
     if visited.contains(&id) {
         return true;
@@ -268,7 +289,15 @@ fn collect_function_graph(
     refs.sort_unstable();
     refs.dedup();
     for dependency in refs {
-        if !collect_function_graph(dependency, functions, visiting, visited, out) {
+        // #10554: a dependency pulled in only because the body reads it as a
+        // VALUE (`Expr::FuncRef`) -- not merely calls it -- must not be
+        // bundled as a private clone when it is ALSO independently exported.
+        // `dependency != id` lets a function's own export status not block
+        // its (already-permitted) self-recursion.
+        if dependency != id && exported_ids.contains(&dependency) {
+            return false;
+        }
+        if !collect_function_graph(dependency, functions, visiting, visited, out, exported_ids) {
             return false;
         }
         if visited.len() > MAX_CROSS_MODULE_FUNCTION_GRAPH {

test-files/_helpers/fn_identity_10554/lib.ts

@@ -0,0 +1,37 @@
+// Shared helpers for #10554: a function referenced inside its own module
+// must be the SAME object an importer sees.
+export function fnDecl() {
+  return 1;
+}
+export const fnExpr = function fnExprNamed() {
+  return 2;
+};
+export const arrowFn = () => 3;
+
+// Value comparisons made FROM WITHIN this module -- the defect's exact
+// shape: `isSame*` is itself exported, so it is a candidate for
+// cross-module inlining, and its body's reference to the sibling export is
+// a plain in-module reference, not an import.
+export function isSameDecl(x: unknown) {
+  return x === fnDecl;
+}
+export function isSameExpr(x: unknown) {
+  return x === fnExpr;
+}
+export function isSameArrow(x: unknown) {
+  return x === arrowFn;
+}
+export function bothSame(a: unknown, b: unknown) {
+  return a === b;
+}
+export function makeSet() {
+  return new Set<unknown>([fnDecl, fnExpr, arrowFn]);
+}
+
+// A default export that ALSO references an exported sibling by value --
+// #10548 fixed the export-ROW identity for `export default F`; this checks
+// the (distinct) cross-module-inliner defect #10554 fixes doesn't resurface
+// under a default export.
+export default function useDecl(x: unknown) {
+  return x === fnDecl;
+}

test-files/_helpers/fn_identity_10554/reexport.ts

@@ -0,0 +1,13 @@
+// Barrel re-export -- a THIRD view of the same bindings, once removed from
+// the declaring module.
+export {
+  fnDecl,
+  fnExpr,
+  arrowFn,
+  isSameDecl,
+  isSameExpr,
+  isSameArrow,
+  bothSame,
+  makeSet,
+} from "./lib.ts";
+export { default as useDeclDefault } from "./lib.ts";

test-files/test_gap_10554_fn_identity_own_module.ts

@@ -0,0 +1,50 @@
+// #10554: a function referenced inside its own module is a different
+// object from the same function imported elsewhere.
+import useDecl, {
+  fnDecl,
+  fnExpr,
+  arrowFn,
+  isSameDecl,
+  isSameExpr,
+  isSameArrow,
+  bothSame,
+  makeSet,
+} from "./_helpers/fn_identity_10554/lib.ts";
+import * as ns from "./_helpers/fn_identity_10554/lib.ts";
+import {
+  fnDecl as reFnDecl,
+  isSameDecl as reIsSameDecl,
+  useDeclDefault,
+} from "./_helpers/fn_identity_10554/reexport.ts";
+
+// 1. function declaration, function expression, arrow: in-module identity
+// checked from a call made through the IMPORTED value.
+console.log("decl:", isSameDecl(fnDecl));
+console.log("expr:", isSameExpr(fnExpr));
+console.log("arrow:", isSameArrow(arrowFn));
+
+// 2. Both directions: importer's namespace view vs named-import view vs
+// in-module (through the exported checker functions).
+console.log("ns decl:", isSameDecl(ns.fnDecl), ns.fnDecl === fnDecl, fnDecl === ns.fnDecl);
+console.log("ns expr:", isSameExpr(ns.fnExpr), ns.fnExpr === fnExpr);
+console.log("ns arrow:", isSameArrow(ns.arrowFn), ns.arrowFn === arrowFn);
+
+// 3. Re-export (barrel): a third view, once removed.
+console.log("reexport decl:", reIsSameDecl(reFnDecl), reFnDecl === fnDecl, isSameDecl(reFnDecl));
+
+// 4. default export whose body ALSO references an exported sibling by
+// value (distinct from #10434/#10548's export-row identity; exercises the
+// cross-module-inliner defect instead).
+console.log("default:", useDecl(fnDecl), useDeclDefault(fnDecl), useDecl === useDeclDefault);
+
+// 5. bothSame -- direct pass-through, no local materialization inside the
+// callee (a control: unaffected by this defect, should always have passed).
+console.log("bothSame decl:", bothSame(fnDecl, fnDecl), bothSame(fnDecl, ns.fnDecl));
+console.log("bothSame cross:", bothSame(fnDecl, fnExpr));
+
+// 6. Set membership -- identity through collection storage/lookup, built
+// FROM WITHIN the module (the same defect shape via a different value
+// consumer than `===`).
+const set = makeSet();
+console.log("set has:", set.has(fnDecl), set.has(fnExpr), set.has(arrowFn));
+console.log("set has via ns:", set.has(ns.fnDecl));