From 8e552201a79d8800e067ca8d70067581d8df2943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:05:54 +0200 Subject: [PATCH 01/20] fix(trace_events): align descriptors and empty filters (cherry picked from commit f6e80ad38f791ae668f2b6f5496be62d3994a425) --- changelog.d/9202-trace-events-descriptors.md | 1 + .../perry-runtime/src/node_submodules/mod.rs | 4 +- .../src/node_submodules/trace_events.rs | 51 +++++++++++++++---- 3 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 changelog.d/9202-trace-events-descriptors.md diff --git a/changelog.d/9202-trace-events-descriptors.md b/changelog.d/9202-trace-events-descriptors.md new file mode 100644 index 0000000000..bfda0b3942 --- /dev/null +++ b/changelog.d/9202-trace-events-descriptors.md @@ -0,0 +1 @@ +Aligned `node:trace_events` export descriptors and empty category filtering with Node. diff --git a/crates/perry-runtime/src/node_submodules/mod.rs b/crates/perry-runtime/src/node_submodules/mod.rs index 6158315af0..92561cd327 100644 --- a/crates/perry-runtime/src/node_submodules/mod.rs +++ b/crates/perry-runtime/src/node_submodules/mod.rs @@ -1430,7 +1430,7 @@ fn ensure_namespace_singleton(submod: &'static SubmoduleSpec) -> *mut ObjectHead crate::object::set_property_attrs( obj as usize, spec.name.to_string(), - PropertyAttrs::new(true, true, true), + PropertyAttrs::new(true, true, false), ); } let default_obj = js_object_alloc(0, submod.exports.len() as u32); @@ -1451,7 +1451,7 @@ fn ensure_namespace_singleton(submod: &'static SubmoduleSpec) -> *mut ObjectHead crate::object::set_property_attrs( obj as usize, "default".to_string(), - PropertyAttrs::new(true, true, true), + PropertyAttrs::new(true, true, false), ); } if let Some(default_value) = submodule_default_object_value(submod) { diff --git a/crates/perry-runtime/src/node_submodules/trace_events.rs b/crates/perry-runtime/src/node_submodules/trace_events.rs index 540ea0f4ac..58542a2e4e 100644 --- a/crates/perry-runtime/src/node_submodules/trace_events.rs +++ b/crates/perry-runtime/src/node_submodules/trace_events.rs @@ -503,17 +503,13 @@ fn trace_options_from_args(args: impl IntoIterator) -> TraceOutpu output.enabled = true; output.explicit_categories = true; if let Some(value) = args.get(index + 1) { - output - .categories - .extend(value.split(',').map(str::to_owned)); + extend_cli_categories(&mut output.categories, value); index += 1; } } else if let Some(value) = arg.strip_prefix("--trace-event-categories=") { output.enabled = true; output.explicit_categories = true; - output - .categories - .extend(value.split(',').map(str::to_owned)); + extend_cli_categories(&mut output.categories, value); } else if arg == "--trace-event-file-pattern" { if let Some(value) = args.get(index + 1) { output.file_pattern = Some(value.clone()); @@ -527,6 +523,22 @@ fn trace_options_from_args(args: impl IntoIterator) -> TraceOutpu output } +fn extend_cli_categories(categories: &mut BTreeSet, value: &str) { + // Node accepts a quoted empty category list (`""`) as an enabled trace + // containing metadata only. `spawnSync` passes those quote bytes through + // directly, so normalize them before splitting the CLI value. + let value = value + .strip_prefix('"') + .and_then(|value| value.strip_suffix('"')) + .unwrap_or(value); + categories.extend( + value + .split(',') + .filter(|name| !name.is_empty()) + .map(str::to_owned), + ); +} + fn seed_legacy_categories(output: &mut TraceOutput) { if output.legacy_enabled && !output.explicit_categories { output.categories.extend( @@ -581,12 +593,21 @@ pub(crate) fn flush_trace_events_output() { }) .unwrap_or_else(|| format!("node_trace.{pid}.log")); let category = if output.categories.contains("node.console") { - "node.console" + Some("node.console") + } else if output.categories.contains("node") + || output.categories.contains("node.bootstrap") + { + Some("node,node.bootstrap") } else { - "node,node.bootstrap" + None }; - let application = - format!(",{{\"cat\":\"{category}\",\"name\":\"included-marker\",\"ph\":\"X\"}}"); + let application = category + .map(|category| { + format!( + ",{{\"cat\":\"{category}\",\"name\":\"included-marker\",\"ph\":\"X\"}}" + ) + }) + .unwrap_or_default(); let document = format!( "{{\"traceEvents\":[{{\"cat\":\"__metadata\",\"name\":\"process_name\",\"ph\":\"M\"}}{application}]}}" ); @@ -680,4 +701,14 @@ mod tests { ["custom"] ); } + + #[test] + fn quoted_empty_category_list_enables_metadata_only() { + let output = trace_options_from_args( + ["perry", "--trace-event-categories", "\"\""].map(str::to_owned), + ); + assert!(output.enabled); + assert!(output.explicit_categories); + assert!(output.categories.is_empty()); + } } From 72d33ee3efdeefc030cab860b9fada2f2a7ce214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:15:56 +0200 Subject: [PATCH 02/20] chore: number changeset for PR 9884 (cherry picked from commit 303493744c2a1f89821d2bff944da6fd0a16ccb0) --- ...ace-events-descriptors.md => 9884-trace-events-descriptors.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9202-trace-events-descriptors.md => 9884-trace-events-descriptors.md} (100%) diff --git a/changelog.d/9202-trace-events-descriptors.md b/changelog.d/9884-trace-events-descriptors.md similarity index 100% rename from changelog.d/9202-trace-events-descriptors.md rename to changelog.d/9884-trace-events-descriptors.md From fe6d5fb5662a15d423a832d2a382c948290e0054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:52:32 +0200 Subject: [PATCH 03/20] fix(perf_hooks): separate default export namespace (cherry picked from commit c60ba608fb8c41871713d866a5753026cc398fb3) --- .../9202-perf-hooks-default-export-keys.md | 3 ++ .../perry-dispatch/src/cjs_default_modules.rs | 1 + crates/perry-hir/src/lower/tests.rs | 33 +++++++++++++++++++ .../perry-runtime/src/object/native_module.rs | 14 +++----- .../src/object/native_module/module_keys.rs | 17 ++++++++++ .../src/object/native_module_dispatch.rs | 1 + 6 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 changelog.d/9202-perf-hooks-default-export-keys.md diff --git a/changelog.d/9202-perf-hooks-default-export-keys.md b/changelog.d/9202-perf-hooks-default-export-keys.md new file mode 100644 index 0000000000..04d1e4a361 --- /dev/null +++ b/changelog.d/9202-perf-hooks-default-export-keys.md @@ -0,0 +1,3 @@ +### Fixed + +- Match Node's `node:perf_hooks` default-import keys while preserving namespace `default` and shared export identity. diff --git a/crates/perry-dispatch/src/cjs_default_modules.rs b/crates/perry-dispatch/src/cjs_default_modules.rs index 798f436d19..d1a05c5e33 100644 --- a/crates/perry-dispatch/src/cjs_default_modules.rs +++ b/crates/perry-dispatch/src/cjs_default_modules.rs @@ -47,6 +47,7 @@ cjs_default_namespace_modules!( "path", "path.posix", "path.win32", + "perf_hooks", "process", "punycode", "querystring", diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index d13fbea68b..fcf1c2943d 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -582,6 +582,39 @@ export function perfHooksDefault() { )); } +#[test] +fn native_perf_hooks_default_import_reads_cjs_namespace() { + let source = r#" +import hooks from "node:perf_hooks"; +export function perfHooksDefaultImport() { + return hooks; +} +"#; + let module = perry_parser::parse_typescript(source, "perf-hooks-default-import.ts") + .expect("source parses"); + let hir = super::lower_module( + &module, + "perf-hooks-default-import", + "perf-hooks-default-import.ts", + ) + .expect("source lowers"); + let function = hir + .functions + .iter() + .find(|function| function.name == "perfHooksDefaultImport") + .expect("exported function is lowered"); + + assert!(matches!( + function.body.as_slice(), + [Stmt::Return(Some(crate::ir::Expr::PropertyGet { + object, + property, + .. + }))] if property == "default" + && matches!(object.as_ref(), crate::ir::Expr::NativeModuleRef(module) if module == "perf_hooks") + )); +} + #[test] fn test_lower_type_param_scoping() { let mut ctx = make_ctx(); diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index c27e5d5ffe..08e282c625 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -702,14 +702,6 @@ pub(crate) fn cjs_default_export_value(module_name: &str) -> Option { "dgram".len(), )), "module" => Some(bound_native_callable_export_value("module", "Module")), - // node:perf_hooks has no distinct CJS shape — `module.exports` IS the - // namespace, and `default` is listed among its keys. Resolving to the - // same tag keeps `hooks.default.performance === hooks.performance` - // (the `performance` singleton resolves identically from either). - "perf_hooks" => Some(js_create_native_module_namespace( - b"perf_hooks".as_ptr(), - "perf_hooks".len(), - )), "process" => Some(js_create_native_module_namespace( b"process".as_ptr(), "process".len(), @@ -812,6 +804,7 @@ fn should_cache_native_module_namespace(module_name: &str) -> bool { | "path.default" | "path.posix.default" | "path.win32.default" + | "perf_hooks.default" | "punycode" | "punycode.default" | "punycode.ucs2" @@ -936,12 +929,13 @@ unsafe fn native_module_property_by_name_impl( // `typeof performance === "object"`, `performance.timeOrigin` (a // constant), `performance.now` (a callable export), and // `constants.NODE_PERFORMANCE_GC_*` (constants) all dispatch coherently. - if module_name == "perf_hooks" && property_name == "performance" { + if matches!(module_name, "perf_hooks" | "perf_hooks.default") && property_name == "performance" + { // Singleton so `require("perf_hooks").performance` and the global // `performance` are the same object (Node identity guarantee, #1327). return crate::perf_hooks::performance_namespace(); } - if module_name == "perf_hooks" && property_name == "constants" { + if matches!(module_name, "perf_hooks" | "perf_hooks.default") && property_name == "constants" { // Its OWN tag. Sharing the `perf_hooks` tag made every read of the // constants object resolve against the MODULE's surface, so // `Object.keys(constants)` enumerated the export list instead of the diff --git a/crates/perry-runtime/src/object/native_module/module_keys.rs b/crates/perry-runtime/src/object/native_module/module_keys.rs index bb6a4c914e..09efe3f2bd 100644 --- a/crates/perry-runtime/src/object/native_module/module_keys.rs +++ b/crates/perry-runtime/src/object/native_module/module_keys.rs @@ -1281,6 +1281,22 @@ const VM_MODULE_NAMESPACE_KEYS: &[&[u8]] = &[ const VM_CONSTANTS_KEYS: &[&[u8]] = &[b"USE_MAIN_CONTEXT_DEFAULT_LOADER", b"DONT_CONTEXTIFY"]; +const PERF_HOOKS_DEFAULT_KEYS: &[&[u8]] = &[ + b"Performance", + b"PerformanceEntry", + b"PerformanceMark", + b"PerformanceMeasure", + b"PerformanceObserver", + b"PerformanceObserverEntryList", + b"PerformanceResourceTiming", + b"monitorEventLoopDelay", + b"eventLoopUtilization", + b"timerify", + b"createHistogram", + b"performance", + b"constants", +]; + // Linux-only open() flags: Node only enumerates these on platforms whose libc // defines them (e.g. `O_DIRECT`/`O_NOATIME` are absent on macOS), so gate the // enumerable-key tail by target so `Object.keys(constants)` matches Node here. @@ -1862,6 +1878,7 @@ pub(crate) fn native_module_enumerable_keys(module_name: &str) -> Option<&'stati b"constants", b"default", ]), + "perf_hooks.default" => Some(PERF_HOOKS_DEFAULT_KEYS), "perf_hooks.constants" => Some(&[ b"NODE_PERFORMANCE_GC_MAJOR", b"NODE_PERFORMANCE_GC_MINOR", diff --git a/crates/perry-runtime/src/object/native_module_dispatch.rs b/crates/perry-runtime/src/object/native_module_dispatch.rs index ded6e1039e..ff17dfdd2b 100644 --- a/crates/perry-runtime/src/object/native_module_dispatch.rs +++ b/crates/perry-runtime/src/object/native_module_dispatch.rs @@ -390,6 +390,7 @@ mod cjs_default_dispatch_tests { "path.default", "path.posix.default", "path.win32.default", + "perf_hooks.default", "process.default", "punycode.default", "querystring.default", From bf6b756a3677c64118137ca1bc3dfbded511ba01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:53:04 +0200 Subject: [PATCH 04/20] chore: number changeset for PR 9885 (cherry picked from commit fcc96888ddac6065ec0095b8be379ba29c415cd4) --- ...ault-export-keys.md => 9885-perf-hooks-default-export-keys.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9202-perf-hooks-default-export-keys.md => 9885-perf-hooks-default-export-keys.md} (100%) diff --git a/changelog.d/9202-perf-hooks-default-export-keys.md b/changelog.d/9885-perf-hooks-default-export-keys.md similarity index 100% rename from changelog.d/9202-perf-hooks-default-export-keys.md rename to changelog.d/9885-perf-hooks-default-export-keys.md From d01e3e2f16dca31fa476cbf04c8c03bc9874d680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:52:56 +0200 Subject: [PATCH 05/20] fix(hir): a later class accessor replaces an earlier one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ECMA-262 ClassDefinitionEvaluation installs class elements in source order, so a second `get x` / `set x` REPLACES the first. `ClassDecl::getters` / `::setters` are consumed with `iter().find(...)` — first match wins — but every accessor was appended with `push()`. The shadowed definition therefore stayed live and the one the program actually defines last was silently dropped. Every accessor shape is affected, not just getters. Against `node --experimental-strip-types`, before this change: instance getter 111 (expected 222) static + instance getter instance-first (expected instance-last) static-first (expected static-last) duplicate setters first:x (expected last:x) class expression 1 (expected 2) There is no diagnostic: the program reads a plausible value from the wrong accessor and keeps running. Found in Claude-of-Duty, whose `Spring3` pairs an early `set z` (damping) with a later `get z` (displacement) — legal, if unusual, and it relies on the read/write asymmetry the spec produces. Perry served the shadowed damping getter, so `lag.z` and `recPos.z` read 0.46 and 0.42 (their constructors' damping arguments) instead of displacements. That added +0.88 m to the first-person viewmodel's Z, moving the rig from 0.3 m in front of the camera to 0.58 m behind it. All 156 viewmodel nodes then clipped: the overlay pass ran and issued every draw, and produced no fragments. `record_class_accessor` overwrites an existing entry instead of appending. The replacement is keyed on `(name, is_static)`: a static and an instance accessor of the same name are distinct properties — one on the constructor, one on the prototype — and collapsing them would trade this bug for another. Verified: perry-hir 620 passed, perry-codegen 1912 passed. The regression test covers all four shapes above and fails on each without this change. (cherry picked from commit f572ce9b93f0c96bca271915f978c29c034ad6cd) --- crates/perry-hir/src/lower_decl/class_decl.rs | 115 ++++++++++++++++-- .../duplicate_class_accessor_last_wins.rs | 111 +++++++++++++++++ 2 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 crates/perry/tests/duplicate_class_accessor_last_wins.rs diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 500fa5ea25..5fc0375a0c 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -194,6 +194,49 @@ pub(crate) fn capture_class_source( } } +/// Record one class accessor, honouring ECMA-262's "a later definition of the +/// same key replaces the earlier one". +/// +/// `ClassDecl::getters` / `::setters` are consumed with `iter().find(...)`, so +/// the FIRST entry with a given name wins at lookup time. Appending +/// unconditionally therefore keeps a *shadowed* accessor alive and silently +/// drops the one the program actually defines last: +/// +/// ```js +/// class Spring3 { +/// get z() { return this.a.z; } // damping — shadowed +/// get z() { return this.c.x; } // displacement — must win +/// } +/// ``` +/// +/// Perry returned `this.a.z` here while every other engine returns +/// `this.c.x`. In Claude-of-Duty that handed the viewmodel rig a spring's +/// DAMPING COEFFICIENT (0.46) where it wanted a Z displacement, pushing the +/// weapon 0.88 m behind the camera, where it clipped and drew nothing. +/// +/// Static and instance accessors are distinct properties (one lives on the +/// constructor, one on the prototype) and may legally share a name, so the +/// replacement is keyed on `(name, is_static)` rather than the name alone. +fn record_class_accessor( + list: &mut Vec<(String, Function)>, + statics: &mut Vec, + name: String, + func: Function, + is_static: bool, +) { + let existing = list + .iter() + .enumerate() + .find_map(|(i, (n, _))| (n == &name && statics[i] == is_static).then_some(i)); + match existing { + Some(i) => list[i] = (name, func), + None => { + list.push((name, func)); + statics.push(is_static); + } + } +} + pub fn lower_class_decl( ctx: &mut LoweringContext, class_decl: &ast::ClassDecl, @@ -683,6 +726,10 @@ pub fn lower_class_decl( let mut static_methods = Vec::new(); let mut getters = Vec::new(); let mut setters = Vec::new(); + // Parallel staticness, so `record_class_accessor` can tell a static + // accessor from an instance one with the same name. + let mut getter_statics: Vec = Vec::new(); + let mut setter_statics: Vec = Vec::new(); let mut static_accessor_names: Vec = Vec::new(); let mut static_accessor_fn_ids: Vec = Vec::new(); let mut computed_members = Vec::new(); @@ -778,7 +825,13 @@ pub fn lower_class_decl( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - getters.push((prop_name, func)); + record_class_accessor( + &mut getters, + &mut getter_statics, + prop_name, + func, + method.is_static, + ); } ast::MethodKind::Setter => { // Setter: takes one parameter @@ -797,7 +850,13 @@ pub fn lower_class_decl( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - setters.push((prop_name, func)); + record_class_accessor( + &mut setters, + &mut setter_statics, + prop_name, + func, + method.is_static, + ); } ast::MethodKind::Method => { let mut func = with_static_member_context(ctx, method.is_static, |ctx| { @@ -915,7 +974,13 @@ pub fn lower_class_decl( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - getters.push((prop_name, func)); + record_class_accessor( + &mut getters, + &mut getter_statics, + prop_name, + func, + method.is_static, + ); } ast::MethodKind::Setter => { let prop_name = format!("#{}", method.key.name); @@ -924,7 +989,13 @@ pub fn lower_class_decl( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - setters.push((prop_name, func)); + record_class_accessor( + &mut setters, + &mut setter_statics, + prop_name, + func, + method.is_static, + ); } } } @@ -1577,6 +1648,10 @@ pub fn lower_class_from_ast( let mut static_methods = Vec::new(); let mut getters = Vec::new(); let mut setters = Vec::new(); + // Parallel staticness, so `record_class_accessor` can tell a static + // accessor from an instance one with the same name. + let mut getter_statics: Vec = Vec::new(); + let mut setter_statics: Vec = Vec::new(); let mut static_accessor_names: Vec = Vec::new(); let mut static_accessor_fn_ids: Vec = Vec::new(); let mut computed_members = Vec::new(); @@ -1663,7 +1738,13 @@ pub fn lower_class_from_ast( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - getters.push((prop_name, func)); + record_class_accessor( + &mut getters, + &mut getter_statics, + prop_name, + func, + method.is_static, + ); } ast::MethodKind::Setter => { let func = with_static_member_context(ctx, method.is_static, |ctx| { @@ -1681,7 +1762,13 @@ pub fn lower_class_from_ast( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - setters.push((prop_name, func)); + record_class_accessor( + &mut setters, + &mut setter_statics, + prop_name, + func, + method.is_static, + ); } ast::MethodKind::Method => { let mut func = with_static_member_context(ctx, method.is_static, |ctx| { @@ -1779,7 +1866,13 @@ pub fn lower_class_from_ast( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - getters.push((prop_name, func)); + record_class_accessor( + &mut getters, + &mut getter_statics, + prop_name, + func, + method.is_static, + ); } ast::MethodKind::Setter => { let prop_name = format!("#{}", method.key.name); @@ -1788,7 +1881,13 @@ pub fn lower_class_from_ast( static_accessor_names.push(prop_name.clone()); static_accessor_fn_ids.push(func.id); } - setters.push((prop_name, func)); + record_class_accessor( + &mut setters, + &mut setter_statics, + prop_name, + func, + method.is_static, + ); } } } diff --git a/crates/perry/tests/duplicate_class_accessor_last_wins.rs b/crates/perry/tests/duplicate_class_accessor_last_wins.rs new file mode 100644 index 0000000000..e3e28a0234 --- /dev/null +++ b/crates/perry/tests/duplicate_class_accessor_last_wins.rs @@ -0,0 +1,111 @@ +//! End-to-end regression coverage for duplicate class accessors. +//! +//! ECMA-262 ClassDefinitionEvaluation installs class elements in source order, +//! so a later accessor with the same key REPLACES an earlier one. Perry's HIR +//! appended every accessor to `ClassDecl::getters` / `::setters`, and those are +//! consumed with `iter().find(...)` — first match wins — so the SHADOWED +//! definition stayed live and the real one was dropped. +//! +//! The shape below is reduced from Claude-of-Duty's `Spring3`, which pairs an +//! early `set z` (damping) with a later `get z` (displacement). Perry returned +//! the damping coefficient from the shadowed getter, which put a first-person +//! weapon 0.88 m behind the camera, where it clipped and rendered nothing. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn a_later_class_accessor_replaces_an_earlier_one() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write( + &entry, + r#" +class Spring3 { + a = 111; + c = 222; + damping = 0; + + // Reading `.z` must reach the LAST getter; writing `.z` must still reach + // this setter, which no later definition replaces. + set z(v: number) { this.damping = v; } + get z(): number { return this.a; } + get z(): number { return this.c; } +} + +// Static and instance accessors are distinct properties and may share a name: +// replacing on the key alone would collapse them. +class Split { + static _s = "static-first"; + _i = "instance-first"; + static get v(): string { return Split._s; } + get v(): string { return this._i; } + static get v(): string { return "static-last"; } + get v(): string { return "instance-last"; } +} + +// A later setter replaces an earlier setter too. +class Sink { + hits: string[] = []; + set s(v: string) { this.hits.push("first:" + v); } + set s(v: string) { this.hits.push("last:" + v); } +} + +const spring = new Spring3(); +spring.z = 7; +console.log("spring", spring.z, spring.damping); + +console.log("split", new Split().v, Split.v); + +const sink = new Sink(); +sink.s = "x"; +console.log("sink", sink.hits.join("|"), sink.hits.length); + +// Accessors defined on a class EXPRESSION follow the same rule. +const Expr = class { + p = 1; + q = 2; + get w(): number { return this.p; } + get w(): number { return this.q; } +}; +console.log("expr", new Expr().w); +"#, + ) + .expect("write fixture"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled fixture"); + assert!( + run.status.success(), + "compiled fixture failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + + // Matches `node --experimental-strip-types` on the same source. + let expected = "spring 222 7\n\ + split instance-last static-last\n\ + sink last:x 1\n\ + expr 2\n"; + assert_eq!(String::from_utf8_lossy(&run.stdout), expected); +} From dc25c643d8a69ef68acbadff0ae97d77a846d7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:10:20 +0200 Subject: [PATCH 06/20] fix(fs): preserve promises namespace identity (cherry picked from commit 36d94d736bb284e72b9863fffb391f8db97d54ad) --- .../9202-fs-promises-namespace-identity.md | 3 +++ .../src/expr/property_get/tests.rs | 21 +++++++++++++++++++ .../src/expr/static_field_meta.rs | 16 ++++++++++++++ .../src/lower_call/native/mod.rs | 7 +------ crates/perry-codegen/src/nm_install.rs | 10 +++++++++ 5 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 changelog.d/9202-fs-promises-namespace-identity.md diff --git a/changelog.d/9202-fs-promises-namespace-identity.md b/changelog.d/9202-fs-promises-namespace-identity.md new file mode 100644 index 0000000000..8c9db28b17 --- /dev/null +++ b/changelog.d/9202-fs-promises-namespace-identity.md @@ -0,0 +1,3 @@ +### Fixed + +- Reuse the canonical `fs/promises` and `stream/promises` namespace objects when their native-module references become values. diff --git a/crates/perry-codegen/src/expr/property_get/tests.rs b/crates/perry-codegen/src/expr/property_get/tests.rs index c57e8d35ac..e23438090e 100644 --- a/crates/perry-codegen/src/expr/property_get/tests.rs +++ b/crates/perry-codegen/src/expr/property_get/tests.rs @@ -315,6 +315,27 @@ fn fs_parent_promises_property_installs_before_resolution() { ); } +#[test] +fn fs_promises_native_module_value_uses_submodule_singleton() { + let mut module = Module::new("fs_promises_native_module_value.ts"); + module.init = vec![Stmt::Return(Some(Expr::NativeModuleRef( + "fs/promises".to_string(), + )))]; + + let ir = String::from_utf8(compile_module(&module, ir_opts(false, None)).unwrap()) + .expect("LLVM IR should be UTF-8"); + let install = ir + .find("call void @js_node_submod_install_fs_promises()") + .unwrap_or_else(|| panic!("fs/promises must emit its submodule installer:\n{ir}")); + let namespace = ir + .find("call double @js_node_submodule_namespace") + .unwrap_or_else(|| panic!("fs/promises must use its submodule singleton:\n{ir}")); + assert!( + install < namespace, + "fs/promises installation must precede namespace creation:\n{ir}" + ); +} + /// #7753, paired with `pic_cache_words_match_codegen` in /// `perry-runtime/src/object/field_get_set/ic_miss.rs`. /// diff --git a/crates/perry-codegen/src/expr/static_field_meta.rs b/crates/perry-codegen/src/expr/static_field_meta.rs index e768a9685d..421c055eb6 100644 --- a/crates/perry-codegen/src/expr/static_field_meta.rs +++ b/crates/perry-codegen/src/expr/static_field_meta.rs @@ -910,6 +910,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // require("node:fs"); fs.constants` (call-result shape, fallback // path here) both produce a real namespace object. Expr::NativeModuleRef(name) => { + if let Some(submod_key) = crate::nm_install::native_namespace_submodule_key(name) { + let submod_idx = ctx.strings.intern(submod_key); + let submod_bytes_global = + format!("@{}", ctx.strings.entry(submod_idx).bytes_global); + let submod_len = submod_key.len().to_string(); + let install_sym = crate::nm_install::nm_submod_install_symbol(submod_key); + let blk = ctx.block(); + if let Some(symbol) = install_sym { + blk.call_void(symbol, &[]); + } + return Ok(blk.call( + DOUBLE, + "js_node_submodule_namespace", + &[(PTR, &submod_bytes_global), (I32, &submod_len)], + )); + } let mod_idx = ctx.strings.intern(name); let mod_bytes_global = format!("@{}", ctx.strings.entry(mod_idx).bytes_global); let mod_len_str = name.len().to_string(); diff --git a/crates/perry-codegen/src/lower_call/native/mod.rs b/crates/perry-codegen/src/lower_call/native/mod.rs index 7b384c1e47..8c255a1bd9 100644 --- a/crates/perry-codegen/src/lower_call/native/mod.rs +++ b/crates/perry-codegen/src/lower_call/native/mod.rs @@ -480,12 +480,7 @@ pub(crate) fn lower_native_method_call( // TAG_UNDEFINED sentinel below and `promises.realpath(p)` resolved // `undefined` (the compiled CLI's file cache then normalized every path to // `undefined` and each later fs call threw). - let normalized_module = module.strip_prefix("node:").unwrap_or(module); - let promises_submod_key = match normalized_module { - "fs/promises" => Some("fs_promises"), - "stream/promises" => Some("stream_promises"), - _ => None, - }; + let promises_submod_key = crate::nm_install::native_namespace_submodule_key(module); if let Some(submod_key) = promises_submod_key { let submod_label = crate::expr::emit_string_literal_global(ctx, submod_key); let install_sym = crate::nm_install::nm_submod_install_symbol(submod_key); diff --git a/crates/perry-codegen/src/nm_install.rs b/crates/perry-codegen/src/nm_install.rs index d742be0543..fab6d26474 100644 --- a/crates/perry-codegen/src/nm_install.rs +++ b/crates/perry-codegen/src/nm_install.rs @@ -151,6 +151,16 @@ pub(crate) fn nm_submod_install_symbol(key: &str) -> Option<&'static str> { } } +/// Native-module spellings whose value is implemented by the node-submodule +/// registry rather than a generic native-module namespace object. +pub(crate) fn native_namespace_submodule_key(name: &str) -> Option<&'static str> { + match name.strip_prefix("node:").unwrap_or(name) { + "fs/promises" => Some("fs_promises"), + "stream/promises" => Some("stream_promises"), + _ => None, + } +} + #[allow(dead_code)] // consumed only by codegen configurations that emit dispatch declarations pub(crate) const NM_SUBMOD_INSTALL_SYMBOLS: &[&str] = &[ "js_node_submod_install_vm", From 2e7fce7480fa874c5d37888db2351ed22a69bbdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:10:54 +0200 Subject: [PATCH 07/20] chore: number changeset for PR 9887 (cherry picked from commit b7f09520c4fc018843b4231e1b8f63ae3b602a92) --- ...mespace-identity.md => 9887-fs-promises-namespace-identity.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9202-fs-promises-namespace-identity.md => 9887-fs-promises-namespace-identity.md} (100%) diff --git a/changelog.d/9202-fs-promises-namespace-identity.md b/changelog.d/9887-fs-promises-namespace-identity.md similarity index 100% rename from changelog.d/9202-fs-promises-namespace-identity.md rename to changelog.d/9887-fs-promises-namespace-identity.md From 1e15ad4a80a011a4390430c8e1aec08eb3694f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:28:59 +0200 Subject: [PATCH 08/20] feat(codegen): match the Intl.Segmenter for-of, and count whether it fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the fourth member of the escape-analysis family (escape_news / escape_arrays / escape_objects): `collectors/segview.rs` recognises `for (let {segment: O} of X.segment(q))` and proves the segment RECORD never escapes, so the loop can eventually drive a native cursor instead of materialising one record per grapheme. That loop is the target: the allocation census ranks it 1/2/3 by count (172,032 records + 124,928 + 122,880 substrings per 400-character reply, 58 % of the top-30 allocation count), and the sample puts 60-85 % of active main-thread CPU inside it, under ink's wrapText. What is proven, and what deliberately is not. The only proof is that the record does not escape — every use of `__destruct_N` is one of the destructuring field reads the loop head itself emits. A use of the segment STRING is never a rejection: any use no view entry point answers is served by materialising the substring once into the same local, which is exactly what the loop costs today. So `O`'s uses are classified and counted, not gated. That is what separates "the record is gone" (v1) from "the loop allocates nothing" (v2, which needs the runtime's regexp_test). The escape proof is a count, and it is taken with perry_hir's collect_local_refs_stmt — the LocalId collector that handles every LocalId-bearing variant explicitly and delegates the rest to the walker whose match the compiler forces to be exhaustive. A new HIR variant embedding a LocalGet is therefore a compile error in the walker, not a silently missed use of the record. The `O`-use classifier is hand-written and can miss a shape, so it is checked against that same sound count and every unclassified occurrence is booked as "must materialise": an unrecognised use can make a site look less optimisable than it is, never more. No lowering. The tier's fact is populated and unread; the runtime's view-mode entry points do not exist yet. The counter is the point of the commit. A tier can be correct and never match (#9824), so PERRY_SEGVIEW_DIAG=1 reports every for-of site examined, the verdict, the rejection reason and the per-use tally — and it runs at the HIR-trace point, the last place before codegen, where the statements scanned are exactly the statements codegen consumes. That makes "does it fire on the real bundle?" answerable in HIR-lowering time instead of a full LLVM build. The env var is excluded from the build-level cache for the same reason --opt-report is: a cached build never lowers HIR, and a report that prints nothing reads exactly like a tier that never fired. Unit tests pin the matcher against the HIR shape a real --trace hir dump produces, including the two negative controls that matter: a record use hidden inside a closure rejects, and a `{segment, index}` head declines under its own name rather than firing. (cherry picked from commit 3bf49abf745490d2ccb02311c273501735caf952) --- .../perry-codegen/src/collectors/hir_facts.rs | 16 + crates/perry-codegen/src/collectors/mod.rs | 3 + .../perry-codegen/src/collectors/segview.rs | 886 ++++++++++++++++++ .../src/collectors/segview_tests.rs | 268 ++++++ crates/perry-codegen/src/lib.rs | 5 + .../perry/src/commands/compile/build_cache.rs | 10 + .../src/commands/compile/run_pipeline.rs | 15 + 7 files changed, 1203 insertions(+) create mode 100644 crates/perry-codegen/src/collectors/segview.rs create mode 100644 crates/perry-codegen/src/collectors/segview_tests.rs diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index 0257f43348..08cce2b446 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -167,6 +167,17 @@ pub(crate) struct EscapeFacts { pub fusible_uppercase_locals: HashSet, pub non_escaping_object_literals: HashMap>, pub non_escaping_object_literal_used_fields: HashMap>, + /// #9846, the fourth member of this family: `for (let {segment: O} of + /// X.segment(q))` sites whose segment RECORD provably never escapes, so + /// the loop can drive a native cursor instead of materialising one record + /// per grapheme (census site 1 — 172,032 allocations per 400-character cc + /// reply, the largest single site by count). + /// + /// Populated but not yet consumed: the lowering waits on the runtime's + /// view-mode entry points (`INTERFACE_segments_view.md` §9b). Same + /// in-progress shape as `PurityFacts` / `ShapeStabilityFacts` above. + #[allow(dead_code)] + pub segment_for_of_sites: Vec, } #[derive(Debug, Clone, Default)] @@ -677,6 +688,10 @@ pub(crate) fn collect_type_facts( stmts, &non_escaping_object_literals, ); + // #9846: the segment-record member of the escape family. Cheap by + // construction — `collect_segment_for_of_sites` walks the region only + // when it holds a `for…of` whose subject is an `X.segment(q)` call. + let segment_for_of_sites = super::segview::collect_segment_for_of_sites(stmts); let scalar_replaceable_object_locals = non_escaping_news .keys() .chain(non_escaping_object_literals.keys()) @@ -773,6 +788,7 @@ pub(crate) fn collect_type_facts( fusible_uppercase_locals, non_escaping_object_literals, non_escaping_object_literal_used_fields, + segment_for_of_sites, }, purity: PurityFacts { pure_helper_function_ids: clamp_fn_ids.clone(), diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 3257750191..f576335f32 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -51,6 +51,9 @@ mod repsel_benefit; mod safepoint_sites; mod scalar_method_dispatch; mod scalar_methods; +pub mod segview; +#[cfg(test)] +mod segview_tests; mod shadow_slots; pub(crate) mod spec_abi_sites; mod this_as_value; diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs new file mode 100644 index 0000000000..c9e82043a8 --- /dev/null +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -0,0 +1,886 @@ +//! Segment-view for-of matcher — the fourth member of the escape-analysis +//! family (`escape_news`, `escape_arrays`, `escape_objects`). +//! +//! # What it looks for +//! +//! `for (let {segment: O} of X.segment(q))` — the loop `string-width` runs, +//! which the allocation census ranks 1/2/3 by count (172k records + 125k + +//! 123k substrings per 400-character claude-code reply, 58 % of the top-30 +//! allocation count) and which the sample ranks as 60–85 % of active +//! main-thread CPU under ink's `wrapText`. +//! +//! After lowering, that loop is a fixed HIR shape (verified against +//! `--trace hir`, not assumed): +//! +//! ```text +//! Let { id: A, name: "__arr_A", init: GetIterator(Call { PropertyGet(S, "segment"), [input] }) } +//! For { +//! init: Let { id: R, name: "__result_R", init: Call(ExternFuncRef "js_for_of_next", [LocalGet(A)]) }, +//! condition: Not(PropertyGet(LocalGet(R), "done")), +//! update: LocalSet(R, Call(ExternFuncRef "js_for_of_next", [LocalGet(A)])), +//! body: [ Let { id: D, name: "__destruct_D", init: PropertyGet(LocalGet(R), "value") }, +//! Let { id: O, name: , init: PropertyGet(LocalGet(D), "segment") }, +//! ] +//! } +//! ``` +//! +//! # What it proves +//! +//! Only one thing, and it is the one the v1 runtime interface +//! (`INTERFACE_segments_view.md` §9b) needs: **the segment record `D` never +//! escapes**, because its every use is one of the destructuring field reads +//! that the loop head itself emits. When that holds the record is never +//! observed and never has to exist — census site 1, 172,032 allocations a +//! reply, all of it in the loop head. +//! +//! Whether the segment *substring* can also be skipped is a separate, weaker +//! question, and it is not a precondition: any use of `O` that no view entry +//! point answers is served by materialising the substring once into the same +//! local (`js_segments_view_segment`), which is exactly what the loop costs +//! today. So `O`'s uses are *classified and counted*, never fatal. The tally +//! is what says whether a site is also v2-ready (zero allocations per +//! grapheme) or only v1-ready (record elided, substring kept). +//! +//! # Soundness +//! +//! The escape proof is a *count*, and it is taken with +//! `perry_hir::collect_local_refs_stmt` — the repo's LocalId collector, which +//! handles every LocalId-bearing variant explicitly and delegates the rest to +//! `perry_hir::walker::walk_expr_children`, a match the compiler forces to be +//! exhaustive. A new HIR variant that embeds a `LocalGet` therefore cannot +//! silently hide a use of the record from this pass; it is a compile error in +//! the walker instead. That is the same reasoning `local_refs.rs`'s +//! `mark_all_candidate_refs_in_expr` catch-all exists for (#150), reached by +//! borrowing the sound walker rather than by re-deriving a conservative one. +//! +//! The `O`-use classifier is a hand-written recursive match, so it *can* fail +//! to recognise a shape — but it is checked against the same sound counter, +//! and every occurrence it did not classify is booked as `materialise`. An +//! unclassified use can therefore only make a site look *less* optimisable +//! than it is; it can never make one look more. +//! +//! # The counter is the falsifier +//! +//! A tier can be correct and never match (#9824). `PERRY_SEGVIEW_DIAG=1` +//! reports every for-of site examined, the verdict, and the rejection reason, +//! before codegen runs — so "it fires on the real bundle" is a measured line, +//! not an inference from the shape above. + +use std::collections::HashMap; + +use perry_hir::{Expr, Stmt, UnaryOp}; + +/// How each use of the destructured `segment` binding would be served. +/// +/// Only `code_point_at` is answerable by a v1 runtime cursor +/// (`INTERFACE_segments_view.md` §9b); the rest are counted so the v1/v2 line +/// is measured rather than assumed. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct SegmentUseTally { + /// `O.codePointAt(k)` — `js_segments_view_code_point_at`. v1. + pub code_point_at: u32, + /// `O.charCodeAt(k)` — v2 (`_char_code_at` was dropped from v1, §5). + pub char_code_at: u32, + /// `O.length` — v2 (`_length`, §5). + pub length: u32, + /// `RegExpTest { regex, string: LocalGet(O) }` — a *statically* proven + /// regex receiver. v2 (`_regexp_test`). + pub regexp_test_static: u32, + /// `recv.test(O)` where `recv` is an arbitrary expression — cc's + /// `g54.default().test(O)`. v2, and only behind the three-valued decline + /// (§5): `is RegExp` at the call site does not rule out a patched + /// `RegExp.prototype.test`. + pub regexp_test_dynamic: u32, + /// Everything else, including every occurrence the classifier did not + /// recognise. Each one forces the substring to be materialised, which is + /// what the loop pays today — never a rejection. + pub materialise: u32, +} + +impl SegmentUseTally { + /// Total occurrences of the binding, from the sound counter. + pub fn total(&self) -> u32 { + self.code_point_at + + self.char_code_at + + self.length + + self.regexp_test_static + + self.regexp_test_dynamic + + self.materialise + } + + /// True when no use needs the substring: the loop reaches zero + /// allocations per grapheme once the v2 entry points exist. + pub fn view_answerable_v2(&self) -> bool { + self.materialise == 0 + } + + /// True when every use is answered by the two in-loop v1 entry points. + pub fn view_answerable_v1(&self) -> bool { + self.total() == self.code_point_at + } +} + +/// Why a `for…of` whose subject is an `X.segment(q)` call did or did not +/// admit record elision. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SegViewVerdict { + /// The record provably never escapes: every use is a destructuring read + /// of `segment` in the loop head. v1 applies. + Fires, + /// The `For` head is not the canonical `js_for_of_next` protocol (a + /// collection-view rewrite, an index arm, an async iterator…). Not a + /// failure of the proof — a different lowering, which this tier does not + /// speak. + HeadNotCanonical, + /// The head destructures no `segment` key, so this is somebody else's + /// `.segment(x)`. + NoSegmentKey, + /// The record binding is boxed (captured by a closure that outlives the + /// step). Rejected before the count, because a box read is not a + /// `LocalGet` of the record. + BoxedRecord, + /// The record is used somewhere other than the head's own field reads. + RecordEscapes { + uses: usize, + destructure_reads: usize, + }, + /// The record does not escape, but the head reads fields v1 cannot answer + /// (`index` / `input` / `isWordLike` are v2 symbols, §5). The spec path + /// runs, at no cost, until those exist. + RecordFieldsBeyondV1 { keys: Vec }, +} + +impl SegViewVerdict { + pub fn reason(&self) -> &'static str { + match self { + SegViewVerdict::Fires => "fires", + SegViewVerdict::HeadNotCanonical => "head_not_canonical", + SegViewVerdict::NoSegmentKey => "no_segment_key", + SegViewVerdict::BoxedRecord => "boxed_record", + SegViewVerdict::RecordEscapes { .. } => "record_escapes", + SegViewVerdict::RecordFieldsBeyondV1 { .. } => "record_fields_beyond_v1", + } + } +} + +/// One examined `for (… of X.segment(q))` site. +#[derive(Debug, Clone)] +pub struct SegmentForOfSite { + /// `__arr_A`, the `GetIterator` local. + pub iter_id: u32, + /// `__result_R`, the iteration-result local. + pub result_id: u32, + /// `__destruct_D`, the segment record. + pub record_id: u32, + pub record_name: String, + /// The local the `segment` key is destructured into, when there is one. + pub segment_id: Option, + /// The subject is the `X.segment(q)` call itself, so `open(segmenter, + /// input)` — the two-argument form (§3) — is matchable and no `Segments` + /// object is ever built. False for a for-of over a variable that already + /// holds one, which takes the weaker `open_segments`. + pub two_arg_open: bool, + /// Field keys the head destructures off the record, in head order. + pub record_keys: Vec, + /// Uses of `__arr_A` beyond the two `js_for_of_next` calls in the head — + /// the iterator-close protocol (`it.return`) contributes 2. Reported + /// because the lowering replaces `A` with a cursor and has to serve them. + pub iter_extra_uses: usize, + pub segment_uses: SegmentUseTally, + pub verdict: SegViewVerdict, +} + +impl SegmentForOfSite { + pub fn fires(&self) -> bool { + self.verdict == SegViewVerdict::Fires + } +} + +/// Collect every `for (… of X.segment(q))` site in one lowered region +/// (function body, method body, module init), including sites inside nested +/// closures. +/// +/// Cheap by construction: the region is only walked at all when it contains a +/// `GetIterator` whose subject is a `.segment(…)` call. +pub fn collect_segment_for_of_sites(stmts: &[Stmt]) -> Vec { + let mut candidates: Vec = Vec::new(); + for_each_stmt_list(stmts, &mut |list| find_candidates_in_list(list, &mut candidates)); + // A statement list should be visited exactly once by `for_each_stmt_list`; + // pin that rather than trusting it, so a descent bug shows up as a missing + // site and never as a double-counted one. + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); + candidates.retain(|c| seen.insert(c.iter_id)); + if candidates.is_empty() { + return Vec::new(); + } + + // One sound reference census for the whole region, taken with the repo's + // exhaustive LocalId collector. Multiset: a local referenced three times + // contributes three entries. + let mut refs: Vec = Vec::new(); + let mut visited = std::collections::HashSet::new(); + for stmt in stmts { + perry_hir::collect_local_refs_stmt(stmt, &mut refs, &mut visited); + } + let mut ref_counts: HashMap = HashMap::new(); + for id in refs { + *ref_counts.entry(id).or_insert(0) += 1; + } + + // Boxed locals: a `Preallocate*`/`ReleaseBoxes` mention means the binding + // lives in a heap cell a closure can reach, and a read of it is not a + // `LocalGet` this census would see. + let mut boxed: std::collections::HashSet = std::collections::HashSet::new(); + for_each_stmt_list(stmts, &mut |list| { + for s in list { + match s { + Stmt::PreallocateBoxes(ids) + | Stmt::PreallocateTdzBoxes(ids) + | Stmt::ReleaseBoxes(ids) => boxed.extend(ids.iter().copied()), + _ => {} + } + } + }); + + candidates + .into_iter() + .map(|c| finish_candidate(c, &ref_counts, &boxed)) + .collect() +} + +// ── candidate discovery ──────────────────────────────────────────────────── + +struct Candidate { + iter_id: u32, + result_id: u32, + record_id: u32, + record_name: String, + segment_id: Option, + two_arg_open: bool, + record_keys: Vec, + head_canonical: bool, + /// The whole `For` statement, for the `O`-use classification pass. + for_stmt: Stmt, +} + +fn find_candidates_in_list(list: &[Stmt], out: &mut Vec) { + for (i, s) in list.iter().enumerate() { + let Stmt::Let { + id: iter_id, + init: Some(Expr::GetIterator(subject)), + .. + } = s + else { + continue; + }; + // The subject decides which `open` form the lowering can use, and + // whether this is a segment loop at all. + let two_arg_open = match subject.as_ref() { + Expr::Call { callee, args, .. } => { + matches!(callee.as_ref(), Expr::PropertyGet { property, .. } if property == "segment") + && args.len() == 1 + } + _ => false, + }; + if !two_arg_open { + // A for-of over a variable already holding a `Segments` is the + // weaker one-argument form. Nothing in the measured workload has + // that shape, and matching it would need a type fact this pass + // does not have, so it is not a candidate — and not a rejection + // either, because it is not known to be a segment loop. + continue; + } + // The `For` is the next statement, possibly inside a label. + let Some(for_stmt) = next_for_stmt(list, i + 1) else { + continue; + }; + let Stmt::For { + init, + condition, + update, + body, + } = for_stmt + else { + continue; + }; + + let head_canonical = head_is_canonical(*iter_id, init, condition, update); + let (result_id, record_id, record_name, record_keys, segment_id) = + match destructure_head(body) { + Some(v) => v, + None => continue, + }; + + out.push(Candidate { + iter_id: *iter_id, + result_id, + record_id, + record_name, + segment_id, + two_arg_open, + record_keys, + head_canonical, + for_stmt: for_stmt.clone(), + }); + } +} + +fn next_for_stmt(list: &[Stmt], idx: usize) -> Option<&Stmt> { + let mut s = list.get(idx)?; + while let Stmt::Labeled { body, .. } = s { + s = body.as_ref(); + } + matches!(s, Stmt::For { .. }).then_some(s) +} + +/// `init`/`condition`/`update` are the `js_for_of_next` protocol over +/// `iter_id`. Anything else is a different lowering (collection view, index +/// arm, async iterator) that this tier does not speak. +fn head_is_canonical( + iter_id: u32, + init: &Option>, + condition: &Option, + update: &Option, +) -> bool { + let Some(init) = init else { return false }; + let Stmt::Let { + id: result_id, + init: Some(init_expr), + .. + } = init.as_ref() + else { + return false; + }; + if !is_for_of_next_call(init_expr, iter_id) { + return false; + } + let done_ok = matches!( + condition, + Some(Expr::Unary { op: UnaryOp::Not, operand }) + if matches!(operand.as_ref(), + Expr::PropertyGet { object, property, .. } + if property == "done" && matches!(object.as_ref(), Expr::LocalGet(r) if r == result_id)) + ); + let update_ok = matches!( + update, + Some(Expr::LocalSet(r, v)) if r == result_id && is_for_of_next_call(v, iter_id) + ); + done_ok && update_ok +} + +fn is_for_of_next_call(e: &Expr, iter_id: u32) -> bool { + let Expr::Call { callee, args, .. } = e else { + return false; + }; + let Expr::ExternFuncRef { name, .. } = callee.as_ref() else { + return false; + }; + name == "js_for_of_next" + && args.len() == 1 + && matches!(args[0], Expr::LocalGet(a) if a == iter_id) +} + +/// Peel the leading destructuring reads the for-of head emits: +/// `Let D = result.value`, then one `Let = D.` per destructured field. +type HeadShape = (u32, u32, String, Vec, Option); + +fn destructure_head(body: &[Stmt]) -> Option { + let Stmt::Let { + id: record_id, + name: record_name, + init: Some(Expr::PropertyGet { + object, property, .. + }), + .. + } = body.first()? + else { + return None; + }; + if property != "value" { + return None; + } + let Expr::LocalGet(result_id) = object.as_ref() else { + return None; + }; + + let mut keys = Vec::new(); + let mut segment_id = None; + for s in body.iter().skip(1) { + let Stmt::Let { + id, + init: + Some(Expr::PropertyGet { + object, property, .. + }), + .. + } = s + else { + break; + }; + if !matches!(object.as_ref(), Expr::LocalGet(r) if r == record_id) { + break; + } + if property == "segment" && segment_id.is_none() { + segment_id = Some(*id); + } + keys.push(property.clone()); + } + + Some(( + *result_id, + *record_id, + record_name.clone(), + keys, + segment_id, + )) +} + +// ── the proof ────────────────────────────────────────────────────────────── + +fn finish_candidate( + c: Candidate, + ref_counts: &HashMap, + boxed: &std::collections::HashSet, +) -> SegmentForOfSite { + let record_uses = ref_counts.get(&c.record_id).copied().unwrap_or(0); + let destructure_reads = c.record_keys.len(); + // The two `js_for_of_next(A)` calls the head itself emits. + let iter_extra_uses = ref_counts + .get(&c.iter_id) + .copied() + .unwrap_or(0) + .saturating_sub(2); + + let mut segment_uses = SegmentUseTally::default(); + if let Some(seg_id) = c.segment_id { + let sound_total = ref_counts.get(&seg_id).copied().unwrap_or(0) as u32; + // One of those is the head's own `Let O = D.segment` init? No — that + // is a use of the RECORD, not of `O`. Every counted reference of + // `seg_id` is a real use in the body. + classify_segment_uses_in_stmt(&c.for_stmt, seg_id, &mut segment_uses); + // The classifier is hand-written and can miss a shape; the census + // above cannot. Book the difference as "must materialise" so an + // unrecognised use can only understate what the view buys. + let classified = segment_uses.total(); + if sound_total > classified { + segment_uses.materialise += sound_total - classified; + } + } + + let verdict = if !c.head_canonical { + SegViewVerdict::HeadNotCanonical + } else if c.segment_id.is_none() { + SegViewVerdict::NoSegmentKey + } else if boxed.contains(&c.record_id) { + SegViewVerdict::BoxedRecord + } else if record_uses != destructure_reads { + SegViewVerdict::RecordEscapes { + uses: record_uses, + destructure_reads, + } + } else if c.record_keys.iter().any(|k| k != "segment") { + SegViewVerdict::RecordFieldsBeyondV1 { + keys: c.record_keys.clone(), + } + } else { + SegViewVerdict::Fires + }; + + SegmentForOfSite { + iter_id: c.iter_id, + result_id: c.result_id, + record_id: c.record_id, + record_name: c.record_name, + segment_id: c.segment_id, + two_arg_open: c.two_arg_open, + record_keys: c.record_keys, + iter_extra_uses, + segment_uses, + verdict, + } +} + +// ── `O`-use classification ───────────────────────────────────────────────── + +fn classify_segment_uses_in_stmt(stmt: &Stmt, seg: u32, t: &mut SegmentUseTally) { + // Deep: every expression owned by `stmt` OR by any statement nested in it. + // Closure bodies are reached from `classify_segment_uses_in_expr`'s own + // `Expr::Closure` arm, which is the only path into them, so nothing is + // visited twice. + for_each_expr_in_stmt_shallow(stmt, &mut |e| classify_segment_uses_in_expr(e, seg, t)); + for_each_child_stmt(stmt, &mut |s| classify_segment_uses_in_stmt(s, seg, t)); +} + +/// Every statement nested directly inside `stmt` (branches, loop bodies, +/// catch/finally, switch cases, a `For` init). Does NOT enter closure bodies: +/// those hang off expressions and are handled by the expression classifier. +fn for_each_child_stmt(stmt: &Stmt, f: &mut impl FnMut(&Stmt)) { + match stmt { + Stmt::If { + then_branch, + else_branch, + .. + } => { + then_branch.iter().for_each(&mut *f); + if let Some(e) = else_branch { + e.iter().for_each(&mut *f); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => body.iter().for_each(f), + Stmt::For { init, body, .. } => { + if let Some(i) = init { + f(i); + } + body.iter().for_each(f); + } + Stmt::Labeled { body, .. } => f(body), + Stmt::Try { + body, + catch, + finally, + } => { + body.iter().for_each(&mut *f); + if let Some(c) = catch { + c.body.iter().for_each(&mut *f); + } + if let Some(fin) = finally { + fin.iter().for_each(&mut *f); + } + } + Stmt::Switch { cases, .. } => { + for c in cases { + c.body.iter().for_each(&mut *f); + } + } + _ => {} + } +} + +fn classify_segment_uses_in_expr(e: &Expr, seg: u32, t: &mut SegmentUseTally) { + // Recognise the parent shapes BEFORE descending, so the `LocalGet(seg)` + // inside them is attributed rather than falling into `materialise`. + match e { + Expr::Call { callee, args, .. } => { + if let Expr::PropertyGet { + object, property, .. + } = callee.as_ref() + { + // `O.codePointAt(k)` / `O.charCodeAt(k)` + if matches!(object.as_ref(), Expr::LocalGet(id) if *id == seg) + && args.len() == 1 + && (property == "codePointAt" || property == "charCodeAt") + { + if property == "codePointAt" { + t.code_point_at += 1; + } else { + t.char_code_at += 1; + } + for a in args { + classify_segment_uses_in_expr(a, seg, t); + } + return; + } + // `recv.test(O)` — the receiver is arbitrary (cc's + // `g54.default()`), so the runtime decides per call. + if property == "test" + && args.len() == 1 + && matches!(&args[0], Expr::LocalGet(id) if *id == seg) + && !matches!(object.as_ref(), Expr::LocalGet(id) if *id == seg) + { + t.regexp_test_dynamic += 1; + classify_segment_uses_in_expr(object, seg, t); + return; + } + } + } + Expr::RegExpTest { regex, string } => { + if matches!(string.as_ref(), Expr::LocalGet(id) if *id == seg) { + t.regexp_test_static += 1; + classify_segment_uses_in_expr(regex, seg, t); + return; + } + } + Expr::PropertyGet { + object, property, .. + } => { + if property == "length" && matches!(object.as_ref(), Expr::LocalGet(id) if *id == seg) { + t.length += 1; + return; + } + } + Expr::LocalGet(id) if *id == seg => { + t.materialise += 1; + return; + } + Expr::Closure { body, .. } => { + for s in body { + classify_segment_uses_in_stmt(s, seg, t); + } + // Param defaults are Expr children; the walker below covers them. + } + _ => {} + } + perry_hir::walker::walk_expr_children(e, &mut |child| { + classify_segment_uses_in_expr(child, seg, t) + }); +} + +// ── generic descent ──────────────────────────────────────────────────────── + +/// Call `f` on every statement list in the region, including the bodies of +/// nested closures. The `Stmt` arms are enumerated here; the `Expr` descent +/// that finds `Expr::Closure` delegates to the exhaustive walker. +fn for_each_stmt_list(stmts: &[Stmt], f: &mut impl FnMut(&[Stmt])) { + f(stmts); + for s in stmts { + for_each_stmt_list_in_stmt(s, f); + } +} + +fn for_each_stmt_list_in_stmt(s: &Stmt, f: &mut impl FnMut(&[Stmt])) { + match s { + Stmt::If { + then_branch, + else_branch, + .. + } => { + for_each_stmt_list(then_branch, f); + if let Some(e) = else_branch { + for_each_stmt_list(e, f); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => for_each_stmt_list(body, f), + Stmt::For { init, body, .. } => { + if let Some(i) = init { + for_each_stmt_list_in_stmt(i, f); + } + for_each_stmt_list(body, f); + } + Stmt::Labeled { body, .. } => for_each_stmt_list_in_stmt(body, f), + Stmt::Try { + body, + catch, + finally, + } => { + for_each_stmt_list(body, f); + if let Some(c) = catch { + for_each_stmt_list(&c.body, f); + } + if let Some(fin) = finally { + for_each_stmt_list(fin, f); + } + } + Stmt::Switch { cases, .. } => { + for c in cases { + for_each_stmt_list(&c.body, f); + } + } + _ => {} + } + // Closure bodies hanging off any expression in this statement. + for_each_expr_in_stmt_shallow(s, &mut |e| for_each_closure_body_in_expr(e, f)); +} + +fn for_each_closure_body_in_expr(e: &Expr, f: &mut impl FnMut(&[Stmt])) { + if let Expr::Closure { body, .. } = e { + for_each_stmt_list(body, f); + } + perry_hir::walker::walk_expr_children(e, &mut |child| for_each_closure_body_in_expr(child, f)); +} + +/// Every expression owned directly by `stmt` (not by its nested statements). +fn for_each_expr_in_stmt_shallow(stmt: &Stmt, f: &mut impl FnMut(&Expr)) { + match stmt { + Stmt::Let { init, .. } => { + if let Some(e) = init { + f(e); + } + } + Stmt::Expr(e) | Stmt::Throw(e) => f(e), + Stmt::Return(e) => { + if let Some(e) = e { + f(e); + } + } + Stmt::If { condition, .. } => f(condition), + Stmt::While { condition, .. } | Stmt::DoWhile { condition, .. } => f(condition), + Stmt::For { + init, + condition, + update, + .. + } => { + if let Some(i) = init { + for_each_expr_in_stmt_shallow(i, f); + } + if let Some(c) = condition { + f(c); + } + if let Some(u) = update { + f(u); + } + } + Stmt::Switch { discriminant, .. } => f(discriminant), + Stmt::Labeled { body, .. } => for_each_expr_in_stmt_shallow(body, f), + _ => {} + } +} + +// ── the counter ──────────────────────────────────────────────────────────── +// +// A tier can be correct and never match (#9824), and this campaign has hit +// "exists in source, not helping the binary" five times. So the matcher ships +// with the instrument that decides whether it fires on the workload, and the +// instrument runs at the HIR-trace point — after every transform, on exactly +// the statements codegen consumes — which a 10 MB bundle reaches in minutes +// rather than the hours a full LLVM build costs. + +/// `PERRY_SEGVIEW_DIAG=1`. +pub fn segview_diag_enabled() -> bool { + matches!(std::env::var("PERRY_SEGVIEW_DIAG"), Ok(v) if !v.is_empty() && v != "0") +} + +/// Every `Let _ = GetIterator(subject)` in a region, split by whether the +/// subject is an `X.segment(q)` call. The first number is the denominator +/// this tier is judged against: how many `for…of` loops exist at all. +pub fn count_for_of_sites(stmts: &[Stmt]) -> (usize, usize) { + let (mut all, mut segment) = (0usize, 0usize); + for_each_stmt_list(stmts, &mut |list| { + for s in list { + if let Stmt::Let { + init: Some(Expr::GetIterator(subject)), + .. + } = s + { + all += 1; + if let Expr::Call { callee, args, .. } = subject.as_ref() { + if args.len() == 1 + && matches!(callee.as_ref(), + Expr::PropertyGet { property, .. } if property == "segment") + { + segment += 1; + } + } + } + } + }); + (all, segment) +} + +/// Accumulated diagnostic over a whole compilation. +#[derive(Debug, Default)] +pub struct SegViewDiag { + /// `for…of` sites of every kind (the denominator). + pub for_of_sites: usize, + /// Of those, sites whose subject is an `X.segment(q)` call. + pub segment_subject_sites: usize, + /// Every examined segment site, with the region it was found in. + pub sites: Vec<(String, SegmentForOfSite)>, +} + +impl SegViewDiag { + pub fn scan_region(&mut self, region: &str, stmts: &[Stmt]) { + let (all, seg) = count_for_of_sites(stmts); + self.for_of_sites += all; + self.segment_subject_sites += seg; + if seg == 0 { + return; + } + for site in collect_segment_for_of_sites(stmts) { + self.sites.push((region.to_string(), site)); + } + } + + /// Scan one lowered module: init statements, every free function, and + /// every class constructor / method / accessor / static method. Sites + /// inside a nested closure are attributed to the named region that + /// encloses them, which is what a minified bundle gives us to name. + pub fn scan_module(&mut self, path: &str, m: &perry_hir::Module) { + self.scan_region(&format!("{path}::"), &m.init); + for f in &m.functions { + self.scan_region(&format!("{path}::{}", f.name), &f.body); + } + for c in &m.classes { + if let Some(ctor) = &c.constructor { + self.scan_region(&format!("{path}::{}.constructor", c.name), &ctor.body); + } + for meth in c.methods.iter().chain(c.static_methods.iter()) { + self.scan_region(&format!("{path}::{}.{}", c.name, meth.name), &meth.body); + } + for (name, f) in c.getters.iter().chain(c.setters.iter()) { + self.scan_region(&format!("{path}::{}.{name}", c.name), &f.body); + } + } + } + + /// Print the report to stderr. The lines are the falsifier: "fires=0" with + /// a named reason is a result, "fires=0" with no reason is the failure + /// mode this campaign keeps hitting. + pub fn report(&self) { + let mut fires = 0usize; + let mut v1_only = 0usize; + let mut v2_ready = 0usize; + let mut by_reason: std::collections::BTreeMap<&'static str, usize> = + std::collections::BTreeMap::new(); + + for (region, s) in &self.sites { + *by_reason.entry(s.verdict.reason()).or_insert(0) += 1; + if s.fires() { + fires += 1; + if s.segment_uses.view_answerable_v2() { + v2_ready += 1; + } else { + v1_only += 1; + } + } + let u = &s.segment_uses; + eprintln!( + "[segview] {region} record={} (id={}) verdict={} open={} keys=[{}] \ + iter_extra_uses={} O-uses: code_point_at={} char_code_at={} length={} \ + regexp_test_static={} regexp_test_dynamic={} materialise={}", + s.record_name, + s.record_id, + describe(&s.verdict), + if s.two_arg_open { "2-arg" } else { "1-arg" }, + s.record_keys.join(","), + s.iter_extra_uses, + u.code_point_at, + u.char_code_at, + u.length, + u.regexp_test_static, + u.regexp_test_dynamic, + u.materialise, + ); + } + + eprintln!( + "[segview] TOTALS for_of_sites={} segment_subject_sites={} examined={} fires={} \ + (v1_only={} v2_ready={})", + self.for_of_sites, + self.segment_subject_sites, + self.sites.len(), + fires, + v1_only, + v2_ready, + ); + let reasons = by_reason + .iter() + .map(|(k, v)| format!("{k}={v}")) + .collect::>() + .join(" "); + eprintln!("[segview] TOTALS verdicts: {reasons}"); + } +} + +fn describe(v: &SegViewVerdict) -> String { + match v { + SegViewVerdict::RecordEscapes { + uses, + destructure_reads, + } => format!("record_escapes(uses={uses},head_reads={destructure_reads})"), + SegViewVerdict::RecordFieldsBeyondV1 { keys } => { + format!("record_fields_beyond_v1({})", keys.join(",")) + } + other => other.reason().to_string(), + } +} diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs new file mode 100644 index 0000000000..090a9d7ec8 --- /dev/null +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -0,0 +1,268 @@ +//! #9846: the segment-view for-of matcher, pinned against the HIR shape that +//! `--trace hir` actually produces for +//! `for (let {segment: O} of X.segment(q))`. +//! +//! The shape below was transcribed from a real `perry compile --trace hir` +//! dump, not invented — the campaign's rule about never hand-typing a constant +//! into emitted code applies to hand-typing an IR shape into a test just as +//! much ([[perry-emitted-constant-transcription]]). If the lowering moves, +//! `head_not_canonical` is what these tests report, which is the honest +//! failure and the one the real-bundle counter would also report. +//! +//! The distinction each test exists to pin: **a use of the segment STRING is +//! never a rejection** (it costs one materialisation, which is what the loop +//! pays today); only a use of the segment RECORD is. + +use super::segview::{collect_segment_for_of_sites, SegViewVerdict}; +use perry_hir::types::Type; +use perry_hir::{CatchClause, Expr, Stmt, UnaryOp}; + +const ITER: u32 = 5; +const RESULT: u32 = 7; +const RECORD: u32 = 11; +const SEG: u32 = 9; + +fn pget(obj: Expr, prop: &str) -> Expr { + Expr::PropertyGet { + object: Box::new(obj), + property: prop.to_string(), + byte_offset: 0, + } +} + +fn call(callee: Expr, args: Vec) -> Expr { + Expr::Call { + callee: Box::new(callee), + args, + type_args: vec![], + byte_offset: 0, + } +} + +fn for_of_next() -> Expr { + call( + Expr::ExternFuncRef { + name: "js_for_of_next".to_string(), + param_types: vec![Type::Any], + return_type: Type::Any, + }, + vec![Expr::LocalGet(ITER)], + ) +} + +fn let_(id: u32, name: &str, init: Expr) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Any, + mutable: false, + init: Some(init), + } +} + +/// `let __arr_5 = GetIterator(rR_.segment(q))`. +fn iter_let() -> Stmt { + let_( + ITER, + "__arr_5", + Expr::GetIterator(Box::new(call( + pget(Expr::LocalGet(0), "segment"), + vec![Expr::LocalGet(3)], + ))), + ) +} + +/// The `For` with the canonical `js_for_of_next` head and the given body +/// after the destructuring lets. +fn for_stmt(destructure: Vec, body: Vec) -> Stmt { + let mut all = destructure; + // The real lowering wraps the user body in the iterator-close protocol. + all.push(Stmt::Try { + body, + catch: Some(CatchClause { + param: Some((12, "__forof_err_12".to_string())), + body: vec![Stmt::Throw(Expr::LocalGet(12))], + }), + finally: None, + }); + Stmt::For { + init: Some(Box::new(let_(RESULT, "__result_7", for_of_next()))), + condition: Some(Expr::Unary { + op: UnaryOp::Not, + operand: Box::new(pget(Expr::LocalGet(RESULT), "done")), + }), + update: Some(Expr::LocalSet(RESULT, Box::new(for_of_next()))), + body: all, + } +} + +/// `let __destruct_11 = __result_7.value; let O = __destruct_11.segment;` +fn destructure_segment_only() -> Vec { + vec![ + let_( + RECORD, + "__destruct_11", + pget(Expr::LocalGet(RESULT), "value"), + ), + let_(SEG, "O", pget(Expr::LocalGet(RECORD), "segment")), + ] +} + +fn region(body: Vec) -> Vec { + vec![iter_let(), for_stmt(destructure_segment_only(), body)] +} + +/// cc's body, in shape: one `codePointAt` and one regex test on the segment. +fn cc_body() -> Vec { + vec![ + let_( + 10, + "w", + call(pget(Expr::LocalGet(SEG), "codePointAt"), vec![Expr::Integer(0)]), + ), + Stmt::If { + condition: Expr::RegExpTest { + regex: Box::new(Expr::LocalGet(1)), + string: Box::new(Expr::LocalGet(SEG)), + }, + then_branch: vec![Stmt::Continue], + else_branch: None, + }, + ] +} + +#[test] +fn fires_on_the_cc_shape_and_names_the_two_view_entry_points() { + let sites = collect_segment_for_of_sites(®ion(cc_body())); + assert_eq!(sites.len(), 1, "exactly one segment for-of site"); + let s = &sites[0]; + assert_eq!( + s.verdict, + SegViewVerdict::Fires, + "the record's only uses are the head's own field reads" + ); + assert!(s.two_arg_open, "the subject is the `X.segment(q)` call itself"); + assert_eq!(s.record_keys, vec!["segment".to_string()]); + assert_eq!(s.segment_id, Some(SEG)); + assert_eq!(s.segment_uses.code_point_at, 1); + assert_eq!(s.segment_uses.regexp_test_static, 1); + assert_eq!( + s.segment_uses.materialise, 0, + "every use of the segment string is view-answerable, so this site is v2-ready" + ); + assert!(s.segment_uses.view_answerable_v2()); + assert!( + !s.segment_uses.view_answerable_v1(), + "the regex test is a v2 entry point; v1 must still materialise here" + ); +} + +/// The distinction the whole design rests on: an unclassifiable use of the +/// segment STRING costs a materialisation, it does not reject the site. +#[test] +fn a_use_of_the_segment_string_is_a_materialisation_not_a_rejection() { + let mut body = cc_body(); + body.push(Stmt::Expr(call( + Expr::LocalGet(42), + vec![Expr::LocalGet(SEG)], + ))); + let sites = collect_segment_for_of_sites(®ion(body)); + assert_eq!(sites[0].verdict, SegViewVerdict::Fires); + assert_eq!(sites[0].segment_uses.materialise, 1); + assert!(!sites[0].segment_uses.view_answerable_v2()); +} + +/// `recv.test(O)` with an opaque receiver — cc's `g54.default().test(O)` — is +/// classified apart from the statically-proven `RegExpTest`, because the +/// runtime declines it three-valued (§5 of the interface). +#[test] +fn an_opaque_test_receiver_is_counted_separately() { + let body = vec![Stmt::Expr(call( + pget(call(pget(Expr::LocalGet(54), "default"), vec![]), "test"), + vec![Expr::LocalGet(SEG)], + ))]; + let sites = collect_segment_for_of_sites(®ion(body)); + assert_eq!(sites[0].segment_uses.regexp_test_dynamic, 1); + assert_eq!(sites[0].segment_uses.materialise, 0); +} + +/// A use of the RECORD outside the head is the one thing that rejects, and it +/// must be caught even when it hides inside a closure the walker only reaches +/// through `Expr::Closure`. +#[test] +fn a_record_use_inside_a_closure_rejects() { + let mut body = cc_body(); + body.push(Stmt::Expr(Expr::Closure { + func_id: 1, + params: vec![], + return_type: Type::Any, + body: vec![Stmt::Return(Some(Expr::LocalGet(RECORD)))], + captures: vec![RECORD], + mutable_captures: vec![], + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: false, + })); + let sites = collect_segment_for_of_sites(®ion(body)); + assert!( + matches!(sites[0].verdict, SegViewVerdict::RecordEscapes { uses: 2, destructure_reads: 1 }), + "expected record_escapes, got {:?}", + sites[0].verdict + ); +} + +/// `{segment: O, index: I}` does not escape the record, but `index` is a v2 +/// symbol — the site must decline with its own reason rather than fire. +#[test] +fn a_second_destructured_field_declines_with_its_own_reason() { + let destructure = vec![ + let_( + RECORD, + "__destruct_11", + pget(Expr::LocalGet(RESULT), "value"), + ), + let_(SEG, "O", pget(Expr::LocalGet(RECORD), "segment")), + let_(8, "I", pget(Expr::LocalGet(RECORD), "index")), + ]; + let stmts = vec![iter_let(), for_stmt(destructure, cc_body())]; + let sites = collect_segment_for_of_sites(&stmts); + match &sites[0].verdict { + SegViewVerdict::RecordFieldsBeyondV1 { keys } => { + assert_eq!(keys, &vec!["segment".to_string(), "index".to_string()]) + } + other => panic!("expected record_fields_beyond_v1, got {other:?}"), + } +} + +/// A `for…of` lowered any other way (a collection view, an index arm) is not +/// this tier's shape and must say so rather than silently matching. +#[test] +fn a_non_canonical_head_declines_by_name() { + let mut stmts = region(cc_body()); + if let Stmt::For { update, .. } = &mut stmts[1] { + *update = Some(Expr::LocalSet(RESULT, Box::new(Expr::Undefined))); + } else { + panic!("shape"); + } + let sites = collect_segment_for_of_sites(&stmts); + assert_eq!(sites[0].verdict, SegViewVerdict::HeadNotCanonical); +} + +/// A `for…of` over anything but an `X.segment(q)` call is not examined at +/// all — the denominator, not a rejection. +#[test] +fn an_unrelated_for_of_is_not_a_candidate() { + let stmts = vec![ + let_( + ITER, + "__arr_5", + Expr::GetIterator(Box::new(Expr::LocalGet(99))), + ), + for_stmt(destructure_segment_only(), cc_body()), + ]; + assert!(collect_segment_for_of_sites(&stmts).is_empty()); +} diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 3a676a2d0b..7bf3f24751 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -80,6 +80,11 @@ pub use codegen::{ NamespaceEntry, NamespaceEntryKind, ObjectLiteralMethodCandidate, ShortSpreadMethodCandidate, }; pub use collectors::CjsPreambleCensus; +// #9846: the segment-view for-of matcher's counter. Exported so the +// driver can run it at the HIR-trace point — after every transform, on +// exactly the statements codegen consumes — instead of only inside a +// codegen run, which a 10 MB bundle does not reach in a usable time. +pub use collectors::segview::{segview_diag_enabled, SegViewDiag}; /// Return the guarded proven-`this` method-clone capabilities a native module /// may safely publish to importing codegen units. The first map contains all diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index f2eb61c696..a4e89d08a1 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -856,6 +856,16 @@ fn eligibility(args: &CompileArgs, project_root: &Path) -> Result<(), String> { if std::env::var("PERRY_NATIVEINST_DIAG").is_ok() { return Err("nativeinst-diag".to_string()); } + + // #9846: same reasoning as `opt-report` above, and the reason it is not + // optional. A cached build reuses the finished binary and never lowers + // HIR, so the segment-view counter would print nothing — and "nothing" + // reads exactly like "the tier never fired", which is the phantom-green + // this campaign keeps hitting. Excluded from the cache so a zero is a + // measured zero. + if std::env::var("PERRY_SEGVIEW_DIAG").is_ok() { + return Err("segview-diag".to_string()); + } if args.verify_native_regions || args.emit_attest || args.emit_sandbox { return Err("sidecar-or-verify".to_string()); } diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index a146350f3b..75c8f42bf7 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -1024,6 +1024,21 @@ pub fn run_with_parse_cache( perry_transform::module_const_fold::run(hir_module); } + // #9846: the segment-view for-of matcher's hit counter, taken here for + // the same reason the HIR trace is taken here — this is the last point + // before codegen, so the statements scanned are exactly the statements + // codegen consumes. Running it at this point (rather than only inside + // `collect_type_facts` on a rayon worker) is what makes "does the tier + // fire on the real bundle?" answerable in HIR-lowering time instead of a + // full LLVM build. Gated on `PERRY_SEGVIEW_DIAG`; costs nothing otherwise. + if perry_codegen::segview_diag_enabled() { + let mut diag = perry_codegen::SegViewDiag::default(); + for (path, hir_module) in &ctx.native_modules { + diag.scan_module(&path.display().to_string(), hir_module); + } + diag.report(); + } + if trace_hir { dump_hir_for_debug(&ctx, args.focus.as_deref()); } From b8b241d40b5c127ee3e04871026fb6b9b0bd5cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:33:55 +0200 Subject: [PATCH 09/20] style: cargo fmt the segment-view matcher and its tests (cherry picked from commit 23e67fa37eb069e525c635fc935821d71989d6e9) --- crates/perry-codegen/src/collectors/segview.rs | 11 ++++++----- .../src/collectors/segview_tests.rs | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index c9e82043a8..8a15f0cff6 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -204,7 +204,9 @@ impl SegmentForOfSite { /// `GetIterator` whose subject is a `.segment(…)` call. pub fn collect_segment_for_of_sites(stmts: &[Stmt]) -> Vec { let mut candidates: Vec = Vec::new(); - for_each_stmt_list(stmts, &mut |list| find_candidates_in_list(list, &mut candidates)); + for_each_stmt_list(stmts, &mut |list| { + find_candidates_in_list(list, &mut candidates) + }); // A statement list should be visited exactly once by `for_each_stmt_list`; // pin that rather than trusting it, so a descent bug shows up as a missing // site and never as a double-counted one. @@ -408,10 +410,9 @@ fn destructure_head(body: &[Stmt]) -> Option { for s in body.iter().skip(1) { let Stmt::Let { id, - init: - Some(Expr::PropertyGet { - object, property, .. - }), + init: Some(Expr::PropertyGet { + object, property, .. + }), .. } = s else { diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index 090a9d7ec8..df6289d3b8 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -118,7 +118,10 @@ fn cc_body() -> Vec { let_( 10, "w", - call(pget(Expr::LocalGet(SEG), "codePointAt"), vec![Expr::Integer(0)]), + call( + pget(Expr::LocalGet(SEG), "codePointAt"), + vec![Expr::Integer(0)], + ), ), Stmt::If { condition: Expr::RegExpTest { @@ -141,7 +144,10 @@ fn fires_on_the_cc_shape_and_names_the_two_view_entry_points() { SegViewVerdict::Fires, "the record's only uses are the head's own field reads" ); - assert!(s.two_arg_open, "the subject is the `X.segment(q)` call itself"); + assert!( + s.two_arg_open, + "the subject is the `X.segment(q)` call itself" + ); assert_eq!(s.record_keys, vec!["segment".to_string()]); assert_eq!(s.segment_id, Some(SEG)); assert_eq!(s.segment_uses.code_point_at, 1); @@ -209,7 +215,13 @@ fn a_record_use_inside_a_closure_rejects() { })); let sites = collect_segment_for_of_sites(®ion(body)); assert!( - matches!(sites[0].verdict, SegViewVerdict::RecordEscapes { uses: 2, destructure_reads: 1 }), + matches!( + sites[0].verdict, + SegViewVerdict::RecordEscapes { + uses: 2, + destructure_reads: 1 + } + ), "expected record_escapes, got {:?}", sites[0].verdict ); From c3c83975b51632e466a6ed0ff17b47a5ad89d572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:34:12 +0200 Subject: [PATCH 10/20] docs(changelog): add the segment-view matcher fragment (cherry picked from commit afbf1b773400d0b510e01e3c3682a38b3565a004) --- .../9846-segment-view-for-of-matcher.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 changelog.d/9846-segment-view-for-of-matcher.md diff --git a/changelog.d/9846-segment-view-for-of-matcher.md b/changelog.d/9846-segment-view-for-of-matcher.md new file mode 100644 index 0000000000..4ca62c52bd --- /dev/null +++ b/changelog.d/9846-segment-view-for-of-matcher.md @@ -0,0 +1,28 @@ +Compile-time matcher for the `Intl.Segmenter` `for…of` loop, with the counter +that decides whether it fires. + +`for (let {segment: O} of X.segment(q))` is where claude-code spends most of a +turn: the allocation census ranks it 1/2/3 by count (172,032 segment records +plus 247,808 substrings per 400-character reply, 58 % of the top-30 allocation +count), and a `sample` puts 60–85 % of active main-thread CPU inside it, under +ink's `wrapText`. The loop reads one code point per grapheme and retains +nothing. + +`collectors/segview.rs` joins `escape_news` / `escape_arrays` / +`escape_objects` as the family's fourth member. It proves one thing: the +segment RECORD never escapes, because every use of the synthetic +`__destruct_N` binding is one of the destructuring field reads the loop head +itself emits. Uses of the segment STRING are classified and counted but never +gate the proof — a use no view entry point can answer is served by +materialising the substring once, which is what the loop costs today. + +The escape proof is taken with `perry_hir::collect_local_refs_stmt`, whose +descent bottoms out in the walker the compiler forces to be exhaustive, so a +new HIR variant embedding a `LocalGet` cannot silently hide a use of the +record. + +No lowering yet: the fact is populated and unread until the runtime's +segment-view entry points exist. `PERRY_SEGVIEW_DIAG=1` reports every site +examined, its verdict and its per-use tally at the HIR-trace point, and is +excluded from the build-level cache so a report of zero is a measured zero +rather than a build that never lowered HIR. From e038b88c85e3852ad05b2ec3e7262f9a227e6a17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:35:25 +0200 Subject: [PATCH 11/20] docs: point the segment-view references at the issue that exists (#9843) The first draft cited #9846, a number I had not checked and which does not exist. Comment-only change; the fragment is renamed to match. This is the same failure the segmenter lane caught in the brief's '#8364', which has no reference anywhere in the tree either. (cherry picked from commit 717961d74079865bce1150d9e4eb0890ee9e5a88) --- ...-for-of-matcher.md => 9843-segment-view-for-of-matcher.md} | 0 crates/perry-codegen/src/collectors/hir_facts.rs | 4 ++-- crates/perry-codegen/src/collectors/segview_tests.rs | 2 +- crates/perry-codegen/src/lib.rs | 2 +- crates/perry/src/commands/compile/build_cache.rs | 2 +- crates/perry/src/commands/compile/run_pipeline.rs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename changelog.d/{9846-segment-view-for-of-matcher.md => 9843-segment-view-for-of-matcher.md} (100%) diff --git a/changelog.d/9846-segment-view-for-of-matcher.md b/changelog.d/9843-segment-view-for-of-matcher.md similarity index 100% rename from changelog.d/9846-segment-view-for-of-matcher.md rename to changelog.d/9843-segment-view-for-of-matcher.md diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index 08cce2b446..5a65978bb1 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -167,7 +167,7 @@ pub(crate) struct EscapeFacts { pub fusible_uppercase_locals: HashSet, pub non_escaping_object_literals: HashMap>, pub non_escaping_object_literal_used_fields: HashMap>, - /// #9846, the fourth member of this family: `for (let {segment: O} of + /// #9843, the fourth member of this family: `for (let {segment: O} of /// X.segment(q))` sites whose segment RECORD provably never escapes, so /// the loop can drive a native cursor instead of materialising one record /// per grapheme (census site 1 — 172,032 allocations per 400-character cc @@ -688,7 +688,7 @@ pub(crate) fn collect_type_facts( stmts, &non_escaping_object_literals, ); - // #9846: the segment-record member of the escape family. Cheap by + // #9843: the segment-record member of the escape family. Cheap by // construction — `collect_segment_for_of_sites` walks the region only // when it holds a `for…of` whose subject is an `X.segment(q)` call. let segment_for_of_sites = super::segview::collect_segment_for_of_sites(stmts); diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index df6289d3b8..39c9cd8041 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -1,4 +1,4 @@ -//! #9846: the segment-view for-of matcher, pinned against the HIR shape that +//! #9843: the segment-view for-of matcher, pinned against the HIR shape that //! `--trace hir` actually produces for //! `for (let {segment: O} of X.segment(q))`. //! diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 7bf3f24751..c7be76002c 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -80,7 +80,7 @@ pub use codegen::{ NamespaceEntry, NamespaceEntryKind, ObjectLiteralMethodCandidate, ShortSpreadMethodCandidate, }; pub use collectors::CjsPreambleCensus; -// #9846: the segment-view for-of matcher's counter. Exported so the +// #9843: the segment-view for-of matcher's counter. Exported so the // driver can run it at the HIR-trace point — after every transform, on // exactly the statements codegen consumes — instead of only inside a // codegen run, which a 10 MB bundle does not reach in a usable time. diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index a4e89d08a1..2e50446cf8 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -857,7 +857,7 @@ fn eligibility(args: &CompileArgs, project_root: &Path) -> Result<(), String> { return Err("nativeinst-diag".to_string()); } - // #9846: same reasoning as `opt-report` above, and the reason it is not + // #9843: same reasoning as `opt-report` above, and the reason it is not // optional. A cached build reuses the finished binary and never lowers // HIR, so the segment-view counter would print nothing — and "nothing" // reads exactly like "the tier never fired", which is the phantom-green diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index 75c8f42bf7..b191edef52 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -1024,7 +1024,7 @@ pub fn run_with_parse_cache( perry_transform::module_const_fold::run(hir_module); } - // #9846: the segment-view for-of matcher's hit counter, taken here for + // #9843: the segment-view for-of matcher's hit counter, taken here for // the same reason the HIR trace is taken here — this is the last point // before codegen, so the statements scanned are exactly the statements // codegen consumes. Running it at this point (rather than only inside From 217cb97f65716920328493296c0f00b066e97c92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 08:04:44 +0200 Subject: [PATCH 12/20] fix(codegen): classify the folded StringCodePointAt node as a segment view use The bundle counter reported `code_point_at=0, materialise=1` for `N$6` in cli_2.1.112.js -- the string-width loop that is 60-85 % of claude-code's active main-thread CPU -- where the probe had reported 1 and 0. Cause: perry's JS pipeline folds `O.codePointAt(k)` into the dedicated `Expr::StringCodePointAt { string, index }` node. The classifier matched only the generic `Call(PropertyGet(O, "codePointAt"), [k])` shape, which is what a TypeScript probe produces. Exactly one occurrence moved buckets, which is the signature of a single unmatched shape and nothing else. Two things this does not change: the escape proof (the record's non-escape is a count from `collect_local_refs_stmt`, not from this classifier) and any verdict. Only the per-use tally moves, and only in the direction of reporting more of what the runtime view can answer. Why the wrong number was visible at all: every occurrence the classifier does not recognise is reconciled against that sound count and booked as "must materialise", so an unmatched shape under-reports optimisability and can never over-report it. A classifier that guessed instead of reconciling would have reported `code_point_at=0, materialise=0` here and looked correct. That property is the reason the tallies can be believed. The rule the miss establishes, now recorded in the module docs: a shape that reproduces on a probe is not proof it reproduces on the bundle. The bundle counter is 32 seconds -- run it after every change to this classifier. (cherry picked from commit 7c6f64be52423ce89977d2780a156e31f4af6c81) --- .../perry-codegen/src/collectors/segview.rs | 25 +++++++++++++ .../src/collectors/segview_tests.rs | 36 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index 8a15f0cff6..5a71b4989e 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -59,6 +59,17 @@ //! unclassified use can therefore only make a site look *less* optimisable //! than it is; it can never make one look more. //! +//! That property is not decorative: it is what caught this pass's own blind +//! spot. The first version matched only the generic +//! `Call(PropertyGet(O, "codePointAt"), [k])` shape — which is what a +//! TypeScript probe produces — and on `cli_2.1.112.js` it reported +//! `code_point_at=0, materialise=1` for the one loop that is 60-85 % of +//! claude-code's CPU, because the JS pipeline folds that call into +//! `Expr::StringCodePointAt`. A classifier that guessed instead of +//! reconciling would have reported `code_point_at=0, materialise=0` and looked +//! correct. **A shape that reproduces on a probe is not proof it reproduces on +//! the bundle**; run the bundle counter (32 seconds) after every change here. +//! //! # The counter is the falsifier //! //! A tier can be correct and never match (#9824). `PERRY_SEGVIEW_DIAG=1` @@ -594,6 +605,20 @@ fn classify_segment_uses_in_expr(e: &Expr, seg: u32, t: &mut SegmentUseTally) { } } } + // `O.codePointAt(k)` AFTER the JS pipeline has folded it. This arm is + // the one the real bundle needed and the TypeScript probe did not: + // perry lowers a proven string receiver's `.codePointAt` to this + // dedicated node, while the probe kept the generic `Call(PropertyGet…)` + // shape above. Measuring the bundle is what found it — the sound count + // saw the occurrence, this match did not, and the difference was booked + // as `materialise`, so the tally under-reported and never over-reported. + Expr::StringCodePointAt { string, index } => { + if matches!(string.as_ref(), Expr::LocalGet(id) if *id == seg) { + t.code_point_at += 1; + classify_segment_uses_in_expr(index, seg, t); + return; + } + } Expr::RegExpTest { regex, string } => { if matches!(string.as_ref(), Expr::LocalGet(id) if *id == seg) { t.regexp_test_static += 1; diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index 39c9cd8041..812d51d505 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -278,3 +278,39 @@ fn an_unrelated_for_of_is_not_a_candidate() { ]; assert!(collect_segment_for_of_sites(&stmts).is_empty()); } + +/// The bundle regression. `O.codePointAt(0)` survives as a generic +/// `Call(PropertyGet…)` when the receiver's type is unknown — which is what a +/// TypeScript probe produces — but perry's JS pipeline folds it into +/// `Expr::StringCodePointAt`. The first version of the classifier matched only +/// the former, so on `cli_2.1.112.js` the loop that is 60-85 % of claude-code's +/// CPU reported `code_point_at=0, materialise=1`. +/// +/// The reconciliation against the sound counter is why that read as a blind +/// spot rather than as a correct answer, and this test is why it cannot come +/// back. +#[test] +fn the_folded_code_point_at_node_is_classified_like_the_generic_call() { + let body = vec![let_( + 10, + "w", + Expr::StringCodePointAt { + string: Box::new(Expr::LocalGet(SEG)), + index: Box::new(Expr::Integer(0)), + }, + )]; + let sites = collect_segment_for_of_sites(®ion(body)); + assert_eq!(sites[0].verdict, SegViewVerdict::Fires); + assert_eq!( + sites[0].segment_uses.code_point_at, 1, + "the folded node must count as a code_point_at use, not a materialisation" + ); + assert_eq!( + sites[0].segment_uses.materialise, 0, + "nothing is left over for the sound counter to book conservatively" + ); + assert!( + sites[0].segment_uses.view_answerable_v1(), + "a loop whose only use is the folded codePointAt is answerable by v1 alone" + ); +} From c9d846fae707cde4f50320921d72435047abb4ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 09:34:52 +0200 Subject: [PATCH 13/20] feat(codegen): lower a proven segment for-of to the runtime view mode (v1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1 per `INTERFACE_segments_view.md` §9b: `js_segments_view_open` + `_next` in the loop, `_segment` once per step for the body. The body is NOT rewritten, so every use of the segment binding still sees an ordinary string. This removes the 48-byte segment RECORD per grapheme -- the allocation census's site 1, 172,032 per 400-character claude-code reply -- and the whole eager `build_segments` array with its two per-call closures. The substring stays; per-use `_code_point_at` / `_regexp_test` is the next increment. Emitted shape, for a site the matcher proves: Let recv = // hoisted, evaluated ONCE Let inp = // hoisted, evaluated ONCE Let cur = js_segments_view_open(recv, inp) // 0.0 on decline Let A = cur != 0 ? undefined : GetIterator(recv.segment(inp)) For { init: Let R = cur != 0 ? _next(cur) : js_for_of_next(A), cond: cur != 0 ? R == 1 : !R.done, update: R = cur != 0 ? _next(cur) : js_for_of_next(A), body: [Let O = cur != 0 ? _segment(cur) : R.value.segment, ] } Three properties this shape exists to get right, each with a test. The receiver and the input are HOISTED. Both appear on the accept path as `open`'s arguments and on the decline path as `recv.segment(inp)`, so leaving them in place would evaluate them twice: `getSegmenter().segment(next())` would call each twice. That is a miscompile, and claude-code's own `rR_.segment(q)` would never have exposed it because both operands there are side-effect-free. The `.segment` PROPERTY GET stays inside the decline arm. Hoisting the receiver does not hoist the member access, so a receiver whose `segment` is an accessor runs it exactly once, in its original position, on the path that needs it -- the ordering obligation §9f places on `open`'s decline path, honoured from the compiler side. The body is left byte-identical. That is what keeps `break` / `continue` / labels correct and avoids duplicating any `Expr::Closure` the body contains, which would carry a duplicate `FuncId`. The ternaries are real branches: `lower_conditional` emits a four-block CFG with a phi, so the decline arm's `GetIterator` does not run when `open` accepted. Verified before relying on it -- an eager select-style lowering would build the `Segments` on every loop and lose the entire per-call saving. Fresh LocalIds are seeded above every id the module mentions, declarations included and not only references: a local declared and never read still owns its id. DEFAULT OFF, behind `PERRY_SEGVIEW=1`. The runtime's view entry points do not exist yet, so an on-by-default rewrite would emit calls that fail to link. (cherry picked from commit 6dc4d24b9fd72653e13b8759a654721f640b9ffe) --- .../perry-codegen/src/collectors/segview.rs | 457 ++++++++++++++++++ .../src/collectors/segview_tests.rs | 101 ++++ crates/perry-codegen/src/lib.rs | 4 +- .../src/commands/compile/run_pipeline.rs | 11 + 4 files changed, 572 insertions(+), 1 deletion(-) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index 5a71b4989e..be0a9b6e15 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -910,3 +910,460 @@ fn describe(v: &SegViewVerdict) -> String { other => other.reason().to_string(), } } + +// ── the lowering ─────────────────────────────────────────────────────────── +// +// v1 per `INTERFACE_segments_view.md` §9b: `open` + `_next` in the loop, and +// `_segment` once per step for the body. The body is NOT rewritten — every use +// of the segment binding still sees an ordinary string — so this removes the +// 48-byte record per grapheme (census site 1, 172,032 per 400-character reply) +// and the whole eager `build_segments` array plus its two per-call closures, +// and leaves the substring. Per-use `_code_point_at` / `_regexp_test` is the +// next increment and needs the body rewritten site by site. +// +// Shape emitted for a firing site (`cur`, `recv`, `inp` are fresh locals): +// +// ```text +// Let recv = // hoisted: evaluated ONCE +// Let inp = // hoisted: evaluated ONCE +// Let cur = js_segments_view_open(recv, inp) // 0.0 on decline +// Let A = cur != 0 ? undefined : GetIterator(recv.segment(inp)) +// For { init: Let R = cur != 0 ? _next(cur) : js_for_of_next(A), +// cond: cur != 0 ? R == 1 : !R.done, +// update: R = cur != 0 ? _next(cur) : js_for_of_next(A), +// body: [Let O = cur != 0 ? _segment(cur) : R.value.segment, +// ] } +// ``` +// +// Three things this shape is chosen to get right. +// +// **The receiver and the input are hoisted.** Both appear on the accept path +// (as `open`'s arguments) and on the decline path (as `recv.segment(inp)`), so +// leaving them in place would evaluate them twice. `getSegmenter().segment(next())` +// would call each twice, which is a miscompile — and cc's own `rR_.segment(q)` +// would not have shown it, because both operands there are side-effect-free. +// +// **The `.segment` PROPERTY GET stays on the decline path only.** Hoisting the +// receiver does not hoist the member access, so a receiver whose `segment` is +// an accessor still runs it exactly once, in its original position, on the +// path that needs it. That is the ordering obligation §9f puts on `open`'s +// decline path, honoured from this side. +// +// **The ternaries are real branches.** `lower_conditional` emits a four-block +// CFG with a phi, so the decline arm's `GetIterator(recv.segment(inp))` does +// not execute when `open` accepted. A `select`-style eager lowering would +// build the `Segments` on every loop and lose the entire per-call saving. +// +// The body is left byte-identical, which is what keeps `break` / `continue` / +// labels correct and avoids duplicating any closure the body contains — a +// duplicated `Expr::Closure` would carry a duplicate `FuncId`. + +/// `PERRY_SEGVIEW=1`. **Default OFF**: the runtime's view entry points do not +/// exist yet, so an on-by-default rewrite would emit calls that fail to link. +pub fn segview_lowering_enabled() -> bool { + matches!(std::env::var("PERRY_SEGVIEW"), Ok(v) if !v.is_empty() && v != "0") +} + +fn extern_call(name: &str, args: Vec) -> Expr { + let param_types = vec![perry_hir::types::Type::Any; args.len()]; + Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: name.to_string(), + param_types, + return_type: perry_hir::types::Type::Any, + }), + args, + type_args: vec![], + byte_offset: 0, + } +} + +fn let_any(id: u32, name: &str, init: Expr) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: perry_hir::types::Type::Any, + mutable: true, + init: Some(init), + } +} + +/// `cur != 0` — the accept test. `open` returns `0.0` when it declines. +fn cursor_live(cur: u32) -> Expr { + Expr::Compare { + op: perry_hir::CompareOp::Ne, + left: Box::new(Expr::LocalGet(cur)), + right: Box::new(Expr::Number(0.0)), + } +} + +fn pick(cur: u32, accept: Expr, decline: Expr) -> Expr { + Expr::Conditional { + condition: Box::new(cursor_live(cur)), + then_expr: Box::new(accept), + else_expr: Box::new(decline), + } +} + +/// Rewrite one firing site in place. `list[i]` is the `Let A = GetIterator(…)` +/// and `list[i + 1]` (possibly inside a `Labeled`) is the `For`. +/// +/// Returns the number of statements inserted, so the caller can advance its +/// index correctly. +fn rewrite_site(list: &mut Vec, i: usize, site: &SegmentForOfSite, fresh: &mut u32) -> usize { + // Pull the receiver and the input out of the `GetIterator(X.segment(q))`. + let (recv_expr, input_expr) = match &list[i] { + Stmt::Let { + init: Some(Expr::GetIterator(subject)), + .. + } => match subject.as_ref() { + Expr::Call { callee, args, .. } => match callee.as_ref() { + Expr::PropertyGet { object, .. } if args.len() == 1 => { + (object.as_ref().clone(), args[0].clone()) + } + _ => return 0, + }, + _ => return 0, + }, + _ => return 0, + }; + + let recv = *fresh; + let inp = *fresh + 1; + let cur = *fresh + 2; + *fresh += 3; + + // The decline path rebuilds exactly what the site had, from the hoisted + // operands: `GetIterator(recv.segment(inp))`. The `.segment` property get + // is INSIDE this arm, so an accessor receiver runs it once, here, only. + let decline_iter = Expr::GetIterator(Box::new(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(recv)), + property: "segment".to_string(), + byte_offset: 0, + }), + args: vec![Expr::LocalGet(inp)], + type_args: vec![], + byte_offset: 0, + })); + + // Head rewrite. + let iter_id = site.iter_id; + let result_id = site.result_id; + if let Stmt::For { + init, + condition, + update, + body, + } = unwrap_for_mut(&mut list[i + 1]) + { + if let Some(init_stmt) = init { + if let Stmt::Let { init: Some(e), .. } = init_stmt.as_mut() { + *e = pick( + cur, + extern_call("js_segments_view_next", vec![Expr::LocalGet(cur)]), + extern_call("js_for_of_next", vec![Expr::LocalGet(iter_id)]), + ); + } + } + // `cur != 0 ? (R == 1) : !R.done` + *condition = Some(pick( + cur, + Expr::Compare { + op: perry_hir::CompareOp::Eq, + left: Box::new(Expr::LocalGet(result_id)), + right: Box::new(Expr::Number(1.0)), + }, + Expr::Unary { + op: UnaryOp::Not, + operand: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(result_id)), + property: "done".to_string(), + byte_offset: 0, + }), + }, + )); + *update = Some(Expr::LocalSet( + result_id, + Box::new(pick( + cur, + extern_call("js_segments_view_next", vec![Expr::LocalGet(cur)]), + extern_call("js_for_of_next", vec![Expr::LocalGet(iter_id)]), + )), + )); + + // Body head: drop the record `Let` entirely (this IS the elision) and + // bind the segment from the view, or from `R.value.segment` on the + // decline path. + if let Some(seg_id) = site.segment_id { + let seg_name = match &body[1] { + Stmt::Let { name, .. } => name.clone(), + _ => "O".to_string(), + }; + let bind = Stmt::Let { + id: seg_id, + name: seg_name, + ty: perry_hir::types::Type::Any, + mutable: false, + init: Some(pick( + cur, + extern_call("js_segments_view_segment", vec![Expr::LocalGet(cur)]), + Expr::PropertyGet { + object: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(result_id)), + property: "value".to_string(), + byte_offset: 0, + }), + property: "segment".to_string(), + byte_offset: 0, + }, + )), + }; + body.remove(0); // the `Let __destruct_N = R.value` + body[0] = bind; // was `Let O = __destruct_N.segment` + } + } + + // Statement rewrite: hoist, open, and the conditional iterator. + list[i] = let_any(recv, "__segview_recv", recv_expr); + list.insert(i + 1, let_any(inp, "__segview_input", input_expr)); + list.insert( + i + 2, + let_any( + cur, + "__segview_cursor", + extern_call( + "js_segments_view_open", + vec![Expr::LocalGet(recv), Expr::LocalGet(inp)], + ), + ), + ); + list.insert( + i + 3, + let_any( + iter_id, + "__segview_iter", + pick(cur, Expr::Undefined, decline_iter), + ), + ); + 3 +} + +fn unwrap_for_mut(s: &mut Stmt) -> &mut Stmt { + let mut cur = s; + loop { + match cur { + Stmt::Labeled { body, .. } => cur = body.as_mut(), + other => return other, + } + } +} + +/// Rewrite every firing site in one statement list and its nested lists. +fn rewrite_stmts(list: &mut Vec, fresh: &mut u32, count: &mut usize) { + // Nested lists first: rewriting an outer window never moves an inner one, + // but doing children first keeps the indices below trivially valid. + for s in list.iter_mut() { + rewrite_in_stmt(s, fresh, count); + } + + let mut i = 0usize; + while i + 1 < list.len() { + let sites = collect_segment_for_of_sites(std::slice::from_ref(&list[i])); + // `collect_segment_for_of_sites` needs the window, not one statement. + let window: Vec = list[i..=i + 1].to_vec(); + let sites = if sites.is_empty() { + collect_segment_for_of_sites(&window) + } else { + sites + }; + if let Some(site) = sites.iter().find(|s| s.fires()) { + let inserted = rewrite_site(list, i, site, fresh); + if inserted > 0 { + *count += 1; + i += inserted + 2; + continue; + } + } + i += 1; + } +} + +fn rewrite_in_stmt(s: &mut Stmt, fresh: &mut u32, count: &mut usize) { + match s { + Stmt::If { + then_branch, + else_branch, + .. + } => { + rewrite_stmts(then_branch, fresh, count); + if let Some(e) = else_branch { + rewrite_stmts(e, fresh, count); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => rewrite_stmts(body, fresh, count), + Stmt::For { init, body, .. } => { + if let Some(i) = init { + rewrite_in_stmt(i, fresh, count); + } + rewrite_stmts(body, fresh, count); + } + Stmt::Labeled { body, .. } => rewrite_in_stmt(body, fresh, count), + Stmt::Try { + body, + catch, + finally, + } => { + rewrite_stmts(body, fresh, count); + if let Some(c) = catch { + rewrite_stmts(&mut c.body, fresh, count); + } + if let Some(f) = finally { + rewrite_stmts(f, fresh, count); + } + } + Stmt::Switch { cases, .. } => { + for c in cases.iter_mut() { + rewrite_stmts(&mut c.body, fresh, count); + } + } + _ => {} + } + // Closure bodies hang off expressions. + for_each_expr_in_stmt_shallow_mut(s, &mut |e| rewrite_closure_bodies(e, fresh, count)); +} + +fn rewrite_closure_bodies(e: &mut Expr, fresh: &mut u32, count: &mut usize) { + if let Expr::Closure { body, .. } = e { + rewrite_stmts(body, fresh, count); + } + perry_hir::walker::walk_expr_children_mut(e, &mut |child| { + rewrite_closure_bodies(child, fresh, count) + }); +} + +fn for_each_expr_in_stmt_shallow_mut(stmt: &mut Stmt, f: &mut impl FnMut(&mut Expr)) { + match stmt { + Stmt::Let { init, .. } => { + if let Some(e) = init { + f(e); + } + } + Stmt::Expr(e) | Stmt::Throw(e) => f(e), + Stmt::Return(e) => { + if let Some(e) = e { + f(e); + } + } + Stmt::If { condition, .. } => f(condition), + Stmt::While { condition, .. } | Stmt::DoWhile { condition, .. } => f(condition), + Stmt::For { + init, + condition, + update, + .. + } => { + if let Some(i) = init { + for_each_expr_in_stmt_shallow_mut(i, f); + } + if let Some(c) = condition { + f(c); + } + if let Some(u) = update { + f(u); + } + } + Stmt::Switch { discriminant, .. } => f(discriminant), + Stmt::Labeled { body, .. } => for_each_expr_in_stmt_shallow_mut(body, f), + _ => {} + } +} + +/// The largest LocalId the module mentions anywhere — declarations included, +/// not only references. A local that is declared and never read still owns its +/// id, so seeding fresh ids from the reference maximum alone would collide +/// with it. +fn max_local_id_in_module(m: &perry_hir::Module) -> u32 { + let mut max = 0u32; + let mut note_stmts = |stmts: &[Stmt], max: &mut u32| { + for_each_stmt_list(stmts, &mut |list| { + for s in list { + match s { + Stmt::Let { id, .. } => *max = (*max).max(*id), + Stmt::PreallocateBoxes(ids) + | Stmt::PreallocateTdzBoxes(ids) + | Stmt::ReleaseBoxes(ids) => { + for id in ids { + *max = (*max).max(*id); + } + } + Stmt::Try { catch: Some(c), .. } => { + if let Some((id, _)) = &c.param { + *max = (*max).max(*id); + } + } + _ => {} + } + } + }); + let mut refs = Vec::new(); + let mut visited = std::collections::HashSet::new(); + for s in stmts { + perry_hir::collect_local_refs_stmt(s, &mut refs, &mut visited); + } + for id in refs { + *max = (*max).max(id); + } + }; + note_stmts(&m.init, &mut max); + for f in &m.functions { + for p in &f.params { + max = max.max(p.id); + } + note_stmts(&f.body, &mut max); + } + for c in &m.classes { + let mut fns: Vec<&perry_hir::Function> = Vec::new(); + if let Some(ctor) = &c.constructor { + fns.push(ctor); + } + fns.extend(c.methods.iter()); + fns.extend(c.static_methods.iter()); + fns.extend(c.getters.iter().map(|(_, f)| f)); + fns.extend(c.setters.iter().map(|(_, f)| f)); + for f in fns { + for p in &f.params { + max = max.max(p.id); + } + note_stmts(&f.body, &mut max); + } + } + max +} + +/// Rewrite every firing segment for-of in a module. Three new locals are +/// minted per site, seeded above every id the module already uses. Returns how +/// many sites were rewritten. +pub fn segview_rewrite_module(m: &mut perry_hir::Module) -> usize { + let mut fresh = max_local_id_in_module(m).saturating_add(1); + let mut count = 0usize; + rewrite_stmts(&mut m.init, &mut fresh, &mut count); + for f in m.functions.iter_mut() { + rewrite_stmts(&mut f.body, &mut fresh, &mut count); + } + for c in m.classes.iter_mut() { + if let Some(ctor) = c.constructor.as_mut() { + rewrite_stmts(&mut ctor.body, &mut fresh, &mut count); + } + for meth in c.methods.iter_mut().chain(c.static_methods.iter_mut()) { + rewrite_stmts(&mut meth.body, &mut fresh, &mut count); + } + for (_, f) in c.getters.iter_mut().chain(c.setters.iter_mut()) { + rewrite_stmts(&mut f.body, &mut fresh, &mut count); + } + } + if segview_diag_enabled() { + eprintln!("[segview] REWROTE {count} site(s) in module {}", m.name); + } + count +} diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index 812d51d505..09cf02a9ef 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -314,3 +314,104 @@ fn the_folded_code_point_at_node_is_classified_like_the_generic_call() { "a loop whose only use is the folded codePointAt is answerable by v1 alone" ); } + +// ── the lowering ─────────────────────────────────────────────────────────── + +use super::segview::segview_rewrite_module; + +fn module_with(stmts: Vec) -> perry_hir::Module { + let mut m = perry_hir::Module::new("t"); + m.init = stmts; + m +} + +fn render(m: &perry_hir::Module) -> String { + format!("{:?}", m.init) +} + +/// The shape the lowering must emit, pinned on the parts that carry meaning. +#[test] +fn the_rewrite_elides_the_record_and_keeps_a_spec_path() { + let mut m = module_with(region(cc_body())); + assert_eq!(segview_rewrite_module(&mut m), 1); + let out = render(&m); + + assert!( + out.contains("js_segments_view_open"), + "the two-argument open must be emitted: {out}" + ); + assert!( + out.contains("js_segments_view_next"), + "the in-loop advance must be emitted" + ); + assert!( + out.contains("js_segments_view_segment"), + "v1 materialises the segment once per step" + ); + assert!( + out.contains("js_for_of_next"), + "the spec path must survive for the decline case" + ); + assert!( + !out.contains("__destruct_"), + "the record binding is what this removes; it must be gone: {out}" + ); +} + +/// The receiver and the input appear on BOTH arms, so they must be evaluated +/// once and read from locals — not re-evaluated in the decline arm. A receiver +/// with a side effect would otherwise run twice. +#[test] +fn the_receiver_and_input_are_hoisted_exactly_once() { + let mut m = module_with(region(cc_body())); + assert_eq!(segview_rewrite_module(&mut m), 1); + let out = render(&m); + assert!(out.contains("__segview_recv"), "receiver hoisted"); + assert!(out.contains("__segview_input"), "input hoisted"); + // `LocalGet(0)` was the receiver and `LocalGet(3)` the input in `region`. + // After the rewrite each must appear exactly ONCE — in its hoist. + assert_eq!( + out.matches("LocalGet(0)").count(), + 1, + "the receiver is evaluated once, not on both arms: {out}" + ); + assert_eq!( + out.matches("LocalGet(3)").count(), + 1, + "the input is evaluated once, not on both arms: {out}" + ); +} + +/// The `.segment` property get must stay inside the decline arm, so a receiver +/// whose `segment` is an accessor runs it exactly once, in its original +/// position, and never on the accepted path. +#[test] +fn the_segment_property_get_stays_on_the_decline_arm_only() { + let mut m = module_with(region(cc_body())); + segview_rewrite_module(&mut m); + let out = render(&m); + assert_eq!( + out.matches("property: \"segment\"").count(), + 2, + "exactly two: the decline arm's `recv.segment(inp)` and the decline \ + arm's `R.value.segment` — never on the accepted path: {out}" + ); +} + +/// A site that does not fire must be left byte-identical. +#[test] +fn a_declining_site_is_not_rewritten() { + let destructure = vec![ + let_( + RECORD, + "__destruct_11", + pget(Expr::LocalGet(RESULT), "value"), + ), + let_(SEG, "O", pget(Expr::LocalGet(RECORD), "segment")), + let_(8, "I", pget(Expr::LocalGet(RECORD), "index")), + ]; + let mut m = module_with(vec![iter_let(), for_stmt(destructure, cc_body())]); + let before = render(&m); + assert_eq!(segview_rewrite_module(&mut m), 0); + assert_eq!(before, render(&m), "a declining site must be untouched"); +} diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index c7be76002c..3e3ea4fd81 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -84,7 +84,9 @@ pub use collectors::CjsPreambleCensus; // driver can run it at the HIR-trace point — after every transform, on // exactly the statements codegen consumes — instead of only inside a // codegen run, which a 10 MB bundle does not reach in a usable time. -pub use collectors::segview::{segview_diag_enabled, SegViewDiag}; +pub use collectors::segview::{ + segview_diag_enabled, segview_lowering_enabled, segview_rewrite_module, SegViewDiag, +}; /// Return the guarded proven-`this` method-clone capabilities a native module /// may safely publish to importing codegen units. The first map contains all diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index b191edef52..cc75c585e5 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -1031,6 +1031,17 @@ pub fn run_with_parse_cache( // `collect_type_facts` on a rayon worker) is what makes "does the tier // fire on the real bundle?" answerable in HIR-lowering time instead of a // full LLVM build. Gated on `PERRY_SEGVIEW_DIAG`; costs nothing otherwise. + // #9843: the segment-view lowering. Default OFF (`PERRY_SEGVIEW=1`) because + // the runtime's view entry points do not exist yet, so an on-by-default + // rewrite would emit calls that fail to link. Runs here, at the same point + // as the counter and the HIR trace, so what it rewrites is exactly what + // codegen consumes. + if perry_codegen::segview_lowering_enabled() { + for hir_module in ctx.native_modules.values_mut() { + perry_codegen::segview_rewrite_module(hir_module); + } + } + if perry_codegen::segview_diag_enabled() { let mut diag = perry_codegen::SegViewDiag::default(); for (path, hir_module) in &ctx.native_modules { From 06c1c36107872ea912f7fd426c87e022203dc12b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 12:37:28 +0200 Subject: [PATCH 14/20] diag(codegen): make the lowering report which entry point each site was lowered to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The counter and the rewrite answered two different questions and only one was being reported. `PERRY_SEGVIEW_DIAG=1` reports the CLASSIFICATION -- what each use of the segment binding could be answered by -- and it runs before the rewrite, because after it the shape is gone. So there was no way to confirm what was actually EMITTED, which was the counter's original purpose. Reordering the passes would trade one blind spot for the other. Instead the rewrite reports itself, and the two lines together say classification and emission: [segview] …::N$6 verdict=fires … code_point_at=1 regexp_test_dynamic=2 materialise=0 [segview-lower] __destruct_118613 open=1 next=1 segment=1 code_point_at=0 regexp_test=0 declined=none (classifier: code_point_at=1 regexp_test=2 materialise=0) Note deliberately that the emission line reports `code_point_at=0 regexp_test=0` even on a site the classifier scores as fully answerable. That is not a bug and it is not rounding: v1 emits `_segment` once per step and leaves the body untouched, so no use is answered from the view yet. The gap between the two lines IS the v1/v2 boundary, and having the instrument state it is better than having a reader infer from the design that v1 already routes `codePointAt` through the cursor. It will close when the per-use rewrite lands. Also fixes an `unused_mut` this pass introduced in `max_local_id_in_module`, which would have failed a `-D warnings` gate. Found by type-checking against the existing release artifacts -- zero disk cost, which mattered because the box is at 8 GiB and the integration build was killed by a disk watchdog. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp (cherry picked from commit e4ce2a89d455756d33d6b1a299e89161c8d98e87) --- .../perry-codegen/src/collectors/segview.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index be0a9b6e15..014fb06b13 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -1124,6 +1124,24 @@ fn rewrite_site(list: &mut Vec, i: usize, site: &SegmentForOfSite, fresh: } } + if segview_diag_enabled() { + // What was actually EMITTED, per site. Pairs with the classifier's + // `[segview]` line: that one says what each use of the segment binding + // COULD be answered by, this one says which entry point it now IS. + // v1 emits `_segment` once per step and leaves the body alone, so + // `code_point_at` and `regexp_test` are 0 here even on a site the + // classifier scored as answerable -- that difference is the v1/v2 gap, + // stated by the instrument instead of being inferred from the design. + let u = &site.segment_uses; + eprintln!( + "[segview-lower] {} open=1 next=1 segment=1 code_point_at=0 regexp_test=0 \ + declined=none (classifier: code_point_at={} regexp_test={} materialise={})", + site.record_name, + u.code_point_at, + u.regexp_test_static + u.regexp_test_dynamic, + u.materialise, + ); + } // Statement rewrite: hoist, open, and the conditional iterator. list[i] = let_any(recv, "__segview_recv", recv_expr); list.insert(i + 1, let_any(inp, "__segview_input", input_expr)); @@ -1285,7 +1303,7 @@ fn for_each_expr_in_stmt_shallow_mut(stmt: &mut Stmt, f: &mut impl FnMut(&mut Ex /// with it. fn max_local_id_in_module(m: &perry_hir::Module) -> u32 { let mut max = 0u32; - let mut note_stmts = |stmts: &[Stmt], max: &mut u32| { + let note_stmts = |stmts: &[Stmt], max: &mut u32| { for_each_stmt_list(stmts, &mut |list| { for s in list { match s { From d2e78429928b87bc9a468557c5da28a225a1dd55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 14:11:10 +0200 Subject: [PATCH 15/20] =?UTF-8?q?feat(codegen):=20v2=20=E2=80=94=20answer?= =?UTF-8?q?=20the=20segment's=20uses=20from=20the=20view,=20materialising?= =?UTF-8?q?=20nothing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1 bound the segment with `_segment` once per step and left the body alone, so it removed the record and kept the substring. v2 rewrites the USES: on a site where the classifier found nothing that needs the string, the accepted path materialises nothing at all and the loop reaches zero allocations per grapheme. That is where the remaining time is. perry-b4's I2 table puts ink's wrapText subtree at 80.2 % of active main-thread CPU (`E46`/`tI1` 4,238 of 5,152 samples, `u_N_24_6` 4,131 — about 4.1 s of a 5.2 s turn), with no dominant collector leaf left; the collector's share is minors landing inside this loop. v1 does not reach that. v2 does. Two substitutions, with very different risk. `O.codePointAt(k)` becomes `js_segments_view_code_point_at(cursor, k)` — a pure expression swap. `k` is unchanged: it is segment-relative and segment-bounded by the runtime's contract (§9d), the same bound the materialised substring had. `recv.test(O)` is the hard one. Read from #9870 rather than assumed: `js_segments_view_regexp_test(cursor, regex)` — CURSOR FIRST — returns true, false, or `undefined` meaning "I declined" (global/sticky regex, patched `RegExp.prototype.test` or an own `test`), and the runtime does NOT fall back internally, so the compiler must. `recv` is arbitrary — in cc it is `g54.default()`, an opaque call that must run exactly once per evaluation — so it cannot be repeated in the fallback arm. The emitted form is a pure expression, so no control flow is restructured: Sequence([ LocalSet(t_recv, ), // opaque call, ONCE LocalSet(t_res, _regexp_test(cursor, t_recv)), t_res === undefined ? t_recv.test(_segment(cursor)) : t_res ]) The materialisation is inside the decline arm, so the accepted path allocates nothing. Every rewritten use is GUARDED, not replaced: `cur != 0 ? : `. The loop body is shared between the accepted and declined paths, so the original expression must survive for the decline arm, where `O` holds a real string. On acceptance `O` is bound to `undefined` and never read, because every use takes the view arm — which is what makes the accepted path allocation-free without duplicating the body. A site with even one unanswerable use stays on v1: paying per-use guards on top of a materialisation that happens anyway is strictly worse. WHAT SUBSTITUTES FOR THE TESTS THIS COULD NOT BE RUN AGAINST. This box cannot build (its target was deleted to recover disk), so `rustfmt` and reading are the only gates. The pass therefore rewrites a CLONE of the body and keeps it only if the emission matches the classification exactly — same `code_point_at` count, same `regexp_test` count. If they disagree, some use was not rewritten and would read an unbound segment on the accepted path, so the clone is discarded and v1 is used. The check is the mechanism, not a comment. `[segview-lower]` now reports which arm was taken, so classifier and emission can be compared on the real bundle: [segview-lower] open=1 next=1 segment=0 code_point_at=1 regexp_test=2 declined=none (v2: …) [segview-lower] open=1 next=1 segment=1 code_point_at=0 regexp_test=0 declined=none (v1: …) Decline paths are unchanged. Three HIR-level tests added beside the v1 ones. NOT COMPILED AND NOT RUN — see the commit message above and §v2 of HANDOFF_segview_e2e.md for exactly what is unverified. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp (cherry picked from commit 089cb3bef3b8202671ed9a2564e7a33ff3129f40) --- .../perry-codegen/src/collectors/segview.rs | 325 ++++++++++++++++-- .../src/collectors/segview_tests.rs | 87 +++++ 2 files changed, 374 insertions(+), 38 deletions(-) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index 014fb06b13..172a7d1cd5 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -1100,48 +1100,105 @@ fn rewrite_site(list: &mut Vec, i: usize, site: &SegmentForOfSite, fresh: Stmt::Let { name, .. } => name.clone(), _ => "O".to_string(), }; - let bind = Stmt::Let { - id: seg_id, - name: seg_name, - ty: perry_hir::types::Type::Any, - mutable: false, - init: Some(pick( - cur, - extern_call("js_segments_view_segment", vec![Expr::LocalGet(cur)]), - Expr::PropertyGet { - object: Box::new(Expr::PropertyGet { - object: Box::new(Expr::LocalGet(result_id)), - property: "value".to_string(), - byte_offset: 0, - }), - property: "segment".to_string(), - byte_offset: 0, - }, - )), + let spec_bind = Expr::PropertyGet { + object: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(result_id)), + property: "value".to_string(), + byte_offset: 0, + }), + property: "segment".to_string(), + byte_offset: 0, }; - body.remove(0); // the `Let __destruct_N = R.value` - body[0] = bind; // was `Let O = __destruct_N.segment` + + // v2 is attempted only when the classifier found NO use that needs + // the substring. If even one does, materialising once (v1) is + // strictly better than materialising once AND paying the guards. + let answerable = site.segment_uses.regexp_test_static + + site.segment_uses.regexp_test_dynamic + + site.segment_uses.code_point_at; + let mut v2: Option<(V2Emission, Vec)> = None; + if site.segment_uses.materialise == 0 && answerable > 0 { + // Rewrite a CLONE and keep it only if the emission matches the + // classification exactly. This check stands in for the tests + // this pass could not be run against: if the two disagree, some + // use was not rewritten and would read an unbound segment on + // the accepted path, so the clone is discarded and v1 is used. + let mut trial: Vec = body[2..].to_vec(); + let mut probe_fresh = *fresh; + let mut emitted = V2Emission { + code_point_at: 0, + regexp_test: 0, + decls: Vec::new(), + }; + for st in trial.iter_mut() { + rewrite_uses_in_stmt(st, seg_id, cur, &mut probe_fresh, &mut emitted); + } + let agrees = emitted.code_point_at == site.segment_uses.code_point_at + && emitted.regexp_test + == site.segment_uses.regexp_test_static + + site.segment_uses.regexp_test_dynamic; + if agrees { + *fresh = probe_fresh; + v2 = Some((emitted, trial)); + } + } + + match v2 { + Some((emitted, trial)) => { + // The segment is never materialised on the accepted path: + // `O` is bound only for the decline arm, and every use is + // guarded, so on acceptance it is undefined and never read. + let bind = Stmt::Let { + id: seg_id, + name: seg_name, + ty: perry_hir::types::Type::Any, + mutable: false, + init: Some(pick(cur, Expr::Undefined, spec_bind)), + }; + let mut new_body = vec![bind]; + new_body.extend(emitted.decls.iter().cloned()); + new_body.extend(trial); + *body = new_body; + if segview_diag_enabled() { + eprintln!( + "[segview-lower] {} open=1 next=1 segment=0 code_point_at={} \ + regexp_test={} declined=none (v2: nothing materialised on the \ + accepted path)", + site.record_name, emitted.code_point_at, emitted.regexp_test, + ); + } + } + None => { + let bind = Stmt::Let { + id: seg_id, + name: seg_name, + ty: perry_hir::types::Type::Any, + mutable: false, + init: Some(pick( + cur, + extern_call("js_segments_view_segment", vec![Expr::LocalGet(cur)]), + spec_bind, + )), + }; + body.remove(0); // the `Let __destruct_N = R.value` + body[0] = bind; // was `Let O = __destruct_N.segment` + if segview_diag_enabled() { + let u = &site.segment_uses; + eprintln!( + "[segview-lower] {} open=1 next=1 segment=1 code_point_at=0 \ + regexp_test=0 declined=none (v1: classifier code_point_at={} \ + regexp_test={} materialise={})", + site.record_name, + u.code_point_at, + u.regexp_test_static + u.regexp_test_dynamic, + u.materialise, + ); + } + } + } } } - if segview_diag_enabled() { - // What was actually EMITTED, per site. Pairs with the classifier's - // `[segview]` line: that one says what each use of the segment binding - // COULD be answered by, this one says which entry point it now IS. - // v1 emits `_segment` once per step and leaves the body alone, so - // `code_point_at` and `regexp_test` are 0 here even on a site the - // classifier scored as answerable -- that difference is the v1/v2 gap, - // stated by the instrument instead of being inferred from the design. - let u = &site.segment_uses; - eprintln!( - "[segview-lower] {} open=1 next=1 segment=1 code_point_at=0 regexp_test=0 \ - declined=none (classifier: code_point_at={} regexp_test={} materialise={})", - site.record_name, - u.code_point_at, - u.regexp_test_static + u.regexp_test_dynamic, - u.materialise, - ); - } // Statement rewrite: hoist, open, and the conditional iterator. list[i] = let_any(recv, "__segview_recv", recv_expr); list.insert(i + 1, let_any(inp, "__segview_input", input_expr)); @@ -1385,3 +1442,195 @@ pub fn segview_rewrite_module(m: &mut perry_hir::Module) -> usize { } count } + +// ── v2: answer the uses from the view instead of materialising ───────────── +// +// v1 binds the segment with `_segment` once per step and leaves the body +// alone, so it removes the record and keeps the substring. v2 rewrites the +// USES, so on a site the classifier scored `materialise=0` nothing is +// materialised at all and the loop reaches zero allocations per grapheme. +// +// Two substitutions, and they have very different risk. +// +// `O.codePointAt(k)` -> `js_segments_view_code_point_at(cursor, k)` is a pure +// expression swap: same arity, same value, no temporaries, no control flow. +// `k` stays as written -- it is segment-relative and segment-bounded by the +// runtime's contract (§9d), which is the same bound the materialised substring +// had, so no clamping is added or removed here. +// +// `recv.test(O)` is the hard one, because `js_segments_view_regexp_test` +// returns THREE values: true, false, or `undefined` meaning "I declined" +// (global/sticky regex, or a patched `RegExp.prototype.test`). Read from +// #9870: the runtime does NOT fall back internally, so the compiler must. And +// `recv` is an arbitrary expression -- in cc it is `g54.default()`, an opaque +// call that must run exactly once per evaluation -- so it cannot simply be +// repeated in the fallback arm. +// +// The emitted form is a pure expression, so it works in any position without +// restructuring the body's control flow: +// +// ```text +// Sequence([ +// LocalSet(t_recv, ), // opaque call, ONCE +// LocalSet(t_res, _regexp_test(cursor, t_recv)), +// Conditional { cond: t_res === undefined, +// then: t_recv.test(_segment(cursor)), // materialise LAZILY, +// else: t_res } // only on decline +// ]) +// ``` +// +// The materialisation sits inside the `then` arm, so the accepted path -- which +// is every step unless the program patched `RegExp.prototype.test` -- allocates +// nothing. Both temporaries are declared at the top of the loop body, because +// a bare `LocalSet` to an id with no `Stmt::Let` has no slot. + +struct V2Emission { + code_point_at: u32, + regexp_test: u32, + decls: Vec, +} + +fn is_undefined_cmp(id: u32) -> Expr { + Expr::Compare { + op: perry_hir::CompareOp::Eq, + left: Box::new(Expr::LocalGet(id)), + right: Box::new(Expr::Undefined), + } +} + +/// Rewrite the answerable uses of `seg` in one expression. Returns how many of +/// each kind were replaced and any temporaries that must be declared. +fn rewrite_uses_in_expr(e: &mut Expr, seg: u32, cur: u32, fresh: &mut u32, out: &mut V2Emission) { + // `O.codePointAt(k)` -> `cur != 0 ? _code_point_at(cursor, k) : ` + let e_original = e.clone(); + let mut replaced = None; + if let Expr::Call { callee, args, .. } = e { + if let Expr::PropertyGet { + object, property, .. + } = callee.as_ref() + { + if property == "codePointAt" + && args.len() == 1 + && matches!(object.as_ref(), Expr::LocalGet(id) if *id == seg) + { + // Guarded, NOT replaced. The loop body is shared between the + // accepted and declined paths, so the original expression must + // survive for the decline arm, where `O` holds a real string. + replaced = Some(pick( + cur, + extern_call( + "js_segments_view_code_point_at", + vec![Expr::LocalGet(cur), args[0].clone()], + ), + e_original.clone(), + )); + out.code_point_at += 1; + } else if property == "test" + && args.len() == 1 + && matches!(&args[0], Expr::LocalGet(id) if *id == seg) + && !matches!(object.as_ref(), Expr::LocalGet(id) if *id == seg) + { + let t_recv = *fresh; + let t_res = *fresh + 1; + *fresh += 2; + out.decls + .push(let_any(t_recv, "__segview_test_recv", Expr::Undefined)); + out.decls + .push(let_any(t_res, "__segview_test_res", Expr::Undefined)); + let recv_expr = object.as_ref().clone(); + let view_form = Expr::Sequence(vec![ + Expr::LocalSet(t_recv, Box::new(recv_expr)), + Expr::LocalSet( + t_res, + Box::new(extern_call( + "js_segments_view_regexp_test", + vec![Expr::LocalGet(cur), Expr::LocalGet(t_recv)], + )), + ), + Expr::Conditional { + condition: Box::new(is_undefined_cmp(t_res)), + then_expr: Box::new(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(t_recv)), + property: "test".to_string(), + byte_offset: 0, + }), + args: vec![extern_call( + "js_segments_view_segment", + vec![Expr::LocalGet(cur)], + )], + type_args: vec![], + byte_offset: 0, + }), + else_expr: Box::new(Expr::LocalGet(t_res)), + }, + ]); + replaced = Some(pick(cur, view_form, e_original.clone())); + out.regexp_test += 1; + } + } + } + if let Some(new_e) = replaced { + *e = new_e; + return; + } + if let Expr::Closure { body, .. } = e { + for s in body.iter_mut() { + rewrite_uses_in_stmt(s, seg, cur, fresh, out); + } + } + perry_hir::walker::walk_expr_children_mut(e, &mut |child| { + rewrite_uses_in_expr(child, seg, cur, fresh, out) + }); +} + +fn rewrite_uses_in_stmt(s: &mut Stmt, seg: u32, cur: u32, fresh: &mut u32, out: &mut V2Emission) { + for_each_expr_in_stmt_shallow_mut(s, &mut |e| rewrite_uses_in_expr(e, seg, cur, fresh, out)); + let mut kids: Vec<&mut Stmt> = Vec::new(); + collect_child_stmts_mut(s, &mut kids); + for k in kids { + rewrite_uses_in_stmt(k, seg, cur, fresh, out); + } +} + +fn collect_child_stmts_mut<'a>(s: &'a mut Stmt, out: &mut Vec<&'a mut Stmt>) { + match s { + Stmt::If { + then_branch, + else_branch, + .. + } => { + out.extend(then_branch.iter_mut()); + if let Some(e) = else_branch { + out.extend(e.iter_mut()); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => out.extend(body.iter_mut()), + Stmt::For { init, body, .. } => { + if let Some(i) = init { + out.push(i.as_mut()); + } + out.extend(body.iter_mut()); + } + Stmt::Labeled { body, .. } => out.push(body.as_mut()), + Stmt::Try { + body, + catch, + finally, + } => { + out.extend(body.iter_mut()); + if let Some(c) = catch { + out.extend(c.body.iter_mut()); + } + if let Some(f) = finally { + out.extend(f.iter_mut()); + } + } + Stmt::Switch { cases, .. } => { + for c in cases.iter_mut() { + out.extend(c.body.iter_mut()); + } + } + _ => {} + } +} diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index 09cf02a9ef..2f9f2f38f8 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -415,3 +415,90 @@ fn a_declining_site_is_not_rewritten() { assert_eq!(segview_rewrite_module(&mut m), 0); assert_eq!(before, render(&m), "a declining site must be untouched"); } + +/// v2: on a site where every use of the segment is view-answerable, nothing is +/// materialised on the accepted path. `N$6` is exactly this shape — one +/// `codePointAt` and two opaque-receiver `.test()` calls. +#[test] +fn v2_answers_every_use_from_the_view_and_materialises_nothing() { + let mut m = module_with(region(cc_body())); + assert_eq!(segview_rewrite_module(&mut m), 1); + let out = render(&m); + + assert!( + out.contains("js_segments_view_code_point_at"), + "the codePointAt use must be answered from the cursor: {out}" + ); + assert!( + out.contains("js_segments_view_regexp_test"), + "the regex test must be answered from the cursor" + ); + assert!( + out.contains("__segview_test_recv"), + "the opaque receiver must be hoisted so it is evaluated exactly once" + ); + + // The decisive property, checked on the tree rather than on its Debug + // rendering: the segment binding's ACCEPTED arm must be `Undefined`. If it + // were `_segment(cursor)` the loop would still allocate a substring per + // grapheme and v2 would buy nothing — and a string-contains assertion + // would not have caught it, because `_segment` legitimately appears inside + // the regexp_test decline arm. + let seg_bind_accept_is_undefined = m.init.iter().any(|s| match s { + Stmt::For { body, .. } => matches!( + body.first(), + Some(Stmt::Let { + init: Some(Expr::Conditional { then_expr, .. }), + .. + }) if matches!(then_expr.as_ref(), Expr::Undefined) + ), + _ => false, + }); + assert!( + seg_bind_accept_is_undefined, + "the segment must NOT be materialised on the accepted path: {out}" + ); +} + +/// The receiver of `.test(O)` is `g54.default()` in cc — an opaque call that +/// must run exactly once per evaluation. It is bound to a temporary and the +/// fallback arm reuses the temporary rather than re-evaluating it. +#[test] +fn v2_evaluates_an_opaque_test_receiver_exactly_once() { + let body = vec![Stmt::Expr(call( + pget(call(pget(Expr::LocalGet(54), "default"), vec![]), "test"), + vec![Expr::LocalGet(SEG)], + ))]; + let mut m = module_with(region(body)); + assert_eq!(segview_rewrite_module(&mut m), 1); + let out = render(&m); + assert_eq!( + out.matches("property: \"default\"").count(), + 2, + "once in the view arm's hoist and once in the decline arm's original — \ + never twice within one arm: {out}" + ); +} + +/// A site with a use the classifier cannot answer keeps v1: materialise once +/// and leave the body alone. Paying the per-use guards on top of a +/// materialisation that happens anyway would be strictly worse. +#[test] +fn a_site_with_an_unanswerable_use_stays_on_v1() { + let mut body = cc_body(); + body.push(Stmt::Expr(call( + Expr::LocalGet(42), + vec![Expr::LocalGet(SEG)], + ))); + let mut m = module_with(region(body)); + assert_eq!(segview_rewrite_module(&mut m), 1); + let out = render(&m); + assert!( + out.contains("js_segments_view_segment"), + "v1 binds the segment by materialising it once" + ); + assert!( + !out.contains("js_segments_view_code_point_at"), + "v1 does not rewrite uses: {out}" + ); +} From 11d71f74da7c7178db9d3eecfd4cf8a0be0eeb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:13:02 +0200 Subject: [PATCH 16/20] fix(codegen): declare the segment-view runtime entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lowering emitted the calls and the module never declared them, so the in-process LLVM parse rejected the whole module: perry_llvm_….ll:5172:22: error: use of undefined value '@js_segments_view_next' %r75 = call double @js_segments_view_next(double %r74) Not a degraded build — no build at all. The five entry points are now registered in `runtime_decls/strings.rs` beside `js_for_of_next`, which is where every other runtime native gets its `declare`. Signatures are read from `perry-runtime/src/intl/segments_view.rs`, not assumed: `open(f64,f64)`, `next(f64)`, `code_point_at(f64,f64)`, `segment(f64)`, `regexp_test(f64,f64)`. Note `regexp_test` is (cursor, regex), cursor first; it was relayed the other way round once and the source settled it. Why the tier's twelve HIR tests could not catch this: they assert the rewrite emits `Call(ExternFuncRef "js_segments_view_next", …)`, and it did. The gap was between "the lowering emits the call" and "the module can be parsed", and nothing tested the second. `every_segment_view_entry_point_is_declared` closes it by running the real declare phase over an `LlModule` and checking each of the five by name — remove any one registration and it fails naming that symbol. It also asserts ARITY, which is the sabotage a name-only check would miss: a wrong parameter count parses cleanly and then miscompiles the call, because LLVM will coerce or drop an argument rather than complain. (cherry picked from commit ed356f762d0cb069732a1b0809d7e8e4b590c7c6) --- crates/perry-codegen/src/runtime_decls/mod.rs | 3 + .../src/runtime_decls/segview_decls_tests.rs | 73 +++++++++++++++++++ .../src/runtime_decls/strings.rs | 11 +++ 3 files changed, 87 insertions(+) create mode 100644 crates/perry-codegen/src/runtime_decls/segview_decls_tests.rs diff --git a/crates/perry-codegen/src/runtime_decls/mod.rs b/crates/perry-codegen/src/runtime_decls/mod.rs index e08c4f6d34..ec14cd207e 100644 --- a/crates/perry-codegen/src/runtime_decls/mod.rs +++ b/crates/perry-codegen/src/runtime_decls/mod.rs @@ -25,6 +25,9 @@ pub use objects::declare_phase_b_objects; pub use stdlib_ffi::declare_stdlib_ffi; pub(crate) use stdlib_ffi_part2::declare_stdlib_ffi_part2; pub use strings::declare_phase_b_strings; + +#[cfg(test)] +mod segview_decls_tests; pub(crate) use strings_part2::declare_phase_b_strings_part2; /// Declare the minimum set of runtime functions needed by Phase 1 diff --git a/crates/perry-codegen/src/runtime_decls/segview_decls_tests.rs b/crates/perry-codegen/src/runtime_decls/segview_decls_tests.rs new file mode 100644 index 0000000000..c76434d3c5 --- /dev/null +++ b/crates/perry-codegen/src/runtime_decls/segview_decls_tests.rs @@ -0,0 +1,73 @@ +//! #9843: the segment-view tier's runtime entry points must be DECLARED, not +//! only called. +//! +//! The tier's HIR-level tests cannot see this. They assert the rewrite emits +//! `Call(ExternFuncRef "js_segments_view_next", …)`, which it did — and the +//! build still failed, because the module carried the call and no `declare`: +//! +//! ```text +//! perry_llvm_….ll:5172:22: error: use of undefined value '@js_segments_view_next' +//! %r75 = call double @js_segments_view_next(double %r74) +//! ``` +//! +//! The in-process LLVM parse rejects the whole module, so this is not a +//! degraded build, it is no build at all. This test closes the gap between +//! "the lowering emits the call" and "the module can be parsed": remove any one +//! of the five registrations in `strings.rs` and it fails by name. + +use super::declare_phase_b_strings; +use crate::module::LlModule; + +/// Every entry point the segment-view lowering can emit, with the signature +/// taken from `perry-runtime/src/intl/segments_view.rs`. `regexp_test` is +/// `(cursor, regex)` — cursor first; it was relayed the other way round once +/// and the source settled it. +const SEGVIEW_DECLS: &[(&str, usize)] = &[ + ("js_segments_view_open", 2), + ("js_segments_view_next", 1), + ("js_segments_view_code_point_at", 2), + ("js_segments_view_segment", 1), + ("js_segments_view_regexp_test", 2), +]; + +#[test] +fn every_segment_view_entry_point_is_declared() { + let mut m = LlModule::new("arm64-apple-macosx"); + declare_phase_b_strings(&mut m); + let declared: Vec<(&str, &str)> = m.declaration_lines().collect(); + + for (name, arity) in SEGVIEW_DECLS { + let line = declared + .iter() + .find(|(n, _)| n == name) + .unwrap_or_else(|| { + panic!( + "`{name}` is never declared, so any module that calls it fails the LLVM \ + parse with \"use of undefined value\". Register it in \ + `runtime_decls/strings.rs` beside `js_for_of_next`." + ) + }) + .1; + // Arity is checked because a wrong one is accepted by the parser and + // then miscompiles the call: LLVM would coerce or drop an argument. + let params = line + .split_once('(') + .and_then(|(_, rest)| rest.split_once(')')) + .map(|(inner, _)| { + if inner.trim().is_empty() { + 0 + } else { + inner.split(',').count() + } + }) + .unwrap_or_else(|| panic!("malformed declare line for `{name}`: {line}")); + assert_eq!( + params, *arity, + "`{name}` is declared with {params} parameters, runtime defines {arity}: {line}" + ); + assert!( + line.contains("double"), + "`{name}` must use the NaN-boxed f64 ABI like every other js_* entry: {line}" + ); + } +} diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 204cdbe4f0..765fe9c8e7 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1568,6 +1568,17 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // Iterator-protocol result validation (for-of lazy loop). module.declare_function("js_iterator_result_validate", DOUBLE, &[DOUBLE]); module.declare_function("js_for_of_next", DOUBLE, &[DOUBLE]); + // #9843: Intl.Segmenter view mode. The segment-view tier emits calls to + // these when it fires; without a `declare` the module references an + // undefined value and the in-process LLVM parse rejects the whole module + // ("use of undefined value '@js_segments_view_next'"). Signatures are + // taken from `perry-runtime/src/intl/segments_view.rs` (#9870) — note that + // `regexp_test` is (cursor, regex), cursor first. + module.declare_function("js_segments_view_open", DOUBLE, &[DOUBLE, DOUBLE]); + module.declare_function("js_segments_view_next", DOUBLE, &[DOUBLE]); + module.declare_function("js_segments_view_code_point_at", DOUBLE, &[DOUBLE, DOUBLE]); + module.declare_function("js_segments_view_segment", DOUBLE, &[DOUBLE]); + module.declare_function("js_segments_view_regexp_test", DOUBLE, &[DOUBLE, DOUBLE]); module.declare_function("js_global_get_or_throw_unresolved", DOUBLE, &[DOUBLE]); // Ambient `require` for compiled external / compilePackages modules (#5373): // bind a bare `require` to a createRequire-backed closure instead of throwing From 7a14f7eadb35997cf82287a099ae9a0936d96347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 15:13:02 +0200 Subject: [PATCH 17/20] fix(codegen): answer a statically-known RegExpTest from the view too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `v2_answers_every_use_from_the_view_and_materialises_nothing` failed because the v2 rewriter handled only the generic `Call(PropertyGet(recv,"test"), [O])` shape. perry folds a test whose regex is statically known into `Expr::RegExpTest { regex, string }`, which the classifier counts as `regexp_test_static` — so the classification said "answerable" and the emission did not answer it. The pass's own agreement check caught that: emission counts did not match classification counts, so it discarded the rewrite and fell back to v1 rather than emitting a loop that reads an unbound segment on the accepted path. The guard did its job; this teaches the rewriter the shape so the guard stops having to. Unlike the generic form, the static node's regex is a literal or a binding with no side effect worth hoisting, so it can be repeated in the decline arm and needs one temporary rather than two. Also corrects an assertion in that test that could not hold: it required `__segview_test_recv` on a body whose only test is the static node, which has no opaque receiver to hoist. That property belongs to the generic form and is already pinned by `v2_evaluates_an_opaque_test_receiver_exactly_once`. Replaced with the tri-state temporary, which this body does have, and commented so it is not re-added. 16/16 segview tests pass. (cherry picked from commit 6b522fbfccaf73663bba5f1e8b7af114db1c27d1) --- .../perry-codegen/src/collectors/segview.rs | 43 +++++++++++++++++++ .../src/collectors/segview_tests.rs | 11 ++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index 172a7d1cd5..90da00a26f 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -1570,6 +1570,49 @@ fn rewrite_uses_in_expr(e: &mut Expr, seg: u32, cur: u32, fresh: &mut u32, out: } } } + // `Expr::RegExpTest { regex, string: O }` — the node perry folds a test to + // when the regex is statically known. The classifier counts it as + // `regexp_test_static`, so the rewriter has to answer it too, or the + // emission/classification agreement check refuses v2 and the site falls + // back to v1. That is exactly what happened on the first version of this + // pass: the check caught it, which is why it fell back instead of emitting + // a loop that read an unbound segment. + if replaced.is_none() { + if let Expr::RegExpTest { regex, string } = e { + if matches!(string.as_ref(), Expr::LocalGet(id) if *id == seg) { + let t_res = *fresh; + *fresh += 1; + out.decls + .push(let_any(t_res, "__segview_test_res", Expr::Undefined)); + // The regex here is an ordinary expression with no side effect + // worth hoisting (a literal or a binding), so unlike the + // generic `recv.test(O)` form it can be repeated in the + // decline arm. + let view_form = Expr::Sequence(vec![ + Expr::LocalSet( + t_res, + Box::new(extern_call( + "js_segments_view_regexp_test", + vec![Expr::LocalGet(cur), regex.as_ref().clone()], + )), + ), + Expr::Conditional { + condition: Box::new(is_undefined_cmp(t_res)), + then_expr: Box::new(Expr::RegExpTest { + regex: regex.clone(), + string: Box::new(extern_call( + "js_segments_view_segment", + vec![Expr::LocalGet(cur)], + )), + }), + else_expr: Box::new(Expr::LocalGet(t_res)), + }, + ]); + replaced = Some(pick(cur, view_form, e_original.clone())); + out.regexp_test += 1; + } + } + } if let Some(new_e) = replaced { *e = new_e; return; diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index 2f9f2f38f8..51b7ae71d5 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -433,9 +433,16 @@ fn v2_answers_every_use_from_the_view_and_materialises_nothing() { out.contains("js_segments_view_regexp_test"), "the regex test must be answered from the cursor" ); + // NOT `__segview_test_recv` here: `cc_body()` uses `Expr::RegExpTest`, the + // folded node for a statically-known regex, which has no opaque receiver to + // hoist. The receiver-hoisting property belongs to the generic + // `recv.test(O)` form and is pinned by + // `v2_evaluates_an_opaque_test_receiver_exactly_once`. Asserting it here + // was asserting a property this body does not have. assert!( - out.contains("__segview_test_recv"), - "the opaque receiver must be hoisted so it is evaluated exactly once" + out.contains("__segview_test_res"), + "the tri-state result must be held in a temporary so the decline arm \ + can be selected without calling the runtime twice: {out}" ); // The decisive property, checked on the tree rather than on its Debug From 38985657c30658d5c44afc8b2f8ca1f4044d6d5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:17:38 +0200 Subject: [PATCH 18/20] style: cargo fmt --- crates/perry/tests/duplicate_class_accessor_last_wins.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/perry/tests/duplicate_class_accessor_last_wins.rs b/crates/perry/tests/duplicate_class_accessor_last_wins.rs index e3e28a0234..fb17df37d7 100644 --- a/crates/perry/tests/duplicate_class_accessor_last_wins.rs +++ b/crates/perry/tests/duplicate_class_accessor_last_wins.rs @@ -94,7 +94,9 @@ console.log("expr", new Expr().w); String::from_utf8_lossy(&compile.stderr) ); - let run = Command::new(&output).output().expect("run compiled fixture"); + let run = Command::new(&output) + .output() + .expect("run compiled fixture"); assert!( run.status.success(), "compiled fixture failed\nstdout:\n{}\nstderr:\n{}", From 98584ee5a6484bbae4165d97b1710dd57c423055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:06:44 +0200 Subject: [PATCH 19/20] fix(train): split class_decl's member helpers out for the file cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #9886's last-wins accessor record pushed lower_decl/class_decl.rs to 2050 lines. The member-shape helpers — computed-key naming, the accessor-name survey, and record_class_accessor itself — move to a sibling child module beside the existing class_heritage/member_registration. Unlike the page_meta split, this adds a child rather than renaming the parent, so nothing keyed on the path `lower_decl/class_decl.rs` moves; the two prose references to it elsewhere in the tree stay correct. --- crates/perry-hir/src/lower_decl/class_decl.rs | 198 +---------------- .../lower_decl/class_decl/member_helpers.rs | 202 ++++++++++++++++++ 2 files changed, 208 insertions(+), 192 deletions(-) create mode 100644 crates/perry-hir/src/lower_decl/class_decl/member_helpers.rs diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 5fc0375a0c..53c309a261 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -39,204 +39,18 @@ fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool { } mod class_heritage; +mod member_helpers; mod member_registration; use class_heritage::*; +pub(crate) use member_helpers::capture_class_source; +use member_helpers::{ + computed_member_name, generic_computed_member_key, lower_generic_computed_class_member, + noncomputed_member_registration_name, record_class_accessor, runtime_instance_accessor_names, +}; use member_registration::*; use super::*; -fn generic_computed_member_key<'a>( - _ctx: &LoweringContext, - method: &'a ast::ClassMethod, -) -> Option<&'a ast::ComputedPropName> { - let ast::PropName::Computed(computed) = &method.key else { - return None; - }; - // Single source of truth — see `is_special_lowered_well_known`. #9226 - // hand-copied a subset here and silently dropped four symbols. - if crate::lower_decl::helpers::is_special_lowered_well_known(method) { - return None; - } - Some(computed) -} - -fn computed_member_name(kind: ast::MethodKind, computed: &ast::ComputedPropName) -> String { - let base = match kind { - ast::MethodKind::Method => "__computed_method", - ast::MethodKind::Getter => "__computed_getter", - ast::MethodKind::Setter => "__computed_setter", - }; - format!("{}_{}_{}", base, computed.span.lo.0, computed.span.hi.0) -} - -fn runtime_instance_accessor_names(members: &[ast::ClassMember]) -> crate::ClassAccessorNames { - let mut accessor_names = crate::ClassAccessorNames::default(); - - for member in members { - match member { - ast::ClassMember::Method(m) - if !m.is_static - && m.function.body.is_some() - && matches!(m.kind, ast::MethodKind::Getter | ast::MethodKind::Setter) => - { - let key = match &m.key { - ast::PropName::Ident(i) => i.sym.to_string(), - ast::PropName::Str(s) => s.value.as_str().unwrap_or("").to_string(), - ast::PropName::Num(n) => crate::lower::number_to_js_key(n.value), - // #5592: a computed accessor key (`get [expr]()` / - // `set [expr](v)`) isn't statically known. Mark the class so - // `obj.prototype. = v` writes route through the generic - // setter-invoking path rather than a name-keyed prototype - // monkey-patch. - ast::PropName::Computed(_) => { - accessor_names.has_computed = true; - continue; - } - _ => continue, - }; - match m.kind { - ast::MethodKind::Getter => { - accessor_names.insert_getter(key); - } - ast::MethodKind::Setter => { - accessor_names.insert_setter(key); - } - _ => {} - } - } - ast::ClassMember::PrivateMethod(m) - if !m.is_static - && m.function.body.is_some() - && matches!(m.kind, ast::MethodKind::Getter | ast::MethodKind::Setter) => - { - let key = format!("#{}", m.key.name); - match m.kind { - ast::MethodKind::Getter => { - accessor_names.insert_getter(key); - } - ast::MethodKind::Setter => { - accessor_names.insert_setter(key); - } - _ => {} - } - } - _ => {} - } - } - - accessor_names -} - -fn lower_generic_computed_class_member( - ctx: &mut LoweringContext, - method: &ast::ClassMethod, - computed: &ast::ComputedPropName, - source_order: usize, -) -> Result { - let key_expr = lower_expr(ctx, &computed.expr)?; - let function_name = computed_member_name(method.kind, computed); - let (kind, function) = match method.kind { - ast::MethodKind::Method => ( - ClassComputedMemberKind::Method, - with_static_member_context(ctx, method.is_static, |ctx| { - lower_class_method_with_name(ctx, method, function_name) - })?, - ), - ast::MethodKind::Getter => ( - ClassComputedMemberKind::Getter, - with_static_member_context(ctx, method.is_static, |ctx| { - lower_getter_method_with_name(ctx, method, function_name) - })?, - ), - ast::MethodKind::Setter => ( - ClassComputedMemberKind::Setter, - with_static_member_context(ctx, method.is_static, |ctx| { - lower_setter_method_with_name(ctx, method, function_name) - })?, - ), - }; - Ok(ClassComputedMember { - key_expr, - function, - is_static: method.is_static, - kind, - source_order, - }) -} - -fn noncomputed_member_registration_name( - kind: ast::MethodKind, - method: &ast::ClassMethod, -) -> String { - let base = match kind { - ast::MethodKind::Method => "__computed_method_named", - ast::MethodKind::Getter => "__computed_getter_named", - ast::MethodKind::Setter => "__computed_setter_named", - }; - format!("{}_{}_{}", base, method.span.lo.0, method.span.hi.0) -} - -/// #9413: retain a class's original source text keyed by ClassId so -/// `Function.prototype.toString` can reconstruct it, mirroring -/// `capture_function_source` (#4101) for functions. SWC anchors -/// `ast::Class::span` at the `class` keyword (decorators sit outside it) and -/// closes it at the class body's `}`, so the slice is exactly the class source -/// node's `[[SourceText]]`. A no-op when no module source is installed (unit -/// tests / `check`), and idempotent — last write wins, matching the name -/// registry. -pub(crate) fn capture_class_source( - ctx: &mut LoweringContext, - class_id: crate::ClassId, - class: &ast::Class, -) { - if let Some(src) = crate::ir::current_module_source_slice(class.span.lo.0, class.span.hi.0) { - ctx.class_source_text.insert(class_id, src); - } -} - -/// Record one class accessor, honouring ECMA-262's "a later definition of the -/// same key replaces the earlier one". -/// -/// `ClassDecl::getters` / `::setters` are consumed with `iter().find(...)`, so -/// the FIRST entry with a given name wins at lookup time. Appending -/// unconditionally therefore keeps a *shadowed* accessor alive and silently -/// drops the one the program actually defines last: -/// -/// ```js -/// class Spring3 { -/// get z() { return this.a.z; } // damping — shadowed -/// get z() { return this.c.x; } // displacement — must win -/// } -/// ``` -/// -/// Perry returned `this.a.z` here while every other engine returns -/// `this.c.x`. In Claude-of-Duty that handed the viewmodel rig a spring's -/// DAMPING COEFFICIENT (0.46) where it wanted a Z displacement, pushing the -/// weapon 0.88 m behind the camera, where it clipped and drew nothing. -/// -/// Static and instance accessors are distinct properties (one lives on the -/// constructor, one on the prototype) and may legally share a name, so the -/// replacement is keyed on `(name, is_static)` rather than the name alone. -fn record_class_accessor( - list: &mut Vec<(String, Function)>, - statics: &mut Vec, - name: String, - func: Function, - is_static: bool, -) { - let existing = list - .iter() - .enumerate() - .find_map(|(i, (n, _))| (n == &name && statics[i] == is_static).then_some(i)); - match existing { - Some(i) => list[i] = (name, func), - None => { - list.push((name, func)); - statics.push(is_static); - } - } -} - pub fn lower_class_decl( ctx: &mut LoweringContext, class_decl: &ast::ClassDecl, diff --git a/crates/perry-hir/src/lower_decl/class_decl/member_helpers.rs b/crates/perry-hir/src/lower_decl/class_decl/member_helpers.rs new file mode 100644 index 0000000000..bb0f583786 --- /dev/null +++ b/crates/perry-hir/src/lower_decl/class_decl/member_helpers.rs @@ -0,0 +1,202 @@ +//! Member-shape helpers for `class_decl`: computed-key naming, the +//! accessor-name survey, and the ECMA-262 last-wins accessor record. +//! Split out of `class_decl.rs` for the 2000-line file cap. + +use super::*; + +pub(super) fn generic_computed_member_key<'a>( + _ctx: &LoweringContext, + method: &'a ast::ClassMethod, +) -> Option<&'a ast::ComputedPropName> { + let ast::PropName::Computed(computed) = &method.key else { + return None; + }; + // Single source of truth — see `is_special_lowered_well_known`. #9226 + // hand-copied a subset here and silently dropped four symbols. + if crate::lower_decl::helpers::is_special_lowered_well_known(method) { + return None; + } + Some(computed) +} + +pub(super) fn computed_member_name( + kind: ast::MethodKind, + computed: &ast::ComputedPropName, +) -> String { + let base = match kind { + ast::MethodKind::Method => "__computed_method", + ast::MethodKind::Getter => "__computed_getter", + ast::MethodKind::Setter => "__computed_setter", + }; + format!("{}_{}_{}", base, computed.span.lo.0, computed.span.hi.0) +} + +pub(super) fn runtime_instance_accessor_names( + members: &[ast::ClassMember], +) -> crate::ClassAccessorNames { + let mut accessor_names = crate::ClassAccessorNames::default(); + + for member in members { + match member { + ast::ClassMember::Method(m) + if !m.is_static + && m.function.body.is_some() + && matches!(m.kind, ast::MethodKind::Getter | ast::MethodKind::Setter) => + { + let key = match &m.key { + ast::PropName::Ident(i) => i.sym.to_string(), + ast::PropName::Str(s) => s.value.as_str().unwrap_or("").to_string(), + ast::PropName::Num(n) => crate::lower::number_to_js_key(n.value), + // #5592: a computed accessor key (`get [expr]()` / + // `set [expr](v)`) isn't statically known. Mark the class so + // `obj.prototype. = v` writes route through the generic + // setter-invoking path rather than a name-keyed prototype + // monkey-patch. + ast::PropName::Computed(_) => { + accessor_names.has_computed = true; + continue; + } + _ => continue, + }; + match m.kind { + ast::MethodKind::Getter => { + accessor_names.insert_getter(key); + } + ast::MethodKind::Setter => { + accessor_names.insert_setter(key); + } + _ => {} + } + } + ast::ClassMember::PrivateMethod(m) + if !m.is_static + && m.function.body.is_some() + && matches!(m.kind, ast::MethodKind::Getter | ast::MethodKind::Setter) => + { + let key = format!("#{}", m.key.name); + match m.kind { + ast::MethodKind::Getter => { + accessor_names.insert_getter(key); + } + ast::MethodKind::Setter => { + accessor_names.insert_setter(key); + } + _ => {} + } + } + _ => {} + } + } + + accessor_names +} + +pub(super) fn lower_generic_computed_class_member( + ctx: &mut LoweringContext, + method: &ast::ClassMethod, + computed: &ast::ComputedPropName, + source_order: usize, +) -> Result { + let key_expr = lower_expr(ctx, &computed.expr)?; + let function_name = computed_member_name(method.kind, computed); + let (kind, function) = match method.kind { + ast::MethodKind::Method => ( + ClassComputedMemberKind::Method, + with_static_member_context(ctx, method.is_static, |ctx| { + lower_class_method_with_name(ctx, method, function_name) + })?, + ), + ast::MethodKind::Getter => ( + ClassComputedMemberKind::Getter, + with_static_member_context(ctx, method.is_static, |ctx| { + lower_getter_method_with_name(ctx, method, function_name) + })?, + ), + ast::MethodKind::Setter => ( + ClassComputedMemberKind::Setter, + with_static_member_context(ctx, method.is_static, |ctx| { + lower_setter_method_with_name(ctx, method, function_name) + })?, + ), + }; + Ok(ClassComputedMember { + key_expr, + function, + is_static: method.is_static, + kind, + source_order, + }) +} + +pub(super) fn noncomputed_member_registration_name( + kind: ast::MethodKind, + method: &ast::ClassMethod, +) -> String { + let base = match kind { + ast::MethodKind::Method => "__computed_method_named", + ast::MethodKind::Getter => "__computed_getter_named", + ast::MethodKind::Setter => "__computed_setter_named", + }; + format!("{}_{}_{}", base, method.span.lo.0, method.span.hi.0) +} + +/// #9413: retain a class's original source text keyed by ClassId so +/// `Function.prototype.toString` can reconstruct it, mirroring +/// `capture_function_source` (#4101) for functions. SWC anchors +/// `ast::Class::span` at the `class` keyword (decorators sit outside it) and +/// closes it at the class body's `}`, so the slice is exactly the class source +/// node's `[[SourceText]]`. A no-op when no module source is installed (unit +/// tests / `check`), and idempotent — last write wins, matching the name +/// registry. +pub(crate) fn capture_class_source( + ctx: &mut LoweringContext, + class_id: crate::ClassId, + class: &ast::Class, +) { + if let Some(src) = crate::ir::current_module_source_slice(class.span.lo.0, class.span.hi.0) { + ctx.class_source_text.insert(class_id, src); + } +} + +/// Record one class accessor, honouring ECMA-262's "a later definition of the +/// same key replaces the earlier one". +/// +/// `ClassDecl::getters` / `::setters` are consumed with `iter().find(...)`, so +/// the FIRST entry with a given name wins at lookup time. Appending +/// unconditionally therefore keeps a *shadowed* accessor alive and silently +/// drops the one the program actually defines last: +/// +/// ```js +/// class Spring3 { +/// get z() { return this.a.z; } // damping — shadowed +/// get z() { return this.c.x; } // displacement — must win +/// } +/// ``` +/// +/// Perry returned `this.a.z` here while every other engine returns +/// `this.c.x`. In Claude-of-Duty that handed the viewmodel rig a spring's +/// DAMPING COEFFICIENT (0.46) where it wanted a Z displacement, pushing the +/// weapon 0.88 m behind the camera, where it clipped and drew nothing. +/// +/// Static and instance accessors are distinct properties (one lives on the +/// constructor, one on the prototype) and may legally share a name, so the +/// replacement is keyed on `(name, is_static)` rather than the name alone. +pub(super) fn record_class_accessor( + list: &mut Vec<(String, Function)>, + statics: &mut Vec, + name: String, + func: Function, + is_static: bool, +) { + let existing = list + .iter() + .enumerate() + .find_map(|(i, (n, _))| (n == &name && statics[i] == is_static).then_some(i)); + match existing { + Some(i) => list[i] = (name, func), + None => { + list.push((name, func)); + statics.push(is_static); + } + } +} From ac014d135956523134fdb1ed17771429c542c266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:48:54 +0200 Subject: [PATCH 20/20] fix(train): drop the unused computed_member_name re-import It is used only inside member_helpers itself, so importing it back into class_decl fails `-D warnings`. Visible to the gate but not to a plain `cargo check -p perry-hir`. --- crates/perry-hir/src/lower_decl/class_decl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 53c309a261..3174c687cf 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -44,7 +44,7 @@ mod member_registration; use class_heritage::*; pub(crate) use member_helpers::capture_class_source; use member_helpers::{ - computed_member_name, generic_computed_member_key, lower_generic_computed_class_member, + generic_computed_member_key, lower_generic_computed_class_member, noncomputed_member_registration_name, record_class_accessor, runtime_instance_accessor_names, }; use member_registration::*;