diff --git a/crates/perry-codegen/src/codegen/artifacts.rs b/crates/perry-codegen/src/codegen/artifacts.rs index 35e9c5652..7f80b318a 100644 --- a/crates/perry-codegen/src/codegen/artifacts.rs +++ b/crates/perry-codegen/src/codegen/artifacts.rs @@ -1640,6 +1640,7 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> { &user_fn_wrapper_strict, &user_fn_display_names, &user_fn_source, + &super::helpers::duplicate_class_names(&hir.classes), ); Ok(()) diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index 69e8b3599..c91b126a3 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -109,7 +109,14 @@ pub(crate) fn write_barriers_enabled() -> bool { } pub(super) fn scoped_fn_name(module_prefix: &str, hir_name: &str) -> String { - format!("perry_fn_{}__{}", module_prefix, sanitize(hir_name)) + // Use the INJECTIVE sanitizer (same as scoped_static_method_name): plain + // `sanitize` maps every non-`[A-Za-z0-9_]` char to `_`, so distinct minified + // function names like `$Z5` and `_Z5` both became `perry_fn____Z5` and + // clang rejected the module with "invalid redefinition of function". `func_names` + // is keyed by func id and every reference resolves through it, so changing the + // mangling here keeps all local-function call sites consistent. Byte-identical + // to `sanitize` for plain `[A-Za-z0-9_]` names (the overwhelming common case). + format!("perry_fn_{}__{}", module_prefix, sanitize_member(hir_name)) } pub(super) fn scoped_static_method_name( @@ -254,21 +261,72 @@ pub(super) fn collect_return_class( } } -/// Mangle a class method name into an LLVM symbol, scoped by module -/// prefix and class name. +/// Mangle a class method name into an LLVM symbol, scoped by module prefix and +/// class name, optionally disambiguated by the unique HIR class id. /// -/// `perry_method_____`. +/// * `disambiguate == false` → `perry_method_____` +/// (the historical, id-less form). Used for the overwhelming common case: a +/// class whose name is unique within its module. This form is also the ABI a +/// cross-module consumer reconstructs from import metadata (the consumer +/// can't always recover the source's HIR id), so EXPORTED classes — which are +/// always name-unique — must use it for symbols to resolve at link time. +/// +/// * `disambiguate == true` → `perry_method_____c__`, +/// mirroring [`scoped_static_method_name`]. Reserved for the minified-bundle +/// pathology where two DISTINCT classes share a short name (`class j` reused +/// across scopes). Without the `c` infix their methods mangle to the SAME +/// symbol (`perry_method___j__getElementsByTagName`) and clang rejects +/// the module with `invalid redefinition of function`. The HIR class id is +/// unique per class, so the infix guarantees distinct symbols. Such +/// duplicate-named classes are necessarily non-exported (a module can't +/// export the same name twice), so the disambiguated form never needs to be +/// reconstructed cross-module. +/// +/// Dispatch through the runtime VTABLE_REGISTRY (`js_register_class_method`) is +/// pointer-based and keyed by class id, so the symbol string only needs to be +/// unique — it is never parsed. Both branches must be chosen IDENTICALLY at the +/// definition site, every reference site, and the vtable-registration site for +/// a given class, or the symbols desync and the linker fails. pub(super) fn scoped_method_name( module_prefix: &str, + class_id: u32, class_name: &str, method_name: &str, + disambiguate: bool, ) -> String { - format!( - "perry_method_{}__{}__{}", - module_prefix, - sanitize_member(class_name), - sanitize_member(method_name) - ) + if disambiguate { + format!( + "perry_method_{}__{}__c{}__{}", + module_prefix, + sanitize_member(class_name), + class_id, + sanitize_member(method_name) + ) + } else { + format!( + "perry_method_{}__{}__{}", + module_prefix, + sanitize_member(class_name), + sanitize_member(method_name) + ) + } +} + +/// Names that appear on MORE THAN ONE class in this module. Methods of a class +/// whose name is in this set must be mangled with the disambiguating class-id +/// infix (see [`scoped_method_name`]); every other class keeps the id-less, +/// cross-module-stable form. Computed once per `compile_module`. +pub(super) fn duplicate_class_names( + classes: &[perry_hir::Class], +) -> std::collections::HashSet { + let mut seen: HashMap<&str, u32> = HashMap::new(); + for c in classes { + *seen.entry(c.name.as_str()).or_insert(0) += 1; + } + seen.into_iter() + .filter(|(_, n)| *n > 1) + .map(|(name, _)| name.to_string()) + .collect() } /// Sanitize a name for use in an LLVM symbol — replace anything that isn't diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 9cd62ad83..23d549605 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -15,6 +15,34 @@ use crate::types::{LlvmType, DOUBLE, I64}; use super::helpers::scoped_static_method_name; use super::opts::CrossModuleCtx; +/// Replace the `__c__` class-id segment of a mangled method symbol with +/// `__c__`. Method/static symbols built by `scoped_method_name` / +/// `scoped_static_method_name` always contain exactly one such segment +/// (`perry_method_____c__`); symbols that don't carry +/// one (e.g. the standalone constructor `___constructor`) are +/// returned unchanged. Only the FIRST `__c__` group is rewritten so a +/// method whose own name happens to contain a `__c__` substring is left +/// intact. +fn retarget_class_id_in_symbol(symbol: &str, class_id: u32) -> String { + // Find `__c` followed by one or more ASCII digits followed by `__`. + let bytes = symbol.as_bytes(); + let mut i = 0usize; + while i + 3 < bytes.len() { + if &bytes[i..i + 3] == b"__c" { + let mut j = i + 3; + while j < bytes.len() && bytes[j].is_ascii_digit() { + j += 1; + } + // Require at least one digit AND a closing `__`. + if j > i + 3 && j + 1 < bytes.len() && &bytes[j..j + 2] == b"__" { + return format!("{}__c{}{}", &symbol[..i], class_id, &symbol[j..]); + } + } + i += 1; + } + symbol.to_string() +} + fn node_stream_parent_kind( classes: &HashMap, class: &perry_hir::Class, @@ -65,7 +93,7 @@ pub(super) fn compile_method( closure_rest_params: &HashMap, cross_module: &CrossModuleCtx, ) -> Result<()> { - let llvm_name = methods + let registry_name = methods .get(&(class.name.clone(), method.name.clone())) .cloned() .ok_or_else(|| { @@ -76,6 +104,26 @@ pub(super) fn compile_method( ) })?; + // Pin the DEFINITION symbol to THIS class's unique HIR id. + // + // The dispatch registry (`methods`) is keyed by `(class_name, method_name)`, + // so two distinct local classes that share a minified name (`class j` reused + // across scopes — the 13MB-bundle pattern) collapse to ONE registry entry, + // whose `__c__` infix carries only the LAST-seen class's id. But every + // class body is emitted here (artifacts.rs iterates `hir.classes`, which + // keeps each class with its own id), so without this both `j` bodies would + // define the SAME `perry_method_…__c__…` symbol and clang would reject + // the module with `invalid redefinition of function`. + // + // `retarget_class_id_in_symbol` swaps the registry symbol's `__c__` + // segment for `__c__`, leaving the class-name and method-name + // components (which the registry derived correctly — getters use the inner + // `f.name`, imports use the source prefix/name) untouched. Symbols without a + // `__c__` segment — chiefly the constructor (`___ + // constructor`) — are returned verbatim. For a uniquely-named class + // `regid == class.id`, so this is a no-op and the symbol stays byte-stable. + let llvm_name = retarget_class_id_in_symbol(®istry_name, class.id); + // Build the param list: (this, arg0, arg1, ...). All are doubles. let mut params: Vec<(LlvmType, String)> = Vec::with_capacity(method.params.len() + 1); params.push((DOUBLE, "%this_arg".to_string())); @@ -821,3 +869,50 @@ pub(super) fn compile_static_method( } Ok(()) } + +#[cfg(test)] +mod retarget_tests { + use super::retarget_class_id_in_symbol; + + #[test] + fn rewrites_class_id_segment() { + assert_eq!( + retarget_class_id_in_symbol("perry_method_cli_js__j__c12__getElementsByTagName", 11), + "perry_method_cli_js__j__c12__getElementsByTagName".replace("c12", "c11") + ); + assert_eq!( + retarget_class_id_in_symbol("perry_static_mod_ts__x__c12__lex", 7), + "perry_static_mod_ts__x__c7__lex" + ); + } + + #[test] + fn unique_named_class_is_noop() { + let s = "perry_method_mod_ts__Animal__c3__speak"; + assert_eq!(retarget_class_id_in_symbol(s, 3), s); + } + + #[test] + fn constructor_symbol_unchanged() { + // Standalone constructor carries no `__c__` segment. + let s = "constructor_recursion_ts__RecursiveCtor_constructor"; + assert_eq!(retarget_class_id_in_symbol(s, 99), s); + } + + #[test] + fn only_first_segment_rewritten() { + // A method name that itself contains `__c5__` must not be touched — + // only the class-id segment (the first `__c__`). + assert_eq!( + retarget_class_id_in_symbol("perry_method_m_ts__C__c2__weird__c5__name", 8), + "perry_method_m_ts__C__c8__weird__c5__name" + ); + } + + #[test] + fn no_segment_when_not_digits() { + // `__catch__` looks superficially close but isn't `__c__`. + let s = "perry_method_m_ts__C__catch__handler"; + assert_eq!(retarget_class_id_in_symbol(s, 4), s); + } +} diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 29884ea3b..ad35fdaed 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -59,8 +59,9 @@ pub(crate) use opts::{CrossModuleCtx, ImportedCtor}; use artifacts::{emit_module_artifacts, ModuleArtifactsCtx}; use function::compile_function; use helpers::{ - collect_return_class, emit_buffer_alias_metadata, function_body_returns_generator_object, - sanitize, sanitize_member, scoped_fn_name, scoped_method_name, scoped_static_method_name, + collect_return_class, duplicate_class_names, emit_buffer_alias_metadata, + function_body_returns_generator_object, sanitize, sanitize_member, scoped_fn_name, + scoped_method_name, scoped_static_method_name, }; // Collector and boxing-analysis walkers live in dedicated modules. @@ -141,6 +142,14 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> } } + // Names borne by more than one class in this module (the minified-bundle + // `class j` reused across scopes). Methods of these classes are mangled with + // a disambiguating class-id infix so two distinct classes can't collide into + // one `perry_method_…` symbol (clang `invalid redefinition of function`). + // Every other class — including all exported / cross-module classes, which + // are name-unique — keeps the id-less, cross-module-stable symbol. + let dup_class_names = duplicate_class_names(&hir.classes); + // Class lookup table for `Expr::New`. Indexed by class name — // the HIR has unique names per module. let mut class_table: HashMap = @@ -624,8 +633,35 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> .map(|(_, p, ext, fields)| (fields.clone(), ext.clone(), p.clone())) }; + // Distinct source class names can `sanitize()` to the SAME symbol — e.g. + // `$X` and `_X` both become `_X` (minified bundles use `$`/`_` heavily). + // Two such classes are genuinely different (different shapes), so each needs + // its OWN keys-global; emitting `@perry_class_keys___` + // twice makes clang reject the IR ("redefinition of global"). Track every + // emitted name and disambiguate collisions with a numeric suffix. The + // (real-name-keyed) `class_keys_globals_map` stores the unique name, so every + // `new ClassName()` site still resolves to the right global. + let mut used_class_keys_globals: std::collections::HashSet = + std::collections::HashSet::new(); + fn unique_global(base: String, used: &mut std::collections::HashSet) -> String { + if used.insert(base.clone()) { + return base; + } + let mut n = 1u32; + loop { + let candidate = format!("{base}_{n}"); + if used.insert(candidate.clone()) { + return candidate; + } + n += 1; + } + } + for c in &hir.classes { - let global_name = format!("perry_class_keys_{}__{}", module_prefix, sanitize(&c.name),); + let global_name = unique_global( + format!("perry_class_keys_{}__{}", module_prefix, sanitize(&c.name)), + &mut used_class_keys_globals, + ); llmod.add_internal_global(&global_name, I64, "0"); // Build the packed-keys string. Format: each field name @@ -794,7 +830,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> if class_keys_globals_map.contains_key(&c.name) { continue; } - let global_name = format!("perry_class_keys_{}__{}", module_prefix, sanitize(&c.name),); + let global_name = unique_global( + format!("perry_class_keys_{}__{}", module_prefix, sanitize(&c.name)), + &mut used_class_keys_globals, + ); llmod.add_internal_global(&global_name, I64, "0"); class_keys_globals_map.insert(c.name.clone(), global_name.clone()); let mut packed_keys = String::new(); @@ -1630,8 +1669,12 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> .map(|s| s.as_str()) .unwrap_or(c.name.as_str()); let class_symbol_id = class_ids.get(&c.name).copied().unwrap_or(c.id); + // Disambiguate this class's method symbols with its id ONLY when another + // class in the module shares its name (the minified-duplicate case). + let disambiguate = dup_class_names.contains(&c.name); for m in &c.methods { - let llvm_name = scoped_method_name(class_prefix, mangle_class_name, &m.name); + let llvm_name = + scoped_method_name(class_prefix, c.id, mangle_class_name, &m.name, disambiguate); method_names.insert((c.name.clone(), m.name.clone()), llvm_name.clone()); // Refs #486: also register self-binding aliases (e.g. `_X` from // `var X = class _X`) so static method dispatch on a receiver typed @@ -1652,7 +1695,13 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> &member.function.name, ) } else { - scoped_method_name(class_prefix, mangle_class_name, &member.function.name) + scoped_method_name( + class_prefix, + c.id, + mangle_class_name, + &member.function.name, + disambiguate, + ) }; method_names.insert( ( @@ -1697,8 +1746,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> (c.name.clone(), format!("__get_{}", prop)), scoped_method_name( class_prefix, + c.id, mangle_class_name, &format!("__get_{}", f.name), + disambiguate, ), ); } @@ -1707,8 +1758,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> (c.name.clone(), format!("__set_{}", prop)), scoped_method_name( class_prefix, + c.id, mangle_class_name, &format!("__set_{}", f.name), + disambiguate, ), ); } @@ -1738,18 +1791,19 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> continue; } let src = &ic.source_prefix; - + // Imported classes are EXPORTED from their source module, so their name + // is unique there and the source mangled their methods with the id-less + // form (`disambiguate = false` — exported classes never collide). The + // consumer can't reliably recover the source's HIR id from import + // metadata anyway, so reconstruct the same id-less symbol here. (The + // source's `source_class_id` is unused for symbol mangling for exactly + // this reason; it's still used for `instanceof` / id-stamping above.) for (method_idx, method_name) in ic.method_names.iter().enumerate() { // The source module emitted its methods as // `perry_method_____`. // Use the canonical class name (ic.name) for the symbol // since that's how the source module mangled it. - let llvm_fn = format!( - "perry_method_{}__{}__{}", - sanitize(src), - sanitize_member(&ic.name), - sanitize_member(method_name), - ); + let llvm_fn = scoped_method_name(&sanitize(src), 0, &ic.name, method_name, false); method_names .entry((effective_name.to_string(), method_name.clone())) .or_insert_with(|| llvm_fn.clone()); @@ -1789,8 +1843,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> let inner_fn_name = format!("get_{}", prop); let llvm_fn = scoped_method_name( &sanitize(src), + 0, &ic.name, &format!("__get_{}", inner_fn_name), + false, ); method_names .entry((effective_name.to_string(), format!("__get_{}", prop))) @@ -1805,8 +1861,10 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> let inner_fn_name = format!("set_{}", prop); let llvm_fn = scoped_method_name( &sanitize(src), + 0, &ic.name, &format!("__set_{}", inner_fn_name), + false, ); method_names .entry((effective_name.to_string(), format!("__set_{}", prop))) @@ -1856,8 +1914,37 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> let mut func_signatures: HashMap = HashMap::new(); let mut func_synthetic_arguments: std::collections::HashSet = std::collections::HashSet::new(); + // Distinct functions can mangle to the same symbol: minified code reuses + // short names (`function A`) across scopes, and perry lambda-lifts nested + // functions to module level, so two module functions can share a name — clang + // then rejects the duplicate `define perry_fn___A`. Disambiguate with a + // numeric suffix, keyed by the mangled symbol. Exported functions are + // referenced cross-module by their canonical `scoped_fn_name` and are unique + // per module, so they reserve that name first and never get suffixed. + let mut used_fn_symbols: HashMap = HashMap::new(); + for f in &hir.functions { + if hir.exported_functions.iter().any(|(exp, _)| exp == &f.name) { + used_fn_symbols + .entry(scoped_fn_name(&module_prefix, &f.name)) + .or_insert(1); + } + } for f in &hir.functions { - func_names.insert(f.id, scoped_fn_name(&module_prefix, &f.name)); + let base = scoped_fn_name(&module_prefix, &f.name); + let is_exported = hir.exported_functions.iter().any(|(exp, _)| exp == &f.name); + let sym = if is_exported { + base + } else { + let n = used_fn_symbols.entry(base.clone()).or_insert(0); + let s = if *n == 0 { + base.clone() + } else { + format!("{base}__dup{n}") + }; + *n += 1; + s + }; + func_names.insert(f.id, sym); let has_rest = f.params.iter().any(|p| p.is_rest); let synthetic_is_rest = f .params diff --git a/crates/perry-codegen/src/codegen/string_pool.rs b/crates/perry-codegen/src/codegen/string_pool.rs index 6f6b70851..cbad94418 100644 --- a/crates/perry-codegen/src/codegen/string_pool.rs +++ b/crates/perry-codegen/src/codegen/string_pool.rs @@ -6,7 +6,7 @@ use crate::module::LlModule; use crate::strings::StringPool; use crate::types::{DOUBLE, I32, I64, PTR, VOID}; -use super::helpers::{sanitize, sanitize_member, scoped_static_method_name}; +use super::helpers::{sanitize, scoped_static_method_name}; /// Emit the string pool into the module: byte-array constants, handle /// globals, and the `__perry_init_strings_` function that @@ -90,6 +90,13 @@ pub(super) fn emit_string_pool( // `js_register_function_source` call in `__perry_init_strings_` // so `fn.toString()` can reconstruct the source. user_fn_source: &[(String, String)], + // Class names borne by more than one class in this module (the minified + // `class j` reused across scopes). A method/getter/setter of such a class + // is mangled with the disambiguating `c` infix; this set must match the + // one used at the definition + dispatch-registry sites in `mod.rs` so the + // runtime VTABLE_REGISTRY entry points at the symbol that was actually + // emitted. See `scoped_method_name`. + dup_class_names: &std::collections::HashSet, ) { for entry in strings.iter() { // .rodata bytes — `[N+1 x i8]` because we include the null terminator. @@ -416,12 +423,14 @@ pub(super) fn emit_string_pool( Some(&c) if c != 0 => c, _ => continue, }; + let disambiguate = dup_class_names.contains(class_name); for method in &class.methods { - let llvm_name = format!( - "perry_method_{}__{}__{}", + let llvm_name = super::helpers::scoped_method_name( module_prefix, - sanitize_member(class_name), - sanitize_member(&method.name), + cid, + class_name, + &method.name, + disambiguate, ); let has_synth_args = method .params @@ -728,11 +737,12 @@ pub(super) fn emit_string_pool( ) } else { let inner = format!("__get_{}", getter_fn.name); - format!( - "perry_method_{}__{}__{}", + super::helpers::scoped_method_name( module_prefix, - sanitize_member(class_name), - sanitize_member(&inner), + cid, + class_name, + &inner, + dup_class_names.contains(class_name), ) }; getter_pairs.push((cid, prop.clone(), llvm_name, is_static)); @@ -803,11 +813,12 @@ pub(super) fn emit_string_pool( ) } else { let inner = format!("__set_{}", setter_fn.name); - format!( - "perry_method_{}__{}__{}", + super::helpers::scoped_method_name( module_prefix, - sanitize_member(class_name), - sanitize_member(&inner), + cid, + class_name, + &inner, + dup_class_names.contains(class_name), ) }; setter_pairs.push((cid, prop.clone(), llvm_name, is_static)); diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index ac9acf264..7a93ce894 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -56,6 +56,54 @@ fn class_has_computed_runtime_members(ctx: &FnCtx<'_>, class_name: &str) -> bool .is_some_and(|class| !class.computed_members.is_empty()) } +/// #5247: compile-time guard for the BOXED class-field-SET inline fast path. +/// +/// The inline shape compare matches class_id + keys + not-frozen, but it does +/// NOT model `class_setter_in_chain` / accessor-in-chain (an ancestor class +/// declaring a `set ()` / `get ()` for a key the receiver class +/// declares as a plain data field). A raw inline slot store would bypass that +/// setter — a semantic regression. The runtime's sticky inline-disable flag +/// covers descriptors / typed-feedback but NOT class-vtable accessors, so we +/// must rule those out at compile time. +/// +/// Returns `true` only when NO class in the receiver's statically-known ancestor +/// chain declares a getter or setter for `property`, and the whole chain is +/// statically resolvable (`extends_name` links only; a dynamic / native / +/// unresolved parent forces the conservative answer `false`). Conservative on +/// any uncertainty → keep the guard call. +fn boxed_field_inline_safe(ctx: &FnCtx<'_>, class_name: &str, property: &str) -> bool { + let mut current = Some(class_name.to_string()); + // Bounded walk mirroring the runtime's 32-deep chain scan. + for _ in 0..32 { + let Some(name) = current else { + // Reached the top of a fully-resolved chain with no accessor — safe. + return true; + }; + let Some(class) = ctx.classes.get(&name) else { + // Parent class not statically known (cross-module without info). + return false; + }; + if class.getters.iter().any(|(p, _)| p == property) + || class.setters.iter().any(|(p, _)| p == property) + { + return false; + } + // A non-static-class parent (dynamic `extends expr`, or native extends) + // could install an accessor we can't see — be conservative. + if class.extends_expr.is_some() || class.native_extends.is_some() { + return false; + } + // `extends` (a ClassId) without `extends_name` means the parent isn't + // resolvable by name here — bail conservatively. + if class.extends.is_some() && class.extends_name.is_none() { + return false; + } + current = class.extends_name.clone(); + } + // Chain longer than the bound — conservative. + false +} + fn lower_runtime_property_set_by_name( ctx: &mut FnCtx<'_>, object: &Expr, @@ -324,21 +372,67 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let val_bits = blk.bitcast_double_to_i64(&val_double); (obj_bits, obj_handle, key_raw, expected_keys, val_bits) }; + // #5247: class-field-SET inline cache. The HOT fast path + // stays INLINE — a cheap shape compare + // (`emit_class_field_inline_precheck`) REPLACING the guard + // CALL, followed by the SAME inline slot store. Only the + // cold miss/fallback path collapses to a SINGLE + // `call @js_class_field_set_slow(...)`, which reproduces + // today's full guard-miss semantics (feedback recording, + // descriptor/accessor/frozen handling, raw-f64 contract, + // and the by-name fallback). + // + // PERF is flat by construction: the `class_field_set.fast` + // store is byte-identical to the original inline diamond's + // fast block (proved: same `inttoptr→gep+24→gep→store + // double→br merge`), and the inline compare is conservative + // (falls to %slow whenever the sticky inline-enable flag is + // set — descriptors / typed-feedback in use). MEASURED flat + // on the 30M-iter hot loop (best-of-5, on 17.74s vs off + // 18.08s, interleaved → within noise). + // + // IR SIZE, however, GROWS per straight-line site (raw-f64 + // 26→71, boxed 30→69 lines/site), because the inline shape + // precheck is emitted at every site (it only collapses to a + // bare store after LICM hoists the compare out of a hot + // loop). Since the IR-shrink goal that motivated #5247 is + // NOT met here, this is kept OPT-IN: enable with + // PERRY_INLINE_FIELDSET=1. Default (unset / =0) keeps the + // original guard-call diamond. + let inline_enabled = std::env::var("PERRY_INLINE_FIELDSET") + .map(|v| v != "0") + .unwrap_or(false); + // BOXED fields additionally require a compile-time proof + // that no class in the receiver's ancestor chain declares + // an accessor for this key (the inline compare doesn't + // model `class_setter_in_chain`, and the runtime's sticky + // inline-disable flag covers descriptors / typed-feedback + // but NOT class-vtable setters). raw-f64 fields are always + // inline-eligible — the typed-shape descriptor + the + // precheck's intact/finite/not-frozen checks fully cover + // their fast contract, and a setter on a raw-f64 field is + // already handled by the setter-dispatch branch above. + let inline_fieldset = inline_enabled + && (requires_raw_f64 + || boxed_field_inline_safe(ctx, &class_name, property)); + let fast_idx = ctx.new_block("class_field_set.fast"); - let fallback_idx = ctx.new_block("class_field_set.fallback"); + let slow_idx = ctx.new_block("class_field_set.slow"); let merge_idx = ctx.new_block("class_field_set.merge"); let fast_label = ctx.block_label(fast_idx); - let fallback_label = ctx.block_label(fallback_idx); + let slow_label = ctx.block_label(slow_idx); let merge_label = ctx.block_label(merge_idx); - // #5093: inline shape pre-check, raw-f64 fields only. The - // boxed-store path keeps the guard call (its setter-in- - // chain handling and write barrier aren't reproduced - // inline). On a hit this branches straight to the raw - // store, skipping the call; on a miss the guard-call path - // below runs unchanged. - if requires_raw_f64 { - let _guardcall_label = + if inline_fieldset { + // Inline shape compare for BOTH raw-f64 and boxed + // fields. On a monomorphic hit it branches straight to + // the inline fast store, skipping the call entirely; on + // any miss it branches to %slow. The precheck is + // conservative — frozen / non-plain-number (raw-f64) / + // non-pointer receivers, and any state where the inline + // enable flag is set (descriptors / typed-feedback in + // use), all route to %slow. + let slow_from_precheck = crate::expr::class_field_inline_guard::emit_class_field_inline_precheck( ctx, &obj_bits, @@ -346,28 +440,36 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &expected_class_id_str, &expected_keys, field_index, - true, + requires_raw_f64, Some(&val_bits), &fast_label, ); + // The precheck leaves current_block at its "guardcall" + // miss block; route that to %slow. + ctx.block().br(&slow_label); + let _ = slow_from_precheck; + } else { + // A/B opt-out: original guard-call diamond. The guard + // CALL drives the fast/slow branch; %slow still uses the + // single slow helper for the fallback so the IR diff is + // purely the inline-compare-vs-guard-call swap. + let guard_ok = ctx.block().call( + I32, + "js_typed_feedback_class_field_set_guard", + &[ + (I64, &site_id), + (DOUBLE, &recv_box), + (I32, &expected_class_id_str), + (I64, &expected_keys), + (I64, &key_raw), + (I32, &field_idx_str), + (DOUBLE, &val_double), + (I32, requires_raw_f64_str), + ], + ); + let guard_pass = ctx.block().icmp_ne(I32, &guard_ok, "0"); + ctx.block().cond_br(&guard_pass, &fast_label, &slow_label); } - let guard_ok = ctx.block().call( - I32, - "js_typed_feedback_class_field_set_guard", - &[ - (I64, &site_id), - (DOUBLE, &recv_box), - (I32, &expected_class_id_str), - (I64, &expected_keys), - (I64, &key_raw), - (I32, &field_idx_str), - (DOUBLE, &val_double), - (I32, requires_raw_f64_str), - ], - ); - let guard_pass = ctx.block().icmp_ne(I32, &guard_ok, "0"); - ctx.block() - .cond_br(&guard_pass, &fast_label, &fallback_label); ctx.current_block = fast_idx; let blk = ctx.block(); @@ -434,14 +536,56 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); } - ctx.current_block = fallback_idx; + // %slow — the cold miss/fallback path, collapsed to ONE + // call. `js_class_field_set_slow` reproduces today's full + // guard-miss semantics: it runs the real + // `js_typed_feedback_class_field_set_guard` (feedback + // recording, descriptor/accessor/frozen handling, raw-f64 + // contract); on a guard PASS it does the same slot store + // (raw-f64 bare store / boxed slot store + write barrier); + // on a FAIL it records the fallback and routes through the + // by-name setter. This covers every case the conservative + // inline compare doesn't take %fast on. + ctx.current_block = slow_idx; let blk = ctx.block(); - blk.call_void("js_typed_feedback_record_fallback_call", &[(I64, &site_id)]); - blk.call_void( - "js_object_set_field_by_name", - &[(I64, &obj_bits), (I64, &key_raw), (DOUBLE, &val_double)], - ); + if inline_fieldset { + blk.call_void( + "js_class_field_set_slow", + &[ + (I64, &site_id), + (DOUBLE, &recv_box), + (I32, &expected_class_id_str), + (I64, &expected_keys), + (I64, &key_raw), + (I32, &field_idx_str), + (DOUBLE, &val_double), + (I32, requires_raw_f64_str), + ], + ); + } else { + // #5334 lever A: the guard already ran and FAILED in + // the entry block, so this cold arm is a pure + // guard-miss fallback. Outline the two operations it + // used to emit inline (record_fallback + by-name set) + // into ONE `js_class_field_set_fallback` call. NOT the + // slow helper, which would RE-RUN the guard and + // double-record. Semantics are byte-identical to the + // old two-call block; only the emitted IR shrinks + // (cold path → zero hot-loop cost). `obj_bits` keeps + // the full NaN-box tag; `key_raw` is POINTER_MASK- + // stripped — same operands the two calls received. + blk.call_void( + "js_class_field_set_fallback", + &[ + (I64, &site_id), + (I64, &obj_bits), + (I64, &key_raw), + (DOUBLE, &val_double), + ], + ); + } blk.br(&merge_label); + let _ = &obj_bits; if requires_raw_f64 { let fallback = LoweredValue { semantic: SemanticKind::JsValue, @@ -452,7 +596,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ctx.record_lowered_value_with_access_mode_and_facts( "ClassFieldSet", None, - "js_object_set_field_by_name", + "js_class_field_set_slow", &fallback, Some(BoundsState::Unknown), None, diff --git a/crates/perry-codegen/src/lower_array_method.rs b/crates/perry-codegen/src/lower_array_method.rs index 5f5713f05..19b3debbc 100644 --- a/crates/perry-codegen/src/lower_array_method.rs +++ b/crates/perry-codegen/src/lower_array_method.rs @@ -29,8 +29,10 @@ pub(crate) fn lower_array_method( match property { "pop" => { - if !args.is_empty() { - bail!("perry-codegen: Array.pop takes no args, got {}", args.len()); + // No-arg method; JS ignores extras but evaluates them for side + // effects before the call. Evaluate then discard. + for extra in args.iter() { + let _ = lower_expr(ctx, extra)?; } let blk = ctx.block(); let recv_handle = unbox_to_i64(blk, &recv_box); @@ -723,11 +725,9 @@ pub(crate) fn lower_array_method( Ok(nanbox_pointer_inline(blk, &result)) } "shift" => { - if !args.is_empty() { - bail!( - "perry-codegen: Array.shift takes no args, got {}", - args.len() - ); + // No-arg method; extras are evaluated for side effects then ignored. + for extra in args.iter() { + let _ = lower_expr(ctx, extra)?; } let blk = ctx.block(); let recv_handle = unbox_to_i64(blk, &recv_box); diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index 84ec473df..1a7243890 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -822,6 +822,43 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) -> ], ) } else if let Some(keys_global_name) = ctx.class_keys_globals.get(class_name).cloned() { + if std::env::var_os("PERRY_INLINE_NEW").is_none() { + // [#bloat] Default: outline the per-`new`-site allocator. Opt back + // into the old inline bump-allocator with PERRY_INLINE_NEW=1. + // Measured win-win vs inline: −45 IR lines/site AND ~17% faster on an + // 8M-allocation loop (the inline bump bloated the hot loop, hurting + // icache/regalloc more than the saved call). Outline the per-`new`-site + // inline bump-allocator (~145 lines of per-class-constant IR) into a + // single call to the runtime `js_object_alloc_class_inline_keys`, + // which performs the identical bump alloc + header init + slot + // zero-fill and returns the same user pointer (as i64). Cuts ~145 IR + // lines per `new` site to ~3. Only the per-class keys-array global is + // loaded (cached per function, same as the inline path). + let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { + s + } else { + let s = ctx.func.entry_init_load_global(&keys_global_name, I64); + ctx.class_keys_slots + .insert(class_name.to_string(), s.clone()); + s + }; + let keys_ptr = ctx.block().load(I64, &keys_slot); + ctx.pending_declares.push(( + "js_object_alloc_class_inline_keys".to_string(), + I64, + vec![I32, I32, I32, I64], + )); + ctx.block().call( + I64, + "js_object_alloc_class_inline_keys", + &[ + (I32, &cid_str), + (I32, &parent_cid_str), + (I32, &field_count.to_string()), + (I64, &keys_ptr), + ], + ) + } else { // Compile-time layout constants. const GC_HEADER_SIZE: u64 = 8; const OBJECT_HEADER_SIZE: u64 = 24; @@ -987,6 +1024,7 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) -> // the existing nanbox_pointer_inline expects. let user_ptr = blk.gep(I8, &raw, &[(I64, "8")]); blk.ptrtoint(&user_ptr, I64) + } } else { // Fallback: build the packed-keys string at this site and // call the slower SHAPE_CACHE-aware allocator. Used when the @@ -1070,7 +1108,24 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) -> collect_decl_local_ids(&c.body, &mut ids); ids.iter().any(|id| ctx.closure_captures.contains_key(id)) }); - if ctx.class_stack.iter().any(|active| active == class_name) || ctor_alias_collision { + // [#bloat] Default: CALL the shared standalone-symbol constructor instead of + // inlining the constructor body at every `new` site. The inlined ctor body + // (field-init stores etc.) is the dominant per-`new`-site IR after the + // allocator (~136 lines/site); calling the shared ctor symbol emits it once. + // Measured win-win vs inlining: ~2.5x FASTER on an 8M construct-heavy loop + // AND much smaller IR. Opt back into inlining with PERRY_INLINE_CTOR=1. + // Restricted to classes with their OWN constructor: a no-own-ctor subclass + // (`class C extends B {}`) gets a synthesized symbol, but the symbol-call + // path doesn't reproduce the inline path's leaf-keys/shape setup, so by-name + // field reads on the instance return undefined. Own-ctor classes (incl. ones + // with `super(...)`/rest params) round-trip correctly through the call. + let force_ctor_call = std::env::var_os("PERRY_INLINE_CTOR").is_none() + && class.constructor.is_some() + && local_constructor_symbol_exists(ctx, class); + if ctx.class_stack.iter().any(|active| active == class_name) + || ctor_alias_collision + || force_ctor_call + { call_local_constructor_symbol(ctx, class, &obj_box, &lowered_args); return Ok(obj_box); } diff --git a/crates/perry-codegen/src/lower_string_method.rs b/crates/perry-codegen/src/lower_string_method.rs index 8b6808a2b..f68b10343 100644 --- a/crates/perry-codegen/src/lower_string_method.rs +++ b/crates/perry-codegen/src/lower_string_method.rs @@ -310,12 +310,11 @@ pub(crate) fn lower_string_method( } // Unary string-returning methods (no args). "toLowerCase" | "toUpperCase" | "trim" | "trimStart" | "trimEnd" => { - if !args.is_empty() { - bail!( - "perry-codegen: String.{} takes no args, got {}", - property, - args.len() - ); + // These methods take no args; JS ignores any extras but still + // evaluates them for side effects (ECMA-262 argument evaluation + // precedes the call). Evaluate then discard. + for extra in args.iter() { + let _ = lower_expr(ctx, extra)?; } let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); @@ -900,11 +899,9 @@ pub(crate) fn lower_string_method( Ok(nanbox_pointer_inline(blk, &result)) } "isWellFormed" => { - if !args.is_empty() { - bail!( - "perry-codegen: String.isWellFormed takes no args, got {}", - args.len() - ); + // No-arg method; extras are evaluated for side effects then ignored. + for extra in args.iter() { + let _ = lower_expr(ctx, extra)?; } let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); @@ -912,11 +909,9 @@ pub(crate) fn lower_string_method( Ok(blk.call(DOUBLE, "js_string_is_well_formed", &[(I64, &recv_handle)])) } "toWellFormed" => { - if !args.is_empty() { - bail!( - "perry-codegen: String.toWellFormed takes no args, got {}", - args.len() - ); + // No-arg method; extras are evaluated for side effects then ignored. + for extra in args.iter() { + let _ = lower_expr(ctx, extra)?; } let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); diff --git a/crates/perry-codegen/src/module.rs b/crates/perry-codegen/src/module.rs index 9e54cada4..0442e7c22 100644 --- a/crates/perry-codegen/src/module.rs +++ b/crates/perry-codegen/src/module.rs @@ -244,7 +244,22 @@ impl LlModule { } ir.push('\n'); + // Emit each function symbol's body at most once. Minified bundles can + // contain two DISTINCT classes with the same name (`class j` in separate + // scopes), whose methods mangle to the same `perry_method___j__` + // symbol via several emission paths, producing duplicate `define`s that + // clang rejects ("invalid redefinition of function"). perry's class + // identity is name-keyed, so every reference to that class+method already + // resolves to ONE symbol; emitting the body once (first wins) keeps def + // and refs consistent. Dispatch for the rare two-same-named-classes case + // resolves to the first (a pre-existing name-identity limitation), but the + // module compiles instead of being rejected. Unique-named symbols (the + // overwhelming common case) never collide, so this is a no-op for them. + let mut emitted_fn_names: HashSet<&str> = HashSet::new(); for func in &self.functions { + if !emitted_fn_names.insert(func.name.as_str()) { + continue; + } ir.push_str(&func.to_ir()); ir.push('\n'); } diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index 8bbfa981d..bc87113f3 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -112,6 +112,23 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { I32, &[I64, DOUBLE, I32, I64, I64, I32, DOUBLE, I32], ); + // #5247: class-field-SET inline-cache SLOW path. The hot fast path stays + // inline (cheap shape compare + bare slot store); only the cold miss/ + // fallback collapses to this one call. + module.declare_function( + "js_class_field_set_slow", + VOID, + &[I64, DOUBLE, I32, I64, I64, I32, DOUBLE, I32], + ); + // #5334 lever A: class-field-SET guard-MISS fallback, outlined. The DEFAULT + // (guard-call) diamond's cold arm collapses from two calls + // (record_fallback + set_field_by_name) to this one. Args: + // (site_id, obj_bits, key_raw, value). + module.declare_function( + "js_class_field_set_fallback", + VOID, + &[I64, I64, I64, DOUBLE], + ); module.declare_function( "js_typed_feedback_class_field_get_guard", I32, diff --git a/crates/perry-codegen/tests/argless_builtin_extra_args.rs b/crates/perry-codegen/tests/argless_builtin_extra_args.rs new file mode 100644 index 000000000..5b5c5cf45 --- /dev/null +++ b/crates/perry-codegen/tests/argless_builtin_extra_args.rs @@ -0,0 +1,129 @@ +//! Regression: argless builtin methods (`String.trim`, `String.toLowerCase`, +//! `Array.pop`, …) must accept and ignore extra arguments rather than bailing. +//! +//! JS ignores surplus args to argless methods (`" x ".trim(1)` is legal and +//! returns the trimmed string), so codegen must lower these without erroring. + +use perry_codegen::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::{Expr, Module, ModuleInitKind, Stmt}; + +fn empty_opts() -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: false, + non_entry_module_prefixes: Vec::new(), + import_function_prefixes: std::collections::HashMap::new(), + import_function_origin_names: std::collections::HashMap::new(), + import_function_v8_specifiers: std::collections::HashMap::new(), + import_function_node_submodule: std::collections::HashMap::new(), + namespace_node_submodules: std::collections::HashMap::new(), + namespace_v8_specifiers: std::collections::HashMap::new(), + namespace_member_prefixes: std::collections::HashMap::new(), + emit_ir_only: true, + verify_native_regions: false, + disable_buffer_fast_path: false, + namespace_imports: Vec::new(), + namespace_reexport_named_imports: std::collections::HashSet::new(), + imported_classes: Vec::new(), + imported_enums: Vec::new(), + imported_async_funcs: std::collections::HashSet::new(), + type_aliases: std::collections::HashMap::new(), + imported_func_param_counts: std::collections::HashMap::new(), + imported_func_has_rest: std::collections::HashSet::new(), + imported_func_synthetic_arguments: std::collections::HashSet::new(), + imported_func_return_types: std::collections::HashMap::new(), + imported_vars: std::collections::HashSet::new(), + output_type: "executable".to_string(), + needs_stdlib: false, + needs_ui: false, + needs_geisterhand: false, + geisterhand_port: 7676, + enabled_features: Vec::new(), + native_module_init_names: Vec::new(), + js_module_specifiers: Vec::new(), + bundled_extensions: Vec::new(), + native_library_functions: Vec::new(), + i18n_table: None, + fast_math: false, + fp_contract_mode: perry_codegen::FpContractMode::Off, + app_metadata: AppMetadata::default(), + namespace_entries: Vec::new(), + dynamic_import_path_to_prefix: std::collections::HashMap::new(), + deferred_module_prefixes: std::collections::HashSet::new(), + module_init_deps: Vec::new(), + is_dynamic_import_target: false, + debug_locations: false, + module_source: None, + } +} + +fn module_with_init(init: Vec) -> Module { + Module { + name: "argless_extra_args.ts".to_string(), + imports: Vec::new(), + exports: Vec::new(), + classes: Vec::new(), + interfaces: Vec::new(), + type_aliases: Vec::new(), + enums: Vec::new(), + globals: Vec::new(), + functions: Vec::new(), + init, + exported_native_instances: Vec::new(), + exported_func_return_native_instances: Vec::new(), + exported_objects: Vec::new(), + exported_functions: Vec::new(), + widgets: Vec::new(), + uses_fetch: false, + uses_webassembly: false, + extern_funcs: Vec::new(), + init_was_unrolled: false, + has_top_level_await: false, + init_kind: ModuleInitKind::Eager, + async_step_closures: std::collections::HashSet::new(), + closure_display_names: std::collections::HashMap::new(), + closure_source_text: std::collections::HashMap::new(), + async_generator_funcs: std::collections::HashSet::new(), + gen_param_prologue_len: std::collections::HashMap::new(), + } +} + +/// `" x ".trim(1)` — String.trim is argless but JS ignores the extra arg. +/// Codegen must lower this without bailing (`String.trim takes no args`). +#[test] +fn string_trim_with_extra_arg_compiles() { + let stmt = Stmt::Expr(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::String(" x ".to_string())), + property: "trim".to_string(), + }), + args: vec![Expr::Number(1.0)], + type_args: Vec::new(), + byte_offset: 0, + }); + let ir = compile_module(&module_with_init(vec![stmt]), empty_opts()) + .expect("\" x \".trim(1) must compile (extra arg ignored, not an error)"); + let ir = String::from_utf8(ir).unwrap(); + // The runtime trim helper must still be emitted — the arg is dropped, not + // routed into a different code path. + assert!( + ir.contains("js_string_trim"), + "expected trim lowering to emit js_string_trim" + ); +} + +/// `[1].pop(99)` — Array.pop is argless; the extra arg must be ignored. +#[test] +fn array_pop_with_extra_arg_compiles() { + let stmt = Stmt::Expr(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::Array(vec![Expr::Number(1.0)])), + property: "pop".to_string(), + }), + args: vec![Expr::Number(99.0)], + type_args: Vec::new(), + byte_offset: 0, + }); + compile_module(&module_with_init(vec![stmt]), empty_opts()) + .expect("[1].pop(99) must compile (extra arg ignored, not an error)"); +} diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index ea8a1a155..5552af358 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -960,8 +960,8 @@ fn pod_layout_constants_specialize_generic_layout_type_params() { module.functions.retain(|func| func.type_params.is_empty()); module.init.clear(); let ir = compile_ir_for_module_with_opts(module, pod_layout_specialization_opts()).unwrap(); - let tiny_ir = function_ir_section(&ir, "perry_fn_pod_layout_specialization_ts__layout_Tiny"); - let wide_ir = function_ir_section(&ir, "perry_fn_pod_layout_specialization_ts__layout_Wide"); + let tiny_ir = function_ir_section(&ir, "perry_fn_pod_layout_specialization_ts__u_layout_24_Tiny"); + let wide_ir = function_ir_section(&ir, "perry_fn_pod_layout_specialization_ts__u_layout_24_Wide"); assert!( tiny_ir.contains("8.0") && tiny_ir.contains("4.0") && !tiny_ir.contains("16.0"), @@ -1000,8 +1000,8 @@ fn native_pod_view_specializes_generic_layout_type_params() { module.functions.retain(|func| func.type_params.is_empty()); module.init.clear(); let ir = compile_ir_for_module_with_opts(module, pod_layout_specialization_opts()).unwrap(); - let tiny_ir = function_ir_section(&ir, "perry_fn_native_pod_view_specialization_ts__view_Tiny"); - let wide_ir = function_ir_section(&ir, "perry_fn_native_pod_view_specialization_ts__view_Wide"); + let tiny_ir = function_ir_section(&ir, "perry_fn_native_pod_view_specialization_ts__u_view_24_Tiny"); + let wide_ir = function_ir_section(&ir, "perry_fn_native_pod_view_specialization_ts__u_view_24_Wide"); assert!( tiny_ir.contains("call i64 @js_native_pod_view") && tiny_ir.contains("i64 8, i64 4"), diff --git a/crates/perry-codegen/tests/static_symbol_hygiene.rs b/crates/perry-codegen/tests/static_symbol_hygiene.rs index 6eef851d6..c341c4866 100644 --- a/crates/perry-codegen/tests/static_symbol_hygiene.rs +++ b/crates/perry-codegen/tests/static_symbol_hygiene.rs @@ -219,6 +219,11 @@ fn static_and_instance_methods_with_same_name_keep_distinct_symbols() { ) .unwrap(); + // The class name `x` is UNIQUE in this module, so its instance method keeps + // the id-less, cross-module-stable symbol (`perry_method_…__x__lex`). Only + // duplicate-named classes get the disambiguating `c` infix on instance + // methods. Static methods always carry the id (`perry_static_…__x__c11__`), + // which is internal to the module and never reconstructed cross-module. assert_eq!( count( &ir, @@ -226,6 +231,15 @@ fn static_and_instance_methods_with_same_name_keep_distinct_symbols() { ), 1 ); + // The instance method must NOT gain a spurious id infix when its name is + // unique (that would break cross-module references to exported classes). + assert_eq!( + count( + &ir, + "define double @perry_method_static_instance_symbol_hygiene_ts__x__c11__lex" + ), + 0 + ); assert_eq!( count( &ir, @@ -234,3 +248,41 @@ fn static_and_instance_methods_with_same_name_keep_distinct_symbols() { 1 ); } + +/// Two distinct classes sharing the minified name `x` (ids 11 and 12) must +/// emit DISTINCT instance-method symbols — the regression that produced +/// `invalid redefinition of function 'perry_method_…__j__getElementsByTagName'` +/// on a 13MB minified bundle. The id infix (`__c11__` vs `__c12__`) is what +/// keeps them apart. +#[test] +fn duplicate_class_instance_methods_use_class_id_in_symbols() { + let mut module = duplicate_static_module(); + module.name = "dup_instance_symbol_hygiene.ts".to_string(); + module.classes[0].methods.push(instance_method(301, 1.0)); + module.classes[1].methods.push(instance_method(302, 2.0)); + + let ir = String::from_utf8(compile_module(&module, empty_opts()).unwrap()).unwrap(); + + assert_eq!( + count( + &ir, + "define double @perry_method_dup_instance_symbol_hygiene_ts__x__c11__lex" + ), + 1 + ); + assert_eq!( + count( + &ir, + "define double @perry_method_dup_instance_symbol_hygiene_ts__x__c12__lex" + ), + 1 + ); + // No id-less form, which would collide across the two classes. + assert_eq!( + count( + &ir, + "define double @perry_method_dup_instance_symbol_hygiene_ts__x__lex" + ), + 0 + ); +} diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index 063a7b7e6..4ee22eca9 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -265,39 +265,83 @@ fn typed_feedback_instruments_property_and_method_boundaries() { #[test] fn typed_feedback_guards_direct_class_field_specialization() { - let point = class(101, "Point", vec![field("x", Type::Number)]); - let ir = ir_for(module_with_classes( - "typed_feedback_class_field.ts", - vec![point], - vec![param(1, "p", Type::Named("Point".to_string()))], - Type::Number, - vec![ - Stmt::Expr(Expr::PropertySet { - object: Box::new(Expr::LocalGet(1)), - property: "x".to_string(), - value: Box::new(Expr::Number(7.0)), - }), - Stmt::Return(Some(Expr::PropertyGet { - object: Box::new(Expr::LocalGet(1)), - property: "x".to_string(), - })), - ], - )); + // Build the same `p.x = 7; return p.x;` module fresh for each gate state so + // the env-var read in codegen is observed deterministically. + let build = || { + let point = class(101, "Point", vec![field("x", Type::Number)]); + module_with_classes( + "typed_feedback_class_field.ts", + vec![point], + vec![param(1, "p", Type::Named("Point".to_string()))], + Type::Number, + vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(1)), + property: "x".to_string(), + value: Box::new(Expr::Number(7.0)), + }), + Stmt::Return(Some(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(1)), + property: "x".to_string(), + })), + ], + ) + }; - assert!(ir.contains("class_field_set_guard")); - assert!(ir.contains("class_field_get_guard")); - assert!(ir.contains("@perry_typed_shape_raw_f64_mask_")); - assert!(ir.contains("js_typed_feedback_class_field_set_guard")); - assert!(ir.contains("js_typed_feedback_class_field_get_guard")); - assert!(ir.contains("class_field_set.fast")); - assert!(ir.contains("class_field_set.fallback")); - assert!(ir.contains("class_field_get.fast")); - assert!(ir.contains("class_field_get.fallback")); - assert!(ir.contains("store double")); - assert!(!ir.contains("call void @js_gc_note_slot_layout")); - assert!(ir.contains("call void @js_typed_feedback_record_fallback_call")); - assert!(ir.contains("call void @js_object_set_field_by_name")); - assert!(ir.contains("call double @js_object_get_field_by_name_f64")); + let _lock = ENV_LOCK.lock().unwrap(); + + // DEFAULT (#5247 kept OPT-IN: PERRY_INLINE_FIELDSET unset). The SET site + // emits the guard-call diamond — guard CALL drives the fast/%slow branch. + // #5334 lever A: the cold %slow arm (guard already FAILED in the entry + // block) collapses from the old inline pair (record_fallback_call + + // by-name set) into ONE outlined `js_class_field_set_fallback` call. NOT + // the slow helper, which would double-run the guard. + { + let _g = EnvVarGuard::set("PERRY_INLINE_FIELDSET", None); + let ir = ir_for(build()); + assert!(ir.contains("js_typed_feedback_class_field_set_guard")); + assert!(ir.contains("class_field_set.fast")); + assert!(ir.contains("class_field_set.slow")); + assert!(!ir.contains("call void @js_class_field_set_slow")); + assert!(ir.contains("call void @js_class_field_set_fallback")); + // The by-name SET call the helper replaces is no longer emitted at the + // set site (the GET diamond uses get_by_name, never set_by_name). + assert!(!ir.contains("call void @js_object_set_field_by_name")); + // NB: `record_fallback_call` is still present — but from the always-on + // class-field-GET fallback block below, not the SET site (the SET site's + // copy is now folded into js_class_field_set_fallback). + // GET path (always-on guard diamond) and shared expectations. + assert!(ir.contains("class_field_get_guard")); + assert!(ir.contains("@perry_typed_shape_raw_f64_mask_")); + assert!(ir.contains("js_typed_feedback_class_field_get_guard")); + assert!(ir.contains("class_field_get.fast")); + assert!(ir.contains("class_field_get.fallback")); + assert!(ir.contains("store double")); + assert!(!ir.contains("call void @js_gc_note_slot_layout")); + assert!(ir.contains("call double @js_object_get_field_by_name_f64")); + } + + // OPT-IN (PERRY_INLINE_FIELDSET=1): fast-inline / slow-outline inline + // cache. The hot path is the inline shape compare + the byte-identical + // bare slot store; the cold miss path collapses to one + // `call @js_class_field_set_slow` (which internally runs the real guard + + // records the fallback + by-name set). The SET site no longer emits the + // guard CALL, the inline `record_fallback_call`, or the by-name set. + { + let _g = EnvVarGuard::set("PERRY_INLINE_FIELDSET", Some("1")); + let ir = ir_for(build()); + assert!(ir.contains("class_field_set.fast")); + assert!(ir.contains("class_field_set.slow")); + assert!(ir.contains("call void @js_class_field_set_slow")); + assert!(!ir.contains("class_field_set.fallback")); + assert!(!ir.contains("call i32 @js_typed_feedback_class_field_set_guard")); + // The inline shape precheck is emitted (the perf-flat hot compare). + assert!(ir.contains("class_field_inline.deref")); + // GET path is unchanged regardless of the SET gate. + assert!(ir.contains("js_typed_feedback_class_field_get_guard")); + assert!(ir.contains("class_field_get.fast")); + assert!(ir.contains("store double")); + } } #[test] @@ -339,7 +383,11 @@ fn typed_feedback_guards_direct_class_method_specialization() { assert!(ir.contains("js_typed_feedback_method_direct_call_guard")); assert!(ir.contains("method_direct.fast")); assert!(ir.contains("method_direct.fallback")); - assert!(ir.contains("call void @js_typed_feedback_record_fallback_call")); + // (The method-direct fallback here has no typed-feedback site, so it calls + // js_native_call_method directly without an inline record_fallback_call. + // #5247: the incidental record_fallback_call this used to observe came from + // the synthesized constructor's field-set, now folded into the SET inline + // cache's slow helper — no longer emitted inline.) assert!(ir.contains("call double @js_native_call_method")); } diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 83bab8f50..4b484d14a 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -116,6 +116,11 @@ impl LoweringContext { classes_index: HashMap::new(), imported_functions_index: HashMap::new(), builtin_module_aliases_index: HashMap::new(), + native_instances_index: HashMap::new(), + module_native_instances_index: HashMap::new(), + func_return_native_instances_index: HashMap::new(), + native_modules_index: HashMap::new(), + class_statics_index: HashMap::new(), weakref_locals: HashSet::new(), finreg_locals: HashSet::new(), weakmap_locals: HashSet::new(), @@ -156,6 +161,7 @@ impl LoweringContext { optional_require_try_depth: 0, fn_ctor_env: super::fn_ctor_env::FnCtorEnv::default(), expr_lower_depth: 0, + prelowered_member_receiver: None, } } @@ -602,23 +608,24 @@ impl LoweringContext { static_fields: Vec, static_methods: Vec, ) { + // Forward-scan (first-match-wins): index keeps the FIRST entry per name. + let idx = self.class_statics.len(); + self.class_statics_index.entry(class_name.clone()).or_insert(idx); self.class_statics .push((class_name, static_fields, static_methods)); } pub(crate) fn has_static_field(&self, class_name: &str, field_name: &str) -> bool { - self.class_statics - .iter() - .find(|(cn, _, _)| cn == class_name) - .map(|(_, fields, _)| fields.contains(&field_name.to_string())) + self.class_statics_index + .get(class_name) + .map(|&idx| self.class_statics[idx].1.iter().any(|f| f == field_name)) .unwrap_or(false) } pub(crate) fn has_static_method(&self, class_name: &str, method_name: &str) -> bool { - self.class_statics - .iter() - .find(|(cn, _, _)| cn == class_name) - .map(|(_, _, methods)| methods.contains(&method_name.to_string())) + self.class_statics_index + .get(class_name) + .map(|&idx| self.class_statics[idx].2.iter().any(|m| m == method_name)) .unwrap_or(false) } @@ -1022,15 +1029,19 @@ impl LoweringContext { module_name: String, method_name: Option, ) { + // Forward-scan (first-match-wins) semantics: only record the FIRST + // index for a name so lookups match the old `.iter().find()`. + let idx = self.native_modules.len(); + self.native_modules_index.entry(local_name.clone()).or_insert(idx); self.native_modules .push((local_name, module_name, method_name)); } pub(crate) fn lookup_native_module(&self, name: &str) -> Option<(&str, Option<&str>)> { - self.native_modules - .iter() - .find(|(n, _, _)| n == name) - .map(|(_, m, method)| (m.as_str(), method.as_ref().map(|s| s.as_str()))) + self.native_modules_index.get(name).map(|&idx| { + let (_, m, method) = &self.native_modules[idx]; + (m.as_str(), method.as_ref().map(|s| s.as_str())) + }) } pub(crate) fn register_builtin_module_alias( @@ -1085,10 +1096,39 @@ impl LoweringContext { if is_compile_package_override(&module_name) { return; } + // Push the new index onto this name's shadow stack (innermost last). + let idx = self.native_instances.len(); + self.native_instances_index + .entry(local_name.clone()) + .or_default() + .push(idx); self.native_instances .push((local_name, module_name, class_name)); } + /// Truncate `native_instances` back to `mark`, keeping the + /// `native_instances_index` shadow stacks in sync: every recorded index + /// `>= mark` is popped (these belong to bindings whose scope is exiting), + /// re-exposing any earlier (outer-scope) binding of the same name. Empty + /// stacks are removed to keep the map small. Use this everywhere + /// `native_instances.truncate(..)` was previously called directly. + pub(crate) fn truncate_native_instances(&mut self, mark: usize) { + if self.native_instances.len() <= mark { + return; + } + self.native_instances.truncate(mark); + // Drop indices >= mark from each name's shadow stack, re-exposing any + // earlier (outer-scope) binding. The map is keyed by distinct + // native-instance names (bounded, not proportional to class count), so + // this stays cheap. + self.native_instances_index.retain(|_, stack| { + while stack.last().is_some_and(|&i| i >= mark) { + stack.pop(); + } + !stack.is_empty() + }); + } + /// #1483: resolve a parameter's declared type name to a perry/ui widget /// class that uses handle-based instance dispatch (Canvas, State, ...). /// Returns the canonical widget name (e.g. "Canvas") when `type_name` @@ -1129,10 +1169,17 @@ impl LoweringContext { // inner `res` always resolved to the outer `("http", // "ServerResponse")` tag and `res.on('data')` misrouted // through ServerResponse dispatch instead of IncomingMessage.) - self.native_instances - .iter() - .rev() - .find(|(n, _, _)| n == name) + // + // Indexed (#5271): `native_instances_index[name]` is the shadow stack + // of indices for this name, innermost (last) on top — so the top index + // is exactly the entry the old `.rev().find()` would have selected. + // The `exposes_plain_object_fields` filter is then applied to THAT + // entry only (matching `.find().filter()`, which never falls through to + // an earlier match when the top one is filtered out). + self.native_instances_index + .get(name) + .and_then(|stack| stack.last()) + .map(|&idx| &self.native_instances[idx]) // `node:repl` constructors allocate real heap objects/errors with // bound methods; routing them through handle-dispatch native // getters turns ordinary fields like `Recoverable.err` into @@ -1141,11 +1188,11 @@ impl LoweringContext { .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. - self.module_native_instances - .iter() - .rev() - .find(|(n, _, _)| n == name) + // Same last-match-wins rule for consistency — the index stores + // the LAST pushed entry per name. + self.module_native_instances_index + .get(name) + .map(|&idx| &self.module_native_instances[idx]) .filter(|(_, module, class)| !exposes_plain_object_fields(module, class)) .map(|(_, module, class)| (module.as_str(), class.as_str())) }) @@ -1155,10 +1202,35 @@ impl LoweringContext { &self, func_name: &str, ) -> Option<(&str, &str)> { - self.func_return_native_instances - .iter() - .find(|(n, _, _)| n == func_name) - .map(|(_, module, class)| (module.as_str(), class.as_str())) + // Forward-scan (first-match-wins): index keeps the FIRST pushed entry. + self.func_return_native_instances_index + .get(func_name) + .map(|&idx| { + let (_, module, class) = &self.func_return_native_instances[idx]; + (module.as_str(), class.as_str()) + }) + } + + /// Push a function-return native instance (push-only, never truncated) and + /// update its perf index. `lookup_func_return_native_instance` scanned + /// FORWARD (first-match-wins), so the index keeps the FIRST pushed entry. + pub(crate) fn push_func_return_native_instance(&mut self, entry: (String, String, String)) { + let idx = self.func_return_native_instances.len(); + self.func_return_native_instances_index + .entry(entry.0.clone()) + .or_insert(idx); + self.func_return_native_instances.push(entry); + } + + /// Push a module-level native instance (module-scoped, never truncated) + /// and update its perf index. `lookup_native_instance`'s fallback arm + /// scans these in reverse (last-match-wins), so the index stores the LAST + /// pushed entry per name (overwrite). + pub(crate) fn push_module_native_instance(&mut self, entry: (String, String, String)) { + let idx = self.module_native_instances.len(); + self.module_native_instances_index + .insert(entry.0.clone(), idx); + self.module_native_instances.push(entry); } } @@ -1241,7 +1313,7 @@ impl LoweringContext { } self.locals.extend(kept); } - self.native_instances.truncate(mark.1); + self.truncate_native_instances(mark.1); // Remove index entries for functions being truncated, then restore any // earlier entries that were shadowed by the removed ones. for i in mark.2..self.functions.len() { diff --git a/crates/perry-hir/src/lower/expr_assign.rs b/crates/perry-hir/src/lower/expr_assign.rs index 40ddb1153..64ee51a3a 100644 --- a/crates/perry-hir/src/lower/expr_assign.rs +++ b/crates/perry-hir/src/lower/expr_assign.rs @@ -192,7 +192,7 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) _ => Some("Instance"), }; if let Some(class_name) = class_name { - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( var_name.clone(), module_name.to_string(), class_name.to_string(), @@ -217,7 +217,7 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) module_name.clone(), class_name_str.to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( var_name.clone(), module_name, class_name_str.to_string(), @@ -230,7 +230,7 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) if let ast::Expr::Ident(rhs_ident) = inner_rhs { let rhs_name = rhs_ident.sym.as_ref(); if let Some((module, class)) = ctx.lookup_native_instance(rhs_name) { - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( var_name, module.to_string(), class.to_string(), diff --git a/crates/perry-hir/src/lower/expr_call/mod.rs b/crates/perry-hir/src/lower/expr_call/mod.rs index 8a4f73ef3..856986aa4 100644 --- a/crates/perry-hir/src/lower/expr_call/mod.rs +++ b/crates/perry-hir/src/lower/expr_call/mod.rs @@ -143,12 +143,21 @@ pub(super) fn lower_call(ctx: &mut LoweringContext, call: &ast::CallExpr) -> Res // are likewise out of scope once we unwind past their owning // call). `truncate` is a no-op when nothing was added. if ctx.native_instances.len() > ni_mark { - ctx.native_instances.truncate(ni_mark); + ctx.truncate_native_instances(ni_mark); } result } fn lower_call_inner(ctx: &mut LoweringContext, call: &ast::CallExpr) -> Result { + // Safety net for the receiver-lowering memo (see + // `LoweringContext::prelowered_member_receiver`): the memo is set by + // `try_static_method_and_instance` only to be consumed by the immediately + // following fall-through tail's `lower_member_inner`. It is single-shot and + // span-keyed, but in case a code path sets it without the tail consuming it, + // drop any stale entry here so it can never leak across calls. This runs + // before the callee tail (which lowers a Member, not a Call), so it never + // clobbers an in-flight memo for the call currently being lowered. + ctx.prelowered_member_receiver = None; // Check if any argument has spread let has_spread = call.args.iter().any(|arg| arg.spread.is_some()); diff --git a/crates/perry-hir/src/lower/expr_call/static_and_instance.rs b/crates/perry-hir/src/lower/expr_call/static_and_instance.rs index 59541ea50..62e983880 100644 --- a/crates/perry-hir/src/lower/expr_call/static_and_instance.rs +++ b/crates/perry-hir/src/lower/expr_call/static_and_instance.rs @@ -4,6 +4,7 @@ use anyhow::{anyhow, Result}; use perry_types::{LocalId, Type}; +use swc_common::Spanned; use swc_ecma_ast as ast; use super::stream::is_stream_api_method; @@ -345,7 +346,27 @@ pub(super) fn try_static_method_and_instance( if !may_lower_to_native_method_call(ctx, &member.obj) { return Ok(Err(args)); } - // Lower the object expression first + // Lower the object expression once. + // + // Perf (O(n²)/exponential → O(n) on long native-fluent chains): this + // is a FULL recursive lowering of the entire receiver prefix, done + // speculatively to inspect whether the inner call lowered to a + // `NativeMethodCall` of a recognized fluent module. On a chain like + // `K.name(..).description(..).option(..)…` (commander / minified CLI + // builders) where `may_lower_to_native_method_call` over-approximates + // to `true` but the inner call actually lowers to a *generic* `Call`, + // every arm below misses, `object_expr` is discarded, and we return + // `Err(args)` — whereupon the `lower_call_inner` fall-through tail + // re-lowers the same member callee (and thus this whole prefix) + // again. Repeated per chain level that is exponential blowup. + // + // To make the tail reuse this lowering instead of redoing it, stash + // it keyed by the receiver's source span. `lower_member_inner` (the + // tail's receiver-lowering site) consumes it for the matching span. + // Reuse is sound: lowering a receiver is idempotent in the value it + // produces, and the fluent-success arms below already reuse this very + // `object_expr`. + let obj_span = member.obj.as_ref().span(); let object_expr = lower_expr(ctx, &member.obj)?; // Check if it's a NativeMethodCall for a fluent-API native module if let Expr::NativeMethodCall { @@ -488,6 +509,12 @@ pub(super) fn try_static_method_and_instance( })); } } + // No fluent arm matched: we are about to return `Err(args)` and the + // `lower_call_inner` fall-through tail will re-lower this same member + // callee. Hand it the receiver we just lowered so it doesn't repeat + // the (potentially whole-prefix) work. Keyed by span so the tail only + // reuses it for the exact receiver subtree. + ctx.prelowered_member_receiver = Some(((obj_span.lo.0, obj_span.hi.0), object_expr)); } } diff --git a/crates/perry-hir/src/lower/expr_function.rs b/crates/perry-hir/src/lower/expr_function.rs index 592d7908a..71b8b3a7a 100644 --- a/crates/perry-hir/src/lower/expr_function.rs +++ b/crates/perry-hir/src/lower/expr_function.rs @@ -219,13 +219,6 @@ pub(super) fn lower_arrow(ctx: &mut LoweringContext, arrow: &ast::ArrowExpr) -> .unwrap_or_default(); ctx.enter_type_param_scope(&arrow_type_params); - // Track which locals exist before entering the closure scope - let outer_locals: Vec<(String, LocalId)> = ctx - .locals - .iter() - .map(|(name, id, _)| (name.clone(), *id)) - .collect(); - // Lower parameters and collect destructuring info let mut params = Vec::new(); let mut destructuring_params: Vec<(LocalId, ast::Pat)> = Vec::new(); @@ -413,7 +406,11 @@ pub(super) fn lower_arrow(ctx: &mut LoweringContext, arrow: &ast::ArrowExpr) -> // arrows don't leak outer T/U bindings into sibling code. ctx.exit_type_param_scope(); - let (captures, mutable_captures) = compute_closure_captures(ctx, &body, &outer_locals, ¶ms); + // The closure's own scope has been popped, so `ctx.locals.id_set()` is now + // exactly the enclosing scope's live locals — the membership view capture + // analysis needs. (Previously rebuilt per closure from a cloned snapshot.) + let (captures, mutable_captures) = + compute_closure_captures(ctx, &body, ctx.locals.id_set(), ¶ms); // Check if this arrow function uses `this` (needs to capture it from enclosing scope) let captures_this = closure_uses_this(&body); @@ -501,11 +498,15 @@ fn lower_named_fn_expr( // what the wrapper itself captures and threads through to the inner // function. let wrapper_scope = ctx.enter_scope(); - let outer_locals: Vec<(String, LocalId)> = ctx - .locals - .iter() - .map(|(name, id, _)| (name.clone(), *id)) - .collect(); + // Snapshot the enclosing locals *before* the self-binding is added, so the + // wrapper captures them (not the self-binding). Unlike the arrow/fn-expr + // paths, capture analysis runs here while the wrapper scope is still open + // (the self-binding lives in it), so we can't use `ctx.locals.id_set()` — + // it would wrongly include `self_id`. This is a rare path (named function + // expressions that recursively self-reference), so an explicit snapshot is + // fine. + let outer_local_ids: std::collections::HashSet = + ctx.locals.iter().map(|(_, id, _)| *id).collect(); let self_id = ctx.define_local(own_name.clone(), Type::Any); // Lower the function itself as an anonymous closure. With `self_id` @@ -533,7 +534,8 @@ fn lower_named_fn_expr( }, Stmt::Return(Some(Expr::LocalGet(self_id))), ]; - let (captures, mutable_captures) = compute_closure_captures(ctx, &body, &outer_locals, &[]); + let (captures, mutable_captures) = + compute_closure_captures(ctx, &body, &outer_local_ids, &[]); ctx.exit_scope(wrapper_scope); Ok(Expr::Call { @@ -577,13 +579,6 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul let saved_field_init = ctx.in_class_field_init; ctx.in_class_field_init = false; - // Track which locals exist before entering the closure scope - let outer_locals: Vec<(String, LocalId)> = ctx - .locals - .iter() - .map(|(name, id, _)| (name.clone(), *id)) - .collect(); - // Lower parameters and collect destructuring info. // // Refs #915 (gap 1 from #899 — Effect's `dual(arity, body)`): TypeScript's @@ -1038,7 +1033,9 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul ctx.exit_scope(scope_mark); ctx.in_class_field_init = saved_field_init; - let (captures, mutable_captures) = compute_closure_captures(ctx, &body, &outer_locals, ¶ms); + // Scope popped: `ctx.locals.id_set()` is now the enclosing scope's locals. + let (captures, mutable_captures) = + compute_closure_captures(ctx, &body, ctx.locals.id_set(), ¶ms); // #2076: a named function expression's own ident is its `fn.name` // per spec, regardless of the binding identifier it's later assigned @@ -1081,7 +1078,7 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul fn compute_closure_captures( ctx: &LoweringContext, body: &[Stmt], - outer_locals: &[(String, LocalId)], + outer_local_ids: &std::collections::HashSet, params: &[Param], ) -> (Vec, Vec) { // Detect captured variables: locals referenced in the body that @@ -1092,10 +1089,13 @@ fn compute_closure_captures( collect_local_refs_stmt(stmt, &mut all_refs, &mut visited_closures); } - // Filter to only include outer locals (not parameters or locals - // defined within the closure). - let outer_local_ids: std::collections::HashSet = - outer_locals.iter().map(|(_, id)| *id).collect(); + // `outer_local_ids` is the membership view of the enclosing scope's + // locals, supplied by the caller (the live `ctx.locals.id_set()` once the + // closure's own scope has been popped). Previously this was rebuilt into a + // fresh `HashSet` from an `&[(String, LocalId)]` snapshot on *every* + // closure — O(scope) per closure, i.e. O(n²) over n sibling closures in a + // large scope. We only ever need membership tests here, so the caller's + // incrementally-maintained set is reused directly. let param_ids: std::collections::HashSet = params.iter().map(|p| p.id).collect(); // dayjs (issue: format() returned `292278994-08`): local IDs are diff --git a/crates/perry-hir/src/lower/expr_member.rs b/crates/perry-hir/src/lower/expr_member.rs index b932d096e..50bc1b02d 100644 --- a/crates/perry-hir/src/lower/expr_member.rs +++ b/crates/perry-hir/src/lower/expr_member.rs @@ -11,6 +11,7 @@ use anyhow::Result; use perry_types::Type; +use swc_common::Spanned; use swc_ecma_ast as ast; use crate::ir::Expr; @@ -1781,7 +1782,17 @@ fn lower_member_inner(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Re } } - let mut object_expr = lower_expr(ctx, &member.obj)?; + // Perf: reuse a receiver already lowered by `try_static_method_and_instance` + // (the chained-native-method dispatch helper) for THIS exact member callee, + // instead of re-lowering the whole prefix. See + // `LoweringContext::prelowered_member_receiver`. Match strictly by span and + // take it (single-shot) so a stale memo can never leak onto a different + // receiver. Any other consumer along the way invalidates it. + let obj_span = member.obj.as_ref().span(); + let mut object_expr = match ctx.prelowered_member_receiver.take() { + Some((key, lowered)) if key == (obj_span.lo.0, obj_span.hi.0) => lowered, + _ => lower_expr(ctx, &member.obj)?, + }; if let ast::MemberProp::Ident(prop_ident) = &member.prop { if let Some(value) = ws_ready_state_value(prop_ident.sym.as_ref()) { if is_ws_ready_state_receiver(ctx, member.obj.as_ref(), &object_expr) { diff --git a/crates/perry-hir/src/lower/fn_ctor_env.rs b/crates/perry-hir/src/lower/fn_ctor_env.rs index 48e7bbb0d..40157159f 100644 --- a/crates/perry-hir/src/lower/fn_ctor_env.rs +++ b/crates/perry-hir/src/lower/fn_ctor_env.rs @@ -214,10 +214,10 @@ pub(crate) fn build_fn_ctor_env(module: &ast::Module) -> FnCtorEnv { let mut decls: HashMap)> = HashMap::new(); let mut writes: HashMap = HashMap::new(); - let empty_shadow = Shadow::new(); + let mut empty_shadow = Shadow::new(); for item in &module.body { if let ast::ModuleItem::Stmt(stmt) = item { - scan_stmt(stmt, &mut decls, &mut writes, &empty_shadow); + scan_stmt(stmt, &mut decls, &mut writes, &mut empty_shadow); } } @@ -783,7 +783,7 @@ fn collect_fn_scope_names(stmts: &[ast::Stmt], out: &mut Shadow) { /// Treat every binding identifier in a pattern as a write (catch clauses, /// destructuring) — it shadows or mutates the name. -fn record_pat_bindings(pat: &ast::Pat, writes: &mut HashMap, shadow: &Shadow) { +fn record_pat_bindings(pat: &ast::Pat, writes: &mut HashMap, shadow: &mut Shadow) { match pat { ast::Pat::Ident(b) => record_write(&b.id.sym, writes, shadow), ast::Pat::Array(arr) => { @@ -820,7 +820,7 @@ fn scan_stmt( stmt: &ast::Stmt, decls: &mut HashMap)>, writes: &mut HashMap, - shadow: &Shadow, + shadow: &mut Shadow, ) { match stmt { ast::Stmt::Decl(ast::Decl::Var(var)) => { @@ -939,7 +939,11 @@ fn scan_stmt( } } -fn scan_for_head_writes(head: &ast::ForHead, writes: &mut HashMap, shadow: &Shadow) { +fn scan_for_head_writes( + head: &ast::ForHead, + writes: &mut HashMap, + shadow: &mut Shadow, +) { match head { ast::ForHead::VarDecl(v) => { for d in &v.decls { @@ -961,7 +965,7 @@ fn scan_for_head_writes(head: &ast::ForHead, writes: &mut HashMap fn scan_assign_target_writes( target: &ast::AssignTarget, writes: &mut HashMap, - shadow: &Shadow, + shadow: &mut Shadow, ) { match target { ast::AssignTarget::Simple(simple) => match simple { @@ -999,27 +1003,52 @@ fn scan_assign_target_writes( /// Walk a nested function for writes to NON-shadowed (module-level) names. /// The function's params and its own hoisted declarations extend the shadow. +/// +/// The shadow is threaded as a single shared `&mut Shadow` that we push the +/// function's own names onto and pop afterwards, instead of `clone()`ing the +/// whole enclosing shadow per nested function. The old clone made scanning a +/// scope of N sibling nested functions O(N²) (each clone copies the ~N +/// enclosing names) — pathological for modules/wrapper-IIFEs that declare many +/// sibling closures (the same class of perf bug as the capture-set rebuild). +/// To restore the shadow exactly, we only remove the names this frame newly +/// inserted (a name already shadowed by an outer scope must stay shadowed). fn scan_fn_body_writes( params: &[&ast::Pat], stmts: &[ast::Stmt], writes: &mut HashMap, - outer_shadow: &Shadow, + shadow: &mut Shadow, ) { - let mut shadow = outer_shadow.clone(); + // Gather the names this function frame introduces (params + hoisted + // declarations) without disturbing the shared shadow. + let mut frame_names = Shadow::new(); for p in params { - collect_pat_names(p, &mut shadow); + collect_pat_names(p, &mut frame_names); + } + collect_fn_scope_names(stmts, &mut frame_names); + + // Insert only the names not already shadowed, remembering them so we can + // pop exactly this frame's additions and leave outer shadows intact. + let mut added: Vec = Vec::new(); + for name in frame_names { + if shadow.insert(name.clone()) { + added.push(name); + } } - collect_fn_scope_names(stmts, &mut shadow); + let mut nested_decls: HashMap)> = HashMap::new(); for s in stmts { - scan_stmt(s, &mut nested_decls, writes, &shadow); + scan_stmt(s, &mut nested_decls, writes, shadow); + } + + for name in added { + shadow.remove(&name); } } fn scan_function_writes( function: &ast::Function, writes: &mut HashMap, - shadow: &Shadow, + shadow: &mut Shadow, ) { let params: Vec<&ast::Pat> = function.params.iter().map(|p| &p.pat).collect(); let stmts: &[ast::Stmt] = function @@ -1030,7 +1059,7 @@ fn scan_function_writes( scan_fn_body_writes(¶ms, stmts, writes, shadow); } -fn scan_class_writes(class: &ast::Class, writes: &mut HashMap, shadow: &Shadow) { +fn scan_class_writes(class: &ast::Class, writes: &mut HashMap, shadow: &mut Shadow) { if let Some(sup) = &class.super_class { scan_expr_writes(sup, writes, shadow); } @@ -1069,7 +1098,7 @@ fn scan_class_writes(class: &ast::Class, writes: &mut HashMap, sh } } -fn scan_expr_writes(expr: &ast::Expr, writes: &mut HashMap, shadow: &Shadow) { +fn scan_expr_writes(expr: &ast::Expr, writes: &mut HashMap, shadow: &mut Shadow) { match expr { ast::Expr::Assign(a) => { scan_assign_target_writes(&a.left, writes, shadow); @@ -1174,11 +1203,23 @@ fn scan_expr_writes(expr: &ast::Expr, writes: &mut HashMap, shado scan_fn_body_writes(¶ms, &b.stmts, writes, shadow); } ast::BlockStmtOrExpr::Expr(e) => { - let mut inner = shadow.clone(); + // Arrow expression body: push the params, scan, then pop — + // mirrors `scan_fn_body_writes` so we don't clone the whole + // enclosing shadow per arrow. + let mut frame_names = Shadow::new(); for p in ¶ms { - collect_pat_names(p, &mut inner); + collect_pat_names(p, &mut frame_names); + } + let mut added: Vec = Vec::new(); + for name in frame_names { + if shadow.insert(name.clone()) { + added.push(name); + } + } + scan_expr_writes(e, writes, shadow); + for name in added { + shadow.remove(&name); } - scan_expr_writes(e, writes, &inner); } } } diff --git a/crates/perry-hir/src/lower/locals.rs b/crates/perry-hir/src/lower/locals.rs index 1912424f4..7e3a150d6 100644 --- a/crates/perry-hir/src/lower/locals.rs +++ b/crates/perry-hir/src/lower/locals.rs @@ -26,7 +26,7 @@ use std::ops::{Deref, DerefMut}; use perry_types::{LocalId, Type}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; /// Ordered stack of scope-local bindings with a name→positions index. #[derive(Debug, Clone, Default)] @@ -36,6 +36,17 @@ pub(crate) struct Locals { /// name -> ascending positions into `entries`. The last entry of each /// list is the innermost (most-recent) binding for that name. index: HashMap>, + /// Set of the `LocalId`s currently present in `entries`. Maintained in + /// lockstep with `entries` by every mutating method, so closure-capture + /// analysis can do O(1) "is this id an enclosing local?" membership tests + /// against the *current* scope without rebuilding a `HashSet` from the + /// `entries` slice on every closure. Each binding gets a fresh monotonic + /// `LocalId` (`fresh_local`), so ids are unique within `entries` and a + /// plain set (no refcount) stays in sync. See `compute_closure_captures` + /// and `nested_fn_decl` — both formerly rebuilt this set per closure, + /// making capture analysis O(scope) per closure = O(n²) over a scope of + /// n sibling closures. + id_set: HashSet, } impl Locals { @@ -47,9 +58,17 @@ impl Locals { pub(crate) fn push(&mut self, entry: (String, LocalId, Type)) { let pos = self.entries.len(); self.index.entry(entry.0.clone()).or_default().push(pos); + self.id_set.insert(entry.1); self.entries.push(entry); } + /// The set of `LocalId`s currently in scope. O(1). Used by closure-capture + /// analysis as the "enclosing locals" membership view, replacing a + /// per-closure rebuild from the `entries` slice (#5267 follow-up). + pub(crate) fn id_set(&self) -> &HashSet { + &self.id_set + } + /// Position of the innermost binding named `name`, if any. O(1). /// Equivalent to the old `iter().rposition(|(n, ..)| n == name)`. pub(crate) fn lookup_index(&self, name: &str) -> Option { @@ -132,6 +151,9 @@ impl Locals { self.index.remove(name); } } + for (_, id, _) in &drained { + self.id_set.remove(id); + } drained } @@ -151,11 +173,13 @@ impl Locals { removed } - /// Rebuild the whole name→positions index from `entries`. + /// Rebuild the whole name→positions index and the id set from `entries`. fn reindex(&mut self) { self.index.clear(); - for (pos, (name, _, _)) in self.entries.iter().enumerate() { + self.id_set.clear(); + for (pos, (name, id, _)) in self.entries.iter().enumerate() { self.index.entry(name.clone()).or_default().push(pos); + self.id_set.insert(*id); } } } diff --git a/crates/perry-hir/src/lower/lower_expr.rs b/crates/perry-hir/src/lower/lower_expr.rs index 5ae2de1ff..2f6402c9d 100644 --- a/crates/perry-hir/src/lower/lower_expr.rs +++ b/crates/perry-hir/src/lower/lower_expr.rs @@ -490,7 +490,68 @@ pub(crate) fn native_module_binding_value(ctx: &LoweringContext, name: &str) -> Expr::NativeModuleRef(module_name.to_string()) } +/// Re-lowering diagnostics, fully gated behind the `PERRY_TRACE_RELOWER` env +/// var (zero overhead unless set). Counts every `lower_expr` invocation keyed +/// by source span, so a span lowered far more than once flags redundant +/// re-lowering (the classic source of super-linear HIR-lowering blowup on +/// minified bundles). On every N-million calls — and so still on a kill — it +/// dumps the total/distinct counts and the top re-lowered spans to stderr. +/// Kept (env-gated) as a standing diagnostic for future lowering perf work. +pub(crate) mod relower_trace { + use std::cell::RefCell; + use std::collections::HashMap; + use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + + static ENABLED: AtomicBool = AtomicBool::new(false); + static INIT: AtomicBool = AtomicBool::new(false); + static TOTAL: AtomicU64 = AtomicU64::new(0); + + thread_local! { + static SPANS: RefCell> = RefCell::new(HashMap::new()); + } + + pub fn enabled() -> bool { + if !INIT.load(Ordering::Relaxed) { + let on = std::env::var("PERRY_TRACE_RELOWER").is_ok(); + ENABLED.store(on, Ordering::Relaxed); + INIT.store(true, Ordering::Relaxed); + } + ENABLED.load(Ordering::Relaxed) + } + + pub fn record(lo: u32, hi: u32) { + let n = TOTAL.fetch_add(1, Ordering::Relaxed) + 1; + SPANS.with(|m| { + *m.borrow_mut().entry((lo, hi)).or_insert(0) += 1; + }); + if n % 5_000_000 == 0 { + dump(&format!("periodic@{n}")); + } + } + + fn dump(tag: &str) { + SPANS.with(|m| { + let m = m.borrow(); + let total = TOTAL.load(Ordering::Relaxed); + let distinct = m.len(); + let mut v: Vec<_> = m.iter().map(|(k, c)| (*c, *k)).collect(); + v.sort_unstable_by(|a, b| b.0.cmp(&a.0)); + eprintln!( + "RELOWER[{tag}] total={total} distinct={distinct} ratio={:.2}", + total as f64 / distinct.max(1) as f64 + ); + for (c, (lo, hi)) in v.into_iter().take(20) { + eprintln!("RELOWER span {lo}..{hi} count={c}"); + } + }); + } +} + pub(crate) fn lower_expr(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result { + if relower_trace::enabled() { + let sp = expr.span(); + relower_trace::record(sp.lo.0, sp.hi.0); + } // #5259: guard the recursive descent. Without this, a pathologically // nested expression (`1+1+…`, `o.a.a.…`, `a||a||…`) overflows the native // stack and SIGABRTs with no diagnostic. The depth counter turns that into diff --git a/crates/perry-hir/src/lower/lowering_context.rs b/crates/perry-hir/src/lower/lowering_context.rs index 94fefa452..46e6eb817 100644 --- a/crates/perry-hir/src/lower/lowering_context.rs +++ b/crates/perry-hir/src/lower/lowering_context.rs @@ -367,6 +367,32 @@ pub struct LoweringContext { pub(crate) imported_functions_index: HashMap, /// Shadow index: local alias name -> index in `builtin_module_aliases` Vec pub(crate) builtin_module_aliases_index: HashMap, + /// Perf index for `native_instances` (which is scope-stack-like: pushed on + /// scope entry, truncated on scope exit). Maps name -> STACK of indices into + /// the `native_instances` Vec, innermost (last) on top. `lookup_native_instance` + /// reads the top index for O(1) last-match-wins resolution (mirrors the old + /// reverse scan's shadowing); on `truncate(mark)` every index >= mark is + /// popped off each name's stack (and empty stacks removed), so an inner + /// binding stops shadowing the moment its scope pops. See + /// `register_native_instance` / `truncate_native_instances`. + pub(crate) native_instances_index: HashMap>, + /// Perf index for `module_native_instances` (module-level, push-only, never + /// truncated). Scanned in reverse (last-match-wins) by + /// `lookup_native_instance`'s fallback arm, so the index stores the LAST + /// pushed index per name (overwritten on every push). O(1) lookup. + pub(crate) module_native_instances_index: HashMap, + /// Perf index for `func_return_native_instances` (push-only, never + /// truncated). The old lookup scanned FORWARD (first-match-wins), so the + /// index keeps the FIRST pushed index per name (`entry().or_insert`). + pub(crate) func_return_native_instances_index: HashMap, + /// Perf index for `native_modules` (push-only, never truncated). The old + /// `lookup_native_module` scanned FORWARD (first-match-wins), so the index + /// keeps the FIRST pushed index per name (`entry().or_insert`). + pub(crate) native_modules_index: HashMap, + /// Perf index for `class_statics` (push-only, never truncated). The old + /// `has_static_method`/`has_static_field` scanned FORWARD (first-match-wins), + /// so the index keeps the FIRST pushed index per class name. + pub(crate) class_statics_index: HashMap, /// Local names bound to a `path` sub-namespace (`const w = path.win32`). /// Maps the local name -> (root identifier name, sub "win32"|"posix"). /// Resolution of the root identifier to the `path` module is deferred to @@ -616,4 +642,19 @@ pub struct LoweringContext { /// pathologically-nested expression chain (bundler/minifier output like /// `1+1+…+1` or `o.a.a.…a`) overflow the native stack and SIGABRT. pub(crate) expr_lower_depth: u32, + /// Perf: a single-slot memo of an already-lowered member receiver, keyed by + /// its source span `(lo, hi)`. Set by the chained-native-method dispatch + /// helper (`try_static_method_and_instance`) just before it returns + /// `Err(args)` after lowering `member.obj` to inspect it; consumed once by + /// `lower_member_inner` when the `lower_call_inner` fall-through tail + /// re-lowers the same member callee. Without it, a long native-fluent + /// method chain (`K.name(..).description(..).option(..)…` — commander/minified + /// CLI builders) re-lowers the entire receiver prefix at every chain level + /// (the helper lowers it, finds the inner result is not a `NativeMethodCall`, + /// discards it, and the tail lowers it again — compounding to exponential + /// blowup). The memo lets the tail reuse the helper's lowering, so each + /// receiver subtree is lowered exactly once. Lowering a receiver is + /// idempotent w.r.t. the value produced (the fluent-success path already + /// reuses the same lowered receiver), so reusing it is semantics-preserving. + pub(crate) prelowered_member_receiver: Option<((u32, u32), Expr)>, } diff --git a/crates/perry-hir/src/lower/module_decl.rs b/crates/perry-hir/src/lower/module_decl.rs index 186e829cc..d9db3a1c7 100644 --- a/crates/perry-hir/src/lower/module_decl.rs +++ b/crates/perry-hir/src/lower/module_decl.rs @@ -486,7 +486,7 @@ pub(crate) fn lower_module_decl( if let Some((module, class)) = native_instance_from_return_type(&func.return_type) { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( func_name.clone(), module.to_string(), class.to_string(), @@ -784,7 +784,7 @@ pub(crate) fn lower_module_decl( // Without this, pool = mysql.createPool() at module top level loses // its native tracking when function scopes are entered/exited, // causing pool.query() inside functions to miss the Pool dispatch. - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), class_module, class_name.to_string(), @@ -868,7 +868,7 @@ pub(crate) fn lower_module_decl( module.clone(), class_name.to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), module, class_name.to_string(), @@ -969,7 +969,7 @@ pub(crate) fn lower_module_decl( module_name.clone(), class_name_str.to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), module_name, class_name_str.to_string(), @@ -1011,7 +1011,7 @@ pub(crate) fn lower_module_decl( module_name.clone(), class_name_str.to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), module_name, class_name_str.to_string(), @@ -1141,7 +1141,7 @@ pub(crate) fn lower_module_decl( } }; if let Some((module, class)) = module_info { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( name.clone(), module.to_string(), class.to_string(), @@ -1699,7 +1699,7 @@ pub(crate) fn lower_module_decl( if let Some((mod_name, class)) = native_instance_from_return_type(&func.return_type) { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( func_name.clone(), mod_name.to_string(), class.to_string(), @@ -2234,7 +2234,7 @@ pub(crate) fn lower_namespace_as_class( if let Some((module, class)) = native_instance_from_return_type(&func.return_type) { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( func.name.clone(), module.to_string(), class.to_string(), diff --git a/crates/perry-hir/src/lower/stmt.rs b/crates/perry-hir/src/lower/stmt.rs index 49ff32bf6..8330e19f3 100644 --- a/crates/perry-hir/src/lower/stmt.rs +++ b/crates/perry-hir/src/lower/stmt.rs @@ -327,7 +327,7 @@ pub(crate) fn lower_stmt( if let Some((module, class)) = native_instance_from_return_type(&func.return_type) { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( func.name.clone(), module.to_string(), class.to_string(), @@ -929,7 +929,7 @@ pub(crate) fn lower_stmt( module_owned.clone(), cn.to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), module_owned, cn.to_string(), @@ -946,7 +946,7 @@ pub(crate) fn lower_stmt( module_owned.clone(), cn.to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), module_owned, cn.to_string(), @@ -990,7 +990,7 @@ pub(crate) fn lower_stmt( "net".to_string(), "Server".to_string(), ); - ctx.module_native_instances.push(( + ctx.push_module_native_instance(( name.clone(), "net".to_string(), "Server".to_string(), @@ -1263,12 +1263,29 @@ pub(crate) fn lower_stmt( } ast::Stmt::Labeled(labeled_stmt) => { let label = labeled_stmt.label.sym.to_string(); - // #2383: a labeled *block* — `a: { ... break a; ... }` — exits the - // block via `break a`. Desugar to a labeled run-once do-while so the - // existing loop-based labeled-break codegen has an exit block to - // target. See the matching comment in lower_decl/body_stmt.rs. - if let ast::Stmt::Block(block) = &*labeled_stmt.body { - let body = lower_block_stmt_scoped(ctx, block)?; + // #2383 + #5247: a labeled *non-loop* statement — a block + // (`a: { ... break a; ... }`) or an `if`/`switch`/expression — exits via + // `break a`. It is not a loop, so the loop-based labeled-break codegen + // has nothing to bind the label to. Desugar any labeled non-loop to a + // labeled run-once do-while so it has an exit block to target (also + // correct when `break a` fires from inside a nested loop in the body). + // Labeled LOOPS skip this and keep the label bound to the loop, so + // `continue a` targets the loop. See the matching code in + // lower_decl/body_stmt.rs. + let body_is_loop = matches!( + &*labeled_stmt.body, + ast::Stmt::For(_) + | ast::Stmt::While(_) + | ast::Stmt::DoWhile(_) + | ast::Stmt::ForIn(_) + | ast::Stmt::ForOf(_) + ); + if !body_is_loop { + let body = if let ast::Stmt::Block(block) = &*labeled_stmt.body { + lower_block_stmt_scoped(ctx, block)? + } else { + lower_body_stmt(ctx, &labeled_stmt.body)? + }; module.init.push(Stmt::Labeled { label, body: Box::new(Stmt::DoWhile { diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 6df169565..79a2d982f 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -312,6 +312,105 @@ fn test_lower_rejects_deep_logical_chain() { assert_too_deep(format!("var a = 0;\nvar x = {};\n", chain.join("||"))); } +/// #5271: the perf index over `native_instances` must reproduce the old +/// reverse-scan semantics exactly — innermost (last-registered) binding wins, +/// and `truncate_native_instances` re-exposes the outer binding when the inner +/// scope pops. Mirrors the `lookup_native_instance` last-match-wins rule. +#[test] +fn test_native_instance_index_shadowing_and_truncation() { + let mut ctx = make_ctx(); + // Outer binding `e` -> events/EventEmitter. + ctx.register_native_instance( + "e".to_string(), + "events".to_string(), + "EventEmitter".to_string(), + ); + assert_eq!( + ctx.lookup_native_instance("e"), + Some(("events", "EventEmitter")) + ); + + // Enter an inner scope: shadow `e` with a different native type. + let mark = ctx.native_instances.len(); + ctx.register_native_instance("e".to_string(), "stream".to_string(), "Readable".to_string()); + // Inner (last) binding wins. + assert_eq!(ctx.lookup_native_instance("e"), Some(("stream", "Readable"))); + + // Pop the inner scope: the outer binding must be restored. + ctx.truncate_native_instances(mark); + assert_eq!( + ctx.lookup_native_instance("e"), + Some(("events", "EventEmitter")) + ); + + // Pop the outer binding too: no entry remains. + ctx.truncate_native_instances(0); + assert!(ctx.lookup_native_instance("e").is_none()); +} + +/// #5271: module-level native instances (never truncated) keep last-match-wins +/// via the overwrite index, matching the old reverse scan of the fallback arm. +#[test] +fn test_module_native_instance_index_last_wins() { + let mut ctx = make_ctx(); + ctx.push_module_native_instance(( + "db".to_string(), + "mongodb".to_string(), + "MongoClient".to_string(), + )); + assert_eq!( + ctx.lookup_native_instance("db"), + Some(("mongodb", "MongoClient")) + ); + // A later registration of the same name shadows the earlier one. + ctx.push_module_native_instance(( + "db".to_string(), + "mysql2/promise".to_string(), + "Pool".to_string(), + )); + assert_eq!( + ctx.lookup_native_instance("db"), + Some(("mysql2/promise", "Pool")) + ); +} + +/// #5271 perf gate (run with `--release --ignored`): time M lookups against a +/// K-sized registry to show indexed lookups are ~flat in K (O(1)) rather than +/// O(K) per call. Prints timings; not asserted (machine-dependent) but the +/// flatness across K is the observable signal. Covers the registries whose +/// linear scans this change indexed. +#[test] +#[ignore] +fn perf_registry_lookup_is_flat_in_k() { + use std::time::Instant; + const M: usize = 20_000; + for k in [0usize, 2_000, 8_000, 16_000] { + let mut ctx = make_ctx(); + for i in 0..k { + ctx.register_class_statics( + format!("K{i}"), + vec![format!("f{i}")], + vec![format!("s{i}")], + ); + ctx.register_native_instance(format!("ni{i}"), "events".into(), "EventEmitter".into()); + ctx.register_native_module(format!("nm{i}"), "fs".into(), None); + } + // The hot case the bug targets: the receiver is NOT in the registry, so + // the old reverse/forward scan walked the whole Vec and returned None. + let t = Instant::now(); + let mut acc = 0u64; + for _ in 0..M { + acc += ctx.has_static_method("Missing", "s") as u64; + acc += ctx.lookup_native_instance("missing").is_some() as u64; + acc += ctx.lookup_native_module("missing").is_some() as u64; + } + eprintln!( + "K={k:<6} {M} x3 lookups: {:?} (acc={acc})", + t.elapsed() + ); + } +} + /// A chain comfortably under the ceiling still lowers cleanly — the guard /// must not reject ordinary (if large) expressions. #[test] diff --git a/crates/perry-hir/src/lower_decl/body_stmt.rs b/crates/perry-hir/src/lower_decl/body_stmt.rs index 5993e484c..8cd5e0828 100644 --- a/crates/perry-hir/src/lower_decl/body_stmt.rs +++ b/crates/perry-hir/src/lower_decl/body_stmt.rs @@ -448,17 +448,33 @@ pub fn lower_body_stmt(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result { let label = labeled_stmt.label.sym.to_string(); - // #2383: a labeled *block* — `a: { ... break a; ... }` — exits the - // block via `break a` (valid JS/TS; heavily used by minified React). - // It is NOT a loop, so the loop-based labeled-break codegen has - // nothing to bind the label to. Desugar to a labeled run-once - // do-while: `a: do { ... } while (false)`. The do-while's exit block - // becomes the labeled-break target, the body runs exactly once, and - // the `while (false)` falls straight through to the exit. `continue - // a` against a block label is a JS early SyntaxError, so it never - // reaches here. - if let ast::Stmt::Block(block) = &*labeled_stmt.body { - let body = lower_block_stmt_scoped(ctx, block)?; + // #2383 + #5247: a labeled *non-loop* statement — a block + // (`a: { ... break a; ... }`, heavily used by minified React), or an + // `if` (`a: if (...) { ... break a; ... }`, emitted by minified + // bignumber.js / ajv), or a `switch`/expression statement — exits via + // `break a`. It is NOT a loop, so the loop-based labeled-break codegen + // has nothing to bind the label to. Desugar any labeled non-loop to a + // labeled run-once do-while: `a: do { ... } while (false)`. The + // do-while's exit block becomes the labeled-break target, the body runs + // exactly once, and `while (false)` falls straight through, including + // when the `break a` fires from inside a *nested* loop in the body. + // `continue a` against a non-loop label is a JS early SyntaxError, so it + // never reaches here. Labeled LOOPS skip this and keep the label bound + // to the loop itself, so `continue a` targets the loop. + let body_is_loop = matches!( + &*labeled_stmt.body, + ast::Stmt::For(_) + | ast::Stmt::While(_) + | ast::Stmt::DoWhile(_) + | ast::Stmt::ForIn(_) + | ast::Stmt::ForOf(_) + ); + if !body_is_loop { + let body = if let ast::Stmt::Block(block) = &*labeled_stmt.body { + lower_block_stmt_scoped(ctx, block)? + } else { + lower_body_stmt(ctx, &labeled_stmt.body)? + }; result.push(Stmt::Labeled { label, body: Box::new(Stmt::DoWhile { @@ -1848,7 +1864,7 @@ pub fn find_native_return_in_stmts( let var = ident.sym.as_ref(); for i in ni_start..ctx.native_instances.len() { if ctx.native_instances[i].0 == var { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( func_name.to_string(), ctx.native_instances[i].1.clone(), ctx.native_instances[i].2.clone(), diff --git a/crates/perry-hir/src/lower_decl/body_stmt/nested_fn_decl.rs b/crates/perry-hir/src/lower_decl/body_stmt/nested_fn_decl.rs index fe500bafb..ccaeabbee 100644 --- a/crates/perry-hir/src/lower_decl/body_stmt/nested_fn_decl.rs +++ b/crates/perry-hir/src/lower_decl/body_stmt/nested_fn_decl.rs @@ -38,13 +38,6 @@ pub(super) fn lower_nested_fn_decl( let scope_mark = ctx.enter_scope(); - // Track outer locals for capture detection - let outer_locals: Vec<(String, LocalId)> = ctx - .locals - .iter() - .map(|(name, id, _)| (name.clone(), *id)) - .collect(); - // Lower parameters. Skip the TypeScript `this:` annotation — // it has no runtime existence (see the sibling site above for // the full rationale). @@ -170,8 +163,13 @@ pub(super) fn lower_nested_fn_decl( collect_local_refs_stmt(stmt, &mut all_refs, &mut visited_closures); } - let outer_local_ids: std::collections::HashSet = - outer_locals.iter().map(|(_, id)| *id).collect(); + // The function's own scope has been popped (`exit_scope` above), so the + // live `ctx.locals.id_set()` is exactly the enclosing scope's locals — the + // membership view capture detection needs. Previously this was rebuilt into + // a fresh `HashSet` from a per-closure cloned snapshot of `ctx.locals`, + // which made capture analysis O(scope) per nested function = O(n²) over a + // scope of n sibling functions (the perf bug behind this change). + let outer_local_ids = ctx.locals.id_set(); let param_ids: std::collections::HashSet = params.iter().map(|p| p.id).collect(); // dayjs (issue: format() returned `292278994-08`): local diff --git a/crates/perry-hir/src/lower_decl/fn_decl.rs b/crates/perry-hir/src/lower_decl/fn_decl.rs index 07d7e35a1..bca121742 100644 --- a/crates/perry-hir/src/lower_decl/fn_decl.rs +++ b/crates/perry-hir/src/lower_decl/fn_decl.rs @@ -227,7 +227,7 @@ pub fn lower_fn_decl(ctx: &mut LoweringContext, fn_decl: &ast::FnDecl) -> Result let module_alias = &type_name[..dot_pos]; let class_name = &type_name[dot_pos + 1..]; if let Some((module_name, _)) = ctx.lookup_native_module(module_alias) { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( name.clone(), module_name.to_string(), class_name.to_string(), @@ -245,7 +245,7 @@ pub fn lower_fn_decl(ctx: &mut LoweringContext, fn_decl: &ast::FnDecl) -> Result _ => None, }; if let Some((module, class)) = module_info { - ctx.func_return_native_instances.push(( + ctx.push_func_return_native_instance(( name.clone(), module.to_string(), class.to_string(), diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index bba2164b4..b80f98699 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -963,7 +963,8 @@ pub extern "C" fn js_typed_feedback_object_set_field_by_name_fast( #[path = "typed_feedback/guards.rs"] mod guards; pub use guards::{ - js_typed_feedback_class_field_get_guard, js_typed_feedback_class_field_set_guard, + js_class_field_set_slow, js_typed_feedback_class_field_get_guard, + js_typed_feedback_class_field_set_guard, js_typed_feedback_closure_direct_call_guard, js_typed_feedback_method_direct_call_guard, js_typed_feedback_native_call_method, js_typed_feedback_native_call_method_apply, }; diff --git a/crates/perry-runtime/src/typed_feedback/guards.rs b/crates/perry-runtime/src/typed_feedback/guards.rs index 09fe2ad0b..3ec05988e 100644 --- a/crates/perry-runtime/src/typed_feedback/guards.rs +++ b/crates/perry-runtime/src/typed_feedback/guards.rs @@ -599,6 +599,142 @@ pub extern "C" fn js_typed_feedback_class_field_set_guard( } } +/// Class-field-SET inline-cache COLD/SLOW path (#5247, `PERRY_INLINE_FIELDSET`). +/// +/// This is the single helper the inline-cache miss block calls. The hot fast +/// path stays INLINE in codegen: a cheap shape compare (`emit_class_field_inline_precheck`) +/// + the bare slot store. Only the cold miss/fallback path collapses to one +/// `call @js_class_field_set_slow(...)`. The inline compare is conservative — +/// it takes the inline fast store ONLY when the receiver is definitely a plain +/// writable own-field store (matching class id + keys, intact typed layout, not +/// frozen, plain-number for raw-f64, and the process-global inline-enable flag +/// unset). Everything else — first-time shapes, typed-feedback enabled, +/// descriptors in use, polymorphic / non-pointer receivers, frozen / accessor / +/// non-writable / setter-in-chain — falls here. +/// +/// The body reproduces TODAY'S full semantics for every case the inline compare +/// doesn't cover: +/// +/// 1. Run the same `js_typed_feedback_class_field_set_guard` (so the typed- +/// feedback observation/recording is identical to the old inline path). +/// 2. On a guard PASS (e.g. typed-feedback now says fast-ok, or a first-time +/// shape that matches the contract), do the SAME slot store the inline +/// fast path would have done: +/// * `require_raw_f64` slot → a bare `f64` store into the field slot +/// (the inline path emits a plain `store double` with no GC barrier — +/// the slot is pointer-free by typed-shape descriptor and the guard +/// already proved `is_plain_number_bits(value)`). +/// * boxed slot → `js_object_set_field`, which performs the identical +/// slot write + `js_gc_note_slot_layout` + `js_write_barrier_slot` the +/// inline `emit_jsvalue_slot_store_on_block` emits. +/// 3. On a guard FAIL, record the fallback (matching the old inline fallback +/// block's `js_typed_feedback_record_fallback_call`) and route through the +/// by-name setter `js_object_set_field_by_name`, which handles +/// frozen / accessor / non-writable / setter-in-chain semantics. +/// +/// Frozen/accessor/writable/setter handling all live behind the guard (the +/// guard returns 0 for them, sending the write to the by-name fallback), so no +/// special-casing is needed here. +#[no_mangle] +pub extern "C" fn js_class_field_set_slow( + site_id: u64, + receiver: f64, + expected_class_id: u32, + expected_keys: *const ArrayHeader, + key: *const crate::StringHeader, + expected_field_index: u32, + value: f64, + require_raw_f64: i32, +) { + let guard_ok = js_typed_feedback_class_field_set_guard( + site_id, + receiver, + expected_class_id, + expected_keys, + key, + expected_field_index, + value, + require_raw_f64, + ); + + if guard_ok != 0 { + // FAST PATH — identical to the codegen-inlined `class_field_set.fast` + // block. The guard has already validated the receiver class/shape, + // the field index bound, frozen/accessor/writable/setter state, and + // (for raw-f64) the typed-shape descriptor + plain-number value. + let object_addr = normalize_raw_object_addr(receiver.to_bits()); + if require_raw_f64 != 0 { + // Pointer-free raw-f64 slot: bare store, no GC barrier — exactly + // what the inline `blk.store(DOUBLE, val, field_ptr)` emits. + unsafe { + let fields_ptr = + (object_addr as *mut u8).add(std::mem::size_of::()) as *mut f64; + let slot = fields_ptr.add(expected_field_index as usize); + std::ptr::write(slot, value); + } + } else { + // Boxed slot: same slot write + layout note + write barrier the + // inline path emits via `emit_jsvalue_slot_store_on_block`. + crate::object::js_object_set_field( + object_addr as *mut ObjectHeader, + expected_field_index, + crate::value::JSValue::from_bits(value.to_bits()), + ); + } + return; + } + + // FALLBACK — identical to the codegen-inlined `class_field_set.fallback` + // block: record the fallback, then route the write by name. + crate::typed_feedback::js_typed_feedback_record_fallback_call(site_id); + // The inline fallback passes the receiver's FULL bits (NaN-box tag intact — + // `js_object_set_field_by_name` inspects the tag for proxy/exotic dispatch + // before masking to the heap address) and the POINTER_MASK-stripped key. + let obj_bits = receiver.to_bits(); + let key_raw = key as u64 & crate::value::POINTER_MASK; + crate::object::js_object_set_field_by_name( + obj_bits as *mut ObjectHeader, + key_raw as *const crate::StringHeader, + value, + ); +} + +/// Class-field-SET guard-MISS fallback, outlined (#5334, lever A). +/// +/// This is the cold arm of the DEFAULT (guard-CALL) class-field-set diamond. +/// Unlike [`js_class_field_set_slow`] — which the opt-in inline-precheck path +/// uses and which RE-RUNS the guard — this helper is called only after the +/// inline `js_typed_feedback_class_field_set_guard` has ALREADY run and FAILED +/// in the entry block (that failure is what branched control here). So there is +/// nothing left to decide: just reproduce the two operations the inline +/// `class_field_set` fallback block used to emit, collapsed into ONE call so the +/// cold arm costs a single instruction per site instead of two. +/// +/// Byte-identical semantics to the old inline fallback: +/// 1. `js_typed_feedback_record_fallback_call(site_id)` — record the miss. +/// 2. `js_object_set_field_by_name(obj_bits, key_raw, value)` — route the +/// write by name (handles frozen / accessor / non-writable / setter-in- +/// chain). `obj_bits` keeps the full NaN-box tag (the by-name setter +/// inspects it for proxy/exotic dispatch before masking to the heap +/// address); `key_raw` is the POINTER_MASK-stripped key handle. +/// +/// Cold-path only, so the extra call frame has zero hot-loop cost; the win is +/// purely in emitted IR size (2 calls + operand list → 1 call per set site). +#[no_mangle] +pub extern "C" fn js_class_field_set_fallback( + site_id: u64, + obj_bits: u64, + key_raw: u64, + value: f64, +) { + crate::typed_feedback::js_typed_feedback_record_fallback_call(site_id); + crate::object::js_object_set_field_by_name( + obj_bits as *mut ObjectHeader, + key_raw as *const crate::StringHeader, + value, + ); +} + #[no_mangle] pub unsafe extern "C" fn js_typed_feedback_native_call_method( site_id: u64, @@ -835,6 +971,8 @@ mod keep_guard_symbols { use super::*; #[used] static G0: extern "C" fn(u64, f64, u32, *const ArrayHeader, *const crate::StringHeader, u32, i32) -> i32 = js_typed_feedback_class_field_get_guard; #[used] static G1: extern "C" fn(u64, f64, u32, *const ArrayHeader, *const crate::StringHeader, u32, f64, i32) -> i32 = js_typed_feedback_class_field_set_guard; + #[used] static G1B: extern "C" fn(u64, f64, u32, *const ArrayHeader, *const crate::StringHeader, u32, f64, i32) = js_class_field_set_slow; + #[used] static G1C: extern "C" fn(u64, u64, u64, f64) = js_class_field_set_fallback; #[used] static G2: unsafe extern "C" fn(u64, f64, u32, *const ArrayHeader, *const i8, usize, *const u8) -> i32 = js_typed_feedback_method_direct_call_guard; #[used] static G3: extern "C" fn(u64, f64, *const u8, u32, u32) -> i32 = js_typed_feedback_closure_direct_call_guard; } diff --git a/crates/perry-transform/src/generator/break_continue.rs b/crates/perry-transform/src/generator/break_continue.rs index 1e7daab08..0fe2f34cf 100644 --- a/crates/perry-transform/src/generator/break_continue.rs +++ b/crates/perry-transform/src/generator/break_continue.rs @@ -13,6 +13,40 @@ const BREAK_SENTINEL: f64 = 1_000_001.0; /// (for-loops) or `cond_state` (while-loops) post-linearization. const CONTINUE_SENTINEL: f64 = 1_000_002.0; +/// Base for per-label `break