Skip to content
Closed
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
34 changes: 34 additions & 0 deletions changelog.d/native-instance-binding-scope-9847.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
**A native-instance tag created by a bare assignment now follows the binding it
assigns to, not the identifier's spelling** (#9847). One
`O = childProcess.spawn(...)` in a bundled helper used to type *every* binding
named `O` in the module as a `child_process` instance.

`lower_assign` registered the tag through `push_module_native_instance`, whose
key is the identifier text and whose scope is the whole module, and its
class-name match ends in a catch-all — so any method on any native module,
assigned to a variable, claimed that name for the rest of the program. Minified
bundles reuse single letters everywhere, so the collision is the normal case,
not a corner: `cli_2.1.112.js` (claude-code) compiles as one module, contains
`let O; try { O = fA1.spawn(z.file, z.args, z.options) }`, and binds the name
`O` 5,381 more times. Among them is the `for (let { segment: O } of ... )`
binding in `string-width`, which holds a grapheme **string**; its
`O.codePointAt(0)` lowered as
`NativeMethodCall { module: "child_process", class_name: Some("Instance") }`
and reached the right answer only because native-instance dispatch falls
through to a generic path on a string receiver — once per grapheme, in the
loop that dominates a claude-code turn.

The tag is now keyed on the `LocalId` the assignment target resolves to. That
is the same correction #7775 made for `new Proxy` bindings (`proxy_locals` →
`proxy_local_ids`) and it keeps the cross-function reach the module-wide table
existed for: a module-level `let client;` assigned inside `init()` and read
inside `handler()` resolves to the same binding in both. A target that resolves
to no local at all — a bare global — still falls back to the name-keyed table,
which is strictly no worse than the previous behaviour that used it for
everything.

This is a mis-typing fix, not a wrong-answer fix: the mislowered calls already
degraded gracefully. But they degraded through a dispatch that had no business
seeing them, and a `child_process` method table that ever gained a
`codePointAt`, `length` or `test` entry would have captured string calls
silently.
52 changes: 52 additions & 0 deletions crates/perry-hir/src/lower/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ impl LoweringContext {
namespace_vars: Vec::new(),
current_namespace: None,
module_native_instances: Vec::new(),
local_id_native_instances: HashMap::new(),
uses_fetch: false,
uses_webassembly: false,
react_default_import_local: None,
Expand Down Expand Up @@ -1638,10 +1639,41 @@ impl LoweringContext {
// zero-arg FFI calls.
.filter(|(_, module, class)| !exposes_plain_object_fields(module, class))
.map(|(_, module, class)| (module.as_str(), class.as_str()))
.or_else(|| {
// #9847: a bare assignment (`O = cp.spawn(...)`) tags the
// RESOLVED binding, not the spelling. Consulted before the
// name-keyed module-wide table below, so a same-named binding
// in another function is simply a different binding and cannot
// inherit the tag. A different binding has a different
// `LocalId`, so no scope-exit truncation is needed here.
//
// This arm sits on the miss path of every identifier property
// access, so short-circuit the common module that has no
// bare-assignment native handle at all before touching the
// locals index.
if self.local_id_native_instances.is_empty() {
return None;
}
let id = self.lookup_local(name)?;
self.local_id_native_instances
.get(&id)
.filter(|(module, class)| !exposes_plain_object_fields(module, class))
.map(|(module, class)| (module.as_str(), class.as_str()))
})
.or_else(|| {
// Check module-level instances (survive scope exits).
// Same last-match-wins rule for consistency — the index stores
// the LAST pushed entry per name.
//
// #9847 KNOWN HOLE, stated plainly (the same one #7775 left for
// proxies): a receiver that resolves to NO local — a bare
// global, or a module-level binding referenced from a function
// body lowered before that binding was pre-registered — is
// still answered by spelling alone. That arm is kept because
// dropping it would regress the genuine cross-function handles
// that reach the lowering only through a name, and it is
// strictly no worse than the pre-#9847 behaviour, which used it
// for every assignment.
self.module_native_instances_index
.get(name)
.map(|&idx| &self.module_native_instances[idx])
Expand Down Expand Up @@ -1684,6 +1716,26 @@ impl LoweringContext {
.insert(entry.0.clone(), idx);
self.module_native_instances.push(entry);
}

/// #9847: tag the RESOLVED binding `id` as holding a native instance.
///
/// Used by the bare-assignment path (`O = cp.spawn(...)`) in place of
/// `push_module_native_instance`, whose name key was module-wide: in a
/// minified single-module bundle a single native handle poisoned every
/// homonym in the program. Keyed on the `LocalId` the target resolves to,
/// this keeps the cross-function reach the module-wide table was there to
/// provide (a module-level `let client;` assigned inside `init()` and read
/// inside `handler()` resolves to the SAME id in both) while making a
/// same-named binding in another scope a different binding.
pub(crate) fn register_local_id_native_instance(
&mut self,
id: LocalId,
module_name: String,
class_name: String,
) {
self.local_id_native_instances
.insert(id, (module_name, class_name));
}
}

// Internal anchor — keeps the file's outer impl block intact while
Expand Down
47 changes: 42 additions & 5 deletions crates/perry-hir/src/lower/expr_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,48 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr)
_ => Some("Instance"),
};
if let Some(class_name) = class_name {
ctx.push_module_native_instance((
var_name.clone(),
module_name.to_string(),
class_name.to_string(),
));
// #9847: tag the BINDING this assigns
// to, not its spelling. The catch-all
// arm above makes any method on any
// native module tag the target, and
// `push_module_native_instance` keyed
// that on the identifier text for the
// whole module — so one
// `O = cp.spawn(...)` in a bundled
// helper typed all 5,381 bindings named
// `O` in claude-code's single-module
// bundle as `child_process::Instance`,
// including a `for (let {segment: O} of
// …)` binding holding a grapheme
// string. When the target resolves to a
// local we key on its `LocalId`, which
// keeps the cross-function reach the
// module-wide table existed for (a
// module-level `let client;` assigned
// inside one function and read inside
// another resolves to the same id in
// both) without the homonym collision.
// An unresolvable target — a bare
// global with no binding — has no id to
// key on and keeps the old name-keyed
// registration; see the matching arm in
// `lookup_native_instance`.
match ctx.lookup_local(&var_name) {
Some(local_id) => {
ctx.register_local_id_native_instance(
local_id,
module_name.to_string(),
class_name.to_string(),
);
}
None => {
ctx.push_module_native_instance((
var_name.clone(),
module_name.to_string(),
class_name.to_string(),
));
}
}
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions crates/perry-hir/src/lower/lowering_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,29 @@ pub struct LoweringContext {
pub(crate) current_namespace: Option<String>,
/// Module-level native instances that survive scope exits.
/// Used for variables assigned from native calls inside functions (e.g., `mongoClient = await MongoClient.connect(uri)`).
///
/// NAME-keyed and module-wide, so it is no longer authoritative for the
/// bare-assignment form: see `local_id_native_instances`, which supersedes
/// it whenever the assignment target resolves to a binding (#9847). This
/// stays the fallback for a target that resolves to no local at all.
pub(crate) module_native_instances: Vec<(String, String, String)>,
/// #9847: the RESOLVED bindings that hold a native instance produced by a
/// bare assignment (`O = cp.spawn(...)`), so "this binding is a
/// `child_process` handle" stops meaning "something in this module spells
/// it this way".
///
/// `module_native_instances` alone tagged the NAME for the whole module and
/// was never truncated, so one `O = cp.spawn(...)` in a bundled helper made
/// every other `O` in a 13 MB single-module bundle a
/// `child_process::Instance` — including a `for (let {segment: O} of …)`
/// binding holding a grapheme string, whose `O.codePointAt(0)` then lowered
/// as `NativeMethodCall{module:"child_process", class_name:Some("Instance")}`
/// and only worked because native-instance dispatch falls through to a
/// generic path on a string receiver. Same defect class as #7775's
/// `proxy_locals` → `proxy_local_ids`, and keyed the same way: a different
/// binding has a different `LocalId`, so a stale entry can never be reached
/// through a homonym, and no scope-exit truncation is needed.
pub(crate) local_id_native_instances: HashMap<LocalId, (String, String)>,
/// Whether this module uses fetch() — requires perry-stdlib
pub(crate) uses_fetch: bool,
/// Issue #76 — set when any `WebAssembly.*` HIR variant is lowered.
Expand Down
Loading
Loading