diff --git a/CLAUDE.md b/CLAUDE.md index cb5b4e1ded..fcd497bc61 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1383 +**Current Version:** 0.5.1384 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index f43cd35176..96f48f060f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1383" +version = "0.5.1384" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1383" +version = "0.5.1384" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1383" +version = "0.5.1384" [[package]] name = "perry-ui-tvos" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1383" +version = "0.5.1384" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 0d2620e6ee..b328dbe88a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1383" +version = "0.5.1384" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7670-layer1-rooting-slice8-terminal-condition.md b/changelog.d/7670-layer1-rooting-slice8-terminal-condition.md new file mode 100644 index 0000000000..074fc1b3c2 --- /dev/null +++ b/changelog.d/7670-layer1-rooting-slice8-terminal-condition.md @@ -0,0 +1,132 @@ +### GC — Layer 1 rooting, slice 8: the raw API becomes unreachable (#7615) + +The Layer 1 emitter-migration campaign reaches its **terminal condition**. The +plan stated it as "`expr/temp_root.rs` going `pub(in crate::rooting)` — the raw +accessor unreachable, not merely uncounted". + +**As literally spelled, that is not expressible in Rust.** `pub(in path)` +requires `path` to be an ANCESTOR module of the item (E0742), and +`crate::rooting` is not an ancestor of `crate::expr::temp_root`. So the file +moved: the raw API is now `crate::rooting::temp_root`, declared with a +**private** `mod temp_root;` and with `pub(in crate::rooting)` on every +accessor. Either alone would suffice; both are here because the module +declaration is one keyword away from re-widening twenty-five items at once. + +A raw call planted in a migrated module now fails to compile — `error[E0603]: +module temp_root is private` — which is the difference between this and a +ledger line. Both belts are sabotage-verified (ten arms: eight textual, two +visibility). + +Two items keep `pub(crate)` and are re-exported from `rooting/mod.rs`. Neither +is an accessor and neither can be called in the wrong order: `TempRootPool` +(the compile-time slot bookkeeping `FnCtx` owns) and `expr_is_inert_primitive` +(the "can evaluating this run user code?" predicate `crate::loop_purity` +shares). + +**Fourteen entry points were deleted, not narrowed**, because the migration +left them with no caller: `lower_exprs_rooted`, `lower_operand_pair_rooted`, +`any_later_ref_may_trigger_gc`, `RootedOperands::is_rooted`, the whole +`StoreOperandGuard` family (`guard_store_operand`, `guard_store_operand_across`, +`reread_store_operand`, `release_store_operand`), the whole `RootedHandle` +family (`rooted_handle_begin`/`_get`/`_release`) and +`temp_root_scope_begin`/`_end`. CLAUDE.md's kill-policy: the losing mode should +stop compiling. + +#### One live bug: `Expr::ArrayMap` + +`arr.map(cb)` lowered the receiver, lowered the callback, and only THEN unboxed +the receiver — so `unbox_to_i64` sat **below its own window** and masked a stale +box rather than repairing it (#7280 taxonomy (c), an operand-to-operand +window). The callback's lowering allocates a closure at minimum. + +The window is real for receivers with **no slot to rematerialise from**, +verified on emitted IR against a `main` baseline built in a separate target +dir. For a module global the pre-fix IR is: + +```llvm +%r1 = load double, ptr @perry_global_m_ts__0 ; receiver +%r2 = call i64 @js_closure_alloc_singleton(...) ; the window +%r5 = bitcast double %r1 to i64 ; STALE +%r8 = call i64 @js_array_map(i64 %r6, ...) +``` + +and the same shape appears for a class-field read and for a closure capture +(`js_closure_get_capture_bits`, a raw `i64` — taxonomy (a), which `root_reload` +structurally cannot repair). For an array-typed **local** there was no window: +codegen's `ptr addrspace(1)` retype pass rematerialises the load from the +local's own root slot at the use site, so the pre-fix code re-read the receiver +by accident. That distinction is recorded in the tests, because the first +version of them used a local receiver and the sabotage arm came back green. + +#### Modules migrated + +Eight, seven load-bearing on the committed source: `expr/binary.rs`, +`expr/math_simple.rs`, `expr/static_field_meta.rs`, `expr/dyn_extern_i18n.rs`, +`lower_string_method.rs`, `lower_string_concat.rs`, `lower_call/new.rs`, and +`lower_call/new_alloc.rs` (vacuous, listed anyway so an unlisted sibling of a +listed module cannot become the place a raw push goes). + +Nine further files mention the raw API and make **no rooting decision**, so +they are deliberately not listed — a ledger line on a module that never had a +decision to make looks substantive and asserts nothing: `expr/mod.rs` (a module +declaration and a field type, both gone with the move), the four `FnCtx` +constructors (`TempRootPool::default()`), `stmt/loops.rs` (one purity +predicate), `loop_purity.rs` (a doc link only) and `root_reload.rs` / +`gc_call_effects.rs` / `runtime_decls/arrays.rs` plus five test files, whose +`js_gc_temp_root_*` occurrences are runtime SYMBOL NAMES. + +#### Slice 7's three unverified leads + +* **`static_field_meta.rs`'s `caps_arr` — a real accumulator shape with a + provably empty window.** The `__perry_ctor_caps` array held the only + reference to everything pushed so far while the next element was lowered + (#6951's shape) in a bare SSA register. But `captured_args` is built at one + site (`lower/lower_expr/arm_class.rs`) as + `ids.iter().map(|id| Expr::LocalGet(*id))`, and `expr_may_trigger_gc` answers + `false` for every `LocalGet`. It is now a `with_rooted_accumulator` whose + `protect` is **computed rather than assumed**: today that is `false` and the + emitted IR is byte for byte unchanged; the day a non-inert expression reaches + the list it is rooted by construction. +* **`math_simple.rs`'s `ArrayMap` — confirmed live**, above. +* **`dyn_extern_i18n.rs`'s `path_handle` — dismissed, and the lead's premise is + wrong about the CFG.** It says the raw handle is reused across a compare loop + "that runs module `__init` bodies". It does not: each `__init()` is + emitted into that iteration's MATCH block, which branches straight to the + join, so no `__init` dominates any later use of `path_handle`. Along the + fallthrough chain the only emissions between the handle's production and its + last use are `js_get_string_pointer_unified` and `js_string_equals` — neither + re-enters user code nor enumerates an object, which is the standard + `with_operands_rooted_across_call`'s doc sets for an emitted step (#7198). + What that module DID have was the namespace-object accumulator (#7280's + 269-member zod case), which is migrated. + +#### Two file splits, each its own commit + +Both were blockers rather than tidying: the migration ADDS lines (a combinator +owns the body it re-indents) and both files sat against +`scripts/check_file_size.sh`'s 2,000-line cap. + +* `lower_call/new.rs` 1,988 → 1,501; `lower_call/new_alloc.rs` 531 (the + field-count computation and the three-arm instance allocation, verbatim). +* `lower_string_method.rs` 1,957 → 1,368; `lower_string_concat.rs` 646 (the + boundary the file already had: method dispatch above, `a + b` / `s += x` + below). + +#### API + +`RootedGroup::adopt_emitted` gains a `protect` flag — the **window**, not the +strategy; `Reload` and `Reuse` remain unavailable for an emitted value on +principle. `RootedGroup::is_rooted` reports whether a slot exists (never which +one), which is what lets `MapSet` keep its eager unbox, and therefore its exact +register numbering, on the unprotected path. + +#### ★ A trap worth recording: a sabotage harness that restores a `.bak` + +`cp f f.bak; ; cargo test; mv f.bak f` leaves `f` with an **older** +mtime than the sabotaged build, so cargo keeps the sabotaged binary and every +subsequent run measures the sabotage. It presented as an intermittent +lowering-test failure (1 run in 10, then 20 in 20) with byte-identical IR +between the green and red runs — which reads exactly like the process-global +sinks #7665 fixed, and is not. `touch` after the restore; and diagnose from the +wrong VALUE, which here said "the producer is one line above the window", i.e. +precisely the pre-fix lowering. diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index a8c9d721eb..8f1224f771 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -974,7 +974,7 @@ pub(super) fn compile_closure( // Conservative: treat every slot as possibly-bound (param binds are // emitted before FnCtx exists here), so clears never get skipped. shadow_slots_bound: shadow_slot_map.values().copied().collect(), - temp_roots: crate::expr::temp_root::TempRootPool::default(), + temp_roots: crate::rooting::TempRootPool::default(), shadow_slot_map, persistent_shadow_slots: std::collections::HashSet::new(), shadow_slot_clears_after_stmt, diff --git a/crates/perry-codegen/src/codegen/entry.rs b/crates/perry-codegen/src/codegen/entry.rs index 54a1352105..72c16609af 100644 --- a/crates/perry-codegen/src/codegen/entry.rs +++ b/crates/perry-codegen/src/codegen/entry.rs @@ -781,7 +781,7 @@ pub(super) fn compile_module_entry( not_bigint_locals: main_native_facts.not_bigint_locals(), unsigned_i32_locals: main_native_facts.unsigned_i32_locals(), shadow_slots_bound: main_shadow_slot_map.values().copied().collect(), - temp_roots: crate::expr::temp_root::TempRootPool::default(), + temp_roots: crate::rooting::TempRootPool::default(), shadow_slot_map: main_shadow_slot_map, persistent_shadow_slots: std::collections::HashSet::new(), shadow_slot_clears_after_stmt: main_shadow_slot_clears_after_stmt, @@ -1448,7 +1448,7 @@ pub(super) fn compile_module_entry( not_bigint_locals: init_native_facts.not_bigint_locals(), unsigned_i32_locals: init_native_facts.unsigned_i32_locals(), shadow_slots_bound: init_shadow_slot_map.values().copied().collect(), - temp_roots: crate::expr::temp_root::TempRootPool::default(), + temp_roots: crate::rooting::TempRootPool::default(), shadow_slot_map: init_shadow_slot_map, persistent_shadow_slots: std::collections::HashSet::new(), shadow_slot_clears_after_stmt: init_shadow_slot_clears_after_stmt, diff --git a/crates/perry-codegen/src/codegen/function.rs b/crates/perry-codegen/src/codegen/function.rs index 946fb7286a..f7dabf7028 100644 --- a/crates/perry-codegen/src/codegen/function.rs +++ b/crates/perry-codegen/src/codegen/function.rs @@ -771,7 +771,7 @@ pub(super) fn compile_function( persistent_shadow_slots: std::collections::HashSet::new(), shadow_slot_clears_after_stmt, shadow_slots_bound: bound_param_slots, - temp_roots: crate::expr::temp_root::TempRootPool::default(), + temp_roots: crate::rooting::TempRootPool::default(), arena_state_slot: None, class_keys_slots: HashMap::new(), cached_lengths: HashMap::new(), diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index e109ab52d0..88fdffd5e9 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -505,7 +505,7 @@ pub(super) fn compile_method( // Conservative: treat every slot as possibly-bound (param binds are // emitted before FnCtx exists here), so clears never get skipped. shadow_slots_bound: shadow_slot_map.values().copied().collect(), - temp_roots: crate::expr::temp_root::TempRootPool::default(), + temp_roots: crate::rooting::TempRootPool::default(), shadow_slot_map, persistent_shadow_slots: std::collections::HashSet::new(), shadow_slot_clears_after_stmt, @@ -1566,7 +1566,7 @@ pub(super) fn compile_static_method( // Conservative: treat every slot as possibly-bound (param binds are // emitted before FnCtx exists here), so clears never get skipped. shadow_slots_bound: shadow_slot_map.values().copied().collect(), - temp_roots: crate::expr::temp_root::TempRootPool::default(), + temp_roots: crate::rooting::TempRootPool::default(), shadow_slot_map, persistent_shadow_slots: std::collections::HashSet::new(), shadow_slot_clears_after_stmt, diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index 5ea824f5dc..151abe2e02 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -7,7 +7,7 @@ use anyhow::Result; use perry_hir::{BinaryOp, Expr, LogicalOp}; -use crate::lower_string_method::{ +use crate::lower_string_concat::{ flatten_string_add_chain, lower_string_coerce_concat, lower_string_concat, lower_string_concat_chain, }; @@ -22,9 +22,35 @@ use crate::type_analysis::{ }; use crate::types::{DOUBLE, I1, I128, I32, I64}; -use super::temp_root::{lower_operand_pair_rooted, temp_root_release}; +use crate::rooting::with_operands_rooted; + use super::{is_known_finite, lower_expr, FnCtx}; +/// `helper(left, right)` with each operand rooted across the other's lowering +/// and the group released on every path out (#6951). +/// +/// All five dynamic-dispatch arms below are this one shape — the operand pair +/// feeds a runtime helper that runs `ToPrimitive` / `ToNumeric` on both sides, +/// so a pointer-bearing left operand has to survive the right operand's +/// evaluation. Before #7615 slice 8 each spelled it out as +/// `lower_operand_pair_rooted` + a `temp_root_release` on its own `return` +/// path; that is five chances to place the release wrong, and #7462 is what +/// one misplaced release costs. +fn lower_rooted_dynamic_binary( + ctx: &mut FnCtx<'_>, + helper: &str, + left: &Expr, + right: &Expr, +) -> Result { + with_operands_rooted(ctx, &[left, right], |ctx, values| { + Ok(ctx.block().call( + DOUBLE, + helper, + &[(DOUBLE, &values[0]), (DOUBLE, &values[1])], + )) + }) +} + fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> { // #6884: a statically typed numeric TypedArray read is Number|undefined, // not an unconditional raw f64. In arithmetic context the OOB `undefined` @@ -435,14 +461,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if other_known_primitive { return lower_string_coerce_concat(ctx, left, right, l_is_str, r_is_str); } - let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?; - let sum = ctx.block().call( - DOUBLE, + return lower_rooted_dynamic_binary( + ctx, "js_dynamic_string_or_number_add", - &[(DOUBLE, &l), (DOUBLE, &r)], + left, + right, ); - temp_root_release(ctx, guard); - return Ok(sum); } if is_bigint_expr(ctx, left) && is_bigint_expr(ctx, right) { if let Some(value) = try_lower_small_bigint_literal_binary( @@ -453,12 +477,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ) { return Ok(value); } - let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?; - let sum = - ctx.block() - .call(DOUBLE, "js_dynamic_add", &[(DOUBLE, &l), (DOUBLE, &r)]); - temp_root_release(ctx, guard); - return Ok(sum); + return lower_rooted_dynamic_binary(ctx, "js_dynamic_add", left, right); } // Refs #486: neither operand is statically known. Per JS // spec for `+`, if EITHER side is a string at runtime, the @@ -478,14 +497,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { && crate::type_analysis::is_numeric_expr(ctx, right)) || add_operands_have_pod_materialization_hazard(ctx, left, right) { - let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?; - let sum = ctx.block().call( - DOUBLE, + return lower_rooted_dynamic_binary( + ctx, "js_dynamic_string_or_number_add", - &[(DOUBLE, &l), (DOUBLE, &r)], + left, + right, ); - temp_root_release(ctx, guard); - return Ok(sum); } } // BigInt arithmetic fast path. NaN-tagged bigints compare @@ -508,12 +525,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { { return Ok(value); } - let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?; - let value = ctx - .block() - .call(DOUBLE, fname, &[(DOUBLE, &l), (DOUBLE, &r)]); - temp_root_release(ctx, guard); - return Ok(value); + return lower_rooted_dynamic_binary(ctx, fname, left, right); } // A non-primitive operand may `ToNumeric` to a BigInt at runtime // (`Object(1n)`, or an object with a BigInt-returning @@ -568,12 +580,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // operands, so a pointer-bearing left operand must // survive the right operand's evaluation. let fname = bigint_dynamic_helper(*op); - let (l, r, guard) = lower_operand_pair_rooted(ctx, left, right)?; - let value = ctx - .block() - .call(DOUBLE, fname, &[(DOUBLE, &l), (DOUBLE, &r)]); - temp_root_release(ctx, guard); - return Ok(value); + return lower_rooted_dynamic_binary(ctx, fname, left, right); } } } diff --git a/crates/perry-codegen/src/expr/dyn_extern_i18n.rs b/crates/perry-codegen/src/expr/dyn_extern_i18n.rs index a73ff6713c..391de5f875 100644 --- a/crates/perry-codegen/src/expr/dyn_extern_i18n.rs +++ b/crates/perry-codegen/src/expr/dyn_extern_i18n.rs @@ -9,6 +9,7 @@ use perry_hir::types::Type as HirType; use perry_hir::Expr; use crate::nanbox::{double_literal, POINTER_MASK_I64}; +use crate::rooting::{with_rooted_accumulator, Arg, Repr}; use crate::types::{DOUBLE, I32, I64, PTR}; use super::{ @@ -942,38 +943,53 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // core` materializes 269 members here, and the emitted IR // carried ZERO `js_gc_temp_root_*` calls beside its 269 // allocating stores. - let rooted = super::temp_root::rooted_handle_begin(ctx, &handle, true); - for member in &members { - let member_get = Expr::PropertyGet { - byte_offset: 0, - object: Box::new(Expr::ExternFuncRef { - name: name.clone(), - param_types: Vec::new(), - return_type: HirType::Any, - }), - property: member.clone(), - }; - let v_box = lower_expr(ctx, &member_get)?; - let key_idx = ctx.strings.intern(member); - let key_handle_global = - format!("@{}", ctx.strings.entry(key_idx).handle_global); - // Re-read AFTER the member resolution: that is the - // collection point, so a register captured before it is - // the stale one. - let handle = super::temp_root::rooted_handle_get(ctx, &rooted); - let blk = ctx.block(); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); - blk.call_void( - "js_object_set_field_by_name", - &[(I64, &handle), (I64, &key_raw), (DOUBLE, &v_box)], - ); - } - let handle = super::temp_root::rooted_handle_get(ctx, &rooted); - let boxed = nanbox_pointer_inline(ctx.block(), &handle); - super::temp_root::rooted_handle_release(ctx, rooted); - return Ok(boxed); + // + // #7615 slice 8: this is `with_rooted_accumulator`'s shape + // exactly — a half-built container written once per member + // with arbitrary user code lowered between the writes — so + // the re-read is fused to the emission that consumes it + // (`RootedAcc::call_void` materialises argument 0 from the + // slot immediately before the call) instead of being a + // register the loop holds. `protect` is unconditionally + // `true` because both halves of the loop body collect on + // every iteration, which is what the paragraph above + // establishes. + return with_rooted_accumulator( + ctx, + Repr::Ptr, + &handle, + true, + |ctx, acc| { + for member in &members { + let member_get = Expr::PropertyGet { + byte_offset: 0, + object: Box::new(Expr::ExternFuncRef { + name: name.clone(), + param_types: Vec::new(), + return_type: HirType::Any, + }), + property: member.clone(), + }; + let v_box = lower_expr(ctx, &member_get)?; + let key_idx = ctx.strings.intern(member); + let key_handle_global = + format!("@{}", ctx.strings.entry(key_idx).handle_global); + let key_raw = { + let blk = ctx.block(); + let key_box = blk.load(DOUBLE, &key_handle_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + blk.and(I64, &key_bits, POINTER_MASK_I64) + }; + acc.call_void( + ctx, + "js_object_set_field_by_name", + &[Arg::Plain(I64, &key_raw), Arg::Plain(DOUBLE, &v_box)], + ); + } + Ok(()) + }, + |ctx, handle| Ok(nanbox_pointer_inline(ctx.block(), handle)), + ); } return Ok(ctx .block() diff --git a/crates/perry-codegen/src/expr/fs_await.rs b/crates/perry-codegen/src/expr/fs_await.rs index 3ed158496a..9ffc923f95 100644 --- a/crates/perry-codegen/src/expr/fs_await.rs +++ b/crates/perry-codegen/src/expr/fs_await.rs @@ -125,7 +125,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // (GC_TYPE_PROMISE) with frame #1 in generated code. The temp-root // slot is what the collector rewrites, so every block re-reads it // instead of reusing the register. - let promise_root = g.adopt_emitted(ctx, Repr::Boxed, &promise_box); + let promise_root = g.adopt_emitted(ctx, Repr::Boxed, &promise_box, true); let result_slot = ctx.func.alloca_entry(DOUBLE); // Pre-seed with the boxed operand so the non-promise // branch just needs to jump to merge. diff --git a/crates/perry-codegen/src/expr/literals_vars.rs b/crates/perry-codegen/src/expr/literals_vars.rs index 8c0c7aa707..2cf35f1d53 100644 --- a/crates/perry-codegen/src/expr/literals_vars.rs +++ b/crates/perry-codegen/src/expr/literals_vars.rs @@ -8,7 +8,7 @@ use anyhow::Result; use perry_hir::types::Type as HirType; use perry_hir::{BinaryOp, Expr, UpdateOp}; -use crate::lower_string_method::lower_string_self_append; +use crate::lower_string_concat::lower_string_self_append; use crate::nanbox::double_literal; use crate::native_value::MaterializationReason; use crate::type_analysis::{is_map_expr, is_set_expr, receiver_class_name}; diff --git a/crates/perry-codegen/src/expr/math_simple.rs b/crates/perry-codegen/src/expr/math_simple.rs index c81adf7d02..f222a27bb8 100644 --- a/crates/perry-codegen/src/expr/math_simple.rs +++ b/crates/perry-codegen/src/expr/math_simple.rs @@ -8,7 +8,7 @@ use anyhow::Result; use perry_hir::types::Type as HirType; use perry_hir::{BinaryOp, Expr}; -use crate::expr::temp_root; +use crate::rooting::{operand_may_collect, with_operands_rooted, with_rooted_group, RootedGroup}; use crate::type_analysis::{is_definitely_string_expr, is_numeric_expr, map_static_type_args}; use crate::types::{DOUBLE, F32, I1, I32, I64}; @@ -277,11 +277,10 @@ fn guarded_map_number_key_set( /// `m_handle_unrooted` is the eagerly computed handle, so nothing is emitted. fn reread_map_set_receiver_and_key( ctx: &mut FnCtx<'_>, - roots: &temp_root::RootedOperands, - operands: &[&Expr; 2], + group: &RootedGroup<'_>, m_handle_unrooted: &Option, ) -> Result<(String, String)> { - let values = roots.reread(ctx, operands)?; + let values = group.reread_all(ctx)?; let k_box = values[1].clone(); let m_handle = match m_handle_unrooted { Some(handle) => handle.clone(), @@ -502,24 +501,37 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // calls it for each element. The callback expression usually // lowers to a NaN-boxed closure value, which we unbox to i64. Expr::ArrayMap { array, callback } => { - let arr_box = lower_expr(ctx, array)?; - let cb_box = lower_expr(ctx, callback)?; - let blk = ctx.block(); - let arr_handle = unbox_to_i64(blk, &arr_box); - // #4091: throw TypeError for a non-callable callback before iterating. - // `map` uses a receiver-aware validator (TypedArray.map renders its - // non-callable message differently than Array.prototype.map). - let cb_handle = blk.call( - I64, - "js_validate_array_map_callback", - &[(I64, &arr_handle), (DOUBLE, &cb_box)], - ); - let result = blk.call( - I64, - "js_array_map", - &[(I64, &arr_handle), (I64, &cb_handle)], - ); - Ok(nanbox_pointer_inline(blk, &result)) + // #7615 slice 8: the receiver was lowered, then `callback` was + // lowered, and only THEN was the receiver unboxed — so the unbox + // sat *below* its own window and masked a stale box rather than + // repairing it (#7280 taxonomy (c): an operand-to-operand window). + // `arr.map(x => …)` allocates a closure for the callback at + // minimum, and an arbitrary callback expression runs user code, so + // the window is not hypothetical. + // + // `js_validate_array_map_callback` between the unbox and + // `js_array_map` is deliberately NOT treated as a second window: + // it is `AllocNoReentry` in `gc_call_effects.rs` (a type check plus + // a static-message throw), and #7198's position is that a helper + // which merely allocates cannot initiate a moving collection. + with_operands_rooted(ctx, &[array, callback], |ctx, values| { + let blk = ctx.block(); + let arr_handle = unbox_to_i64(blk, &values[0]); + // #4091: throw TypeError for a non-callable callback before iterating. + // `map` uses a receiver-aware validator (TypedArray.map renders its + // non-callable message differently than Array.prototype.map). + let cb_handle = blk.call( + I64, + "js_validate_array_map_callback", + &[(I64, &arr_handle), (DOUBLE, &values[1])], + ); + let result = blk.call( + I64, + "js_array_map", + &[(I64, &arr_handle), (I64, &cb_handle)], + ); + Ok(nanbox_pointer_inline(blk, &result)) + }) } // -------- map.set(key, value) / .get / .has -------- @@ -561,352 +573,319 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // and rooting a list that is already lowered can publish an // already-dangling pointer into a scanned slot — strictly worse // than not rooting at all. - let key_collects = temp_root::expr_may_trigger_gc(ctx, key); - let value_collects = temp_root::expr_may_trigger_gc(ctx, value); - let map_key_operands: [&Expr; 2] = [map, key]; - let mut roots = temp_root::root_operands_begin(2); - let m_box = lower_expr(ctx, map)?; - roots.push(ctx, map, &m_box, key_collects || value_collects); - let k_box = lower_expr(ctx, key)?; - roots.push(ctx, key, &k_box, value_collects); - // Unbox eagerly only on the unprotected path, so its IR — including - // register numbering — is exactly what it was before this change. - // On the protected path the handle has to come from the *re-read* - // box, so it is derived after `value` is lowered instead. - let m_handle_unrooted = (!roots.is_rooted()).then(|| { - let blk = ctx.block(); - unbox_to_i64(blk, &m_box) - }); - let new_handle = if use_string_i32_map { - let value_i32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I32)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, new_handle) = { + let key_collects = operand_may_collect(ctx, key); + let value_collects = operand_may_collect(ctx, value); + let new_handle = with_rooted_group(ctx, 2, |ctx, group| { + group.lower(ctx, map, key_collects || value_collects)?; + group.lower(ctx, key, value_collects)?; + // Unbox eagerly only on the unprotected path, so its IR — including + // register numbering — is exactly what it was before this change. + // On the protected path the handle has to come from the *re-read* + // box, so it is derived after `value` is lowered instead. + // + // `reread` of an UNROOTED operand emits nothing and hands the + // original register back, so this is the same eager unbox it was: + // a Map receiver is never `Expr::String`, the only expression + // `operand_is_reloadable` would re-lower here. + let m_handle_unrooted = if group.is_rooted() { + None + } else { + let m_box = group.reread(ctx, 0)?; let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let new_handle = blk.call( - I64, + Some(unbox_to_i64(blk, &m_box)) + }; + let new_handle = if use_string_i32_map { + let value_i32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I32)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let new_handle = blk.call( + I64, + "js_map_set_string_i32", + &[(I64, &m_handle), (I64, &k_handle), (I32, &value_i32.value)], + ); + (k_handle, new_handle) + }; + record_collection_string_key_value_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_i32", + &value_i32, + "map", + "int32_value_helper", "js_map_set_string_i32", - &[(I64, &m_handle), (I64, &k_handle), (I32, &value_i32.value)], ); - (k_handle, new_handle) - }; - record_collection_string_key_value_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_i32", - &value_i32, - "map", - "int32_value_helper", - "js_map_set_string_i32", - ); - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_i32_key", - &k_handle, - "map", - "js_map_set_string_i32", - ); - new_handle - } else if use_string_u32_map { - let value_u32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::U32)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, new_handle) = { - let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let new_handle = blk.call( - I64, + record_collection_string_key_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_i32_key", + &k_handle, + "map", + "js_map_set_string_i32", + ); + new_handle + } else if use_string_u32_map { + let value_u32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::U32)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let new_handle = blk.call( + I64, + "js_map_set_string_u32", + &[(I64, &m_handle), (I64, &k_handle), (I32, &value_u32.value)], + ); + (k_handle, new_handle) + }; + record_collection_string_key_value_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_u32", + &value_u32, + "map", + "uint32_value_helper", "js_map_set_string_u32", - &[(I64, &m_handle), (I64, &k_handle), (I32, &value_u32.value)], ); - (k_handle, new_handle) - }; - record_collection_string_key_value_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_u32", - &value_u32, - "map", - "uint32_value_helper", - "js_map_set_string_u32", - ); - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_u32_key", - &k_handle, - "map", - "js_map_set_string_u32", - ); - new_handle - } else if use_string_f32_map { - let value_f32 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::F32)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, new_handle) = { - let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let new_handle = blk.call( - I64, + record_collection_string_key_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_u32_key", + &k_handle, + "map", + "js_map_set_string_u32", + ); + new_handle + } else if use_string_f32_map { + let value_f32 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::F32)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let new_handle = blk.call( + I64, + "js_map_set_string_f32", + &[(I64, &m_handle), (I64, &k_handle), (F32, &value_f32.value)], + ); + (k_handle, new_handle) + }; + record_collection_string_key_value_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_f32", + &value_f32, + "map", + "float32_value_helper", "js_map_set_string_f32", - &[(I64, &m_handle), (I64, &k_handle), (F32, &value_f32.value)], ); - (k_handle, new_handle) - }; - record_collection_string_key_value_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_f32", - &value_f32, - "map", - "float32_value_helper", - "js_map_set_string_f32", - ); - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_f32_key", - &k_handle, - "map", - "js_map_set_string_f32", - ); - new_handle - } else if use_string_number_map { - let v_box = lower_expr(ctx, value)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, new_handle) = { - let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let new_handle = blk.call( - I64, + record_collection_string_key_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_f32_key", + &k_handle, + "map", + "js_map_set_string_f32", + ); + new_handle + } else if use_string_number_map { + let v_box = lower_expr(ctx, value)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let new_handle = blk.call( + I64, + "js_map_set_string_number", + &[(I64, &m_handle), (I64, &k_handle), (DOUBLE, &v_box)], + ); + (k_handle, new_handle) + }; + record_collection_string_key_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_number", + &k_handle, + "map", "js_map_set_string_number", - &[(I64, &m_handle), (I64, &k_handle), (DOUBLE, &v_box)], ); - (k_handle, new_handle) - }; - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_number", - &k_handle, - "map", - "js_map_set_string_number", - ); - new_handle - } else if use_string_boolean_map { - let value_i1 = - lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I1)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, new_handle) = { - let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let value_i32 = blk.zext(I1, &value_i1.value, I32); - let new_handle = blk.call( - I64, + new_handle + } else if use_string_boolean_map { + let value_i1 = + lower_expr_native(ctx, value, crate::native_value::ExpectedNativeRep::I1)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let value_i32 = blk.zext(I1, &value_i1.value, I32); + let new_handle = blk.call( + I64, + "js_map_set_string_bool", + &[(I64, &m_handle), (I64, &k_handle), (I32, &value_i32)], + ); + (k_handle, new_handle) + }; + record_collection_string_key_value_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_bool", + &value_i1, + "map", + "boolean_value_helper", "js_map_set_string_bool", - &[(I64, &m_handle), (I64, &k_handle), (I32, &value_i32)], ); - (k_handle, new_handle) - }; - record_collection_string_key_value_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_bool", - &value_i1, - "map", - "boolean_value_helper", - "js_map_set_string_bool", - ); - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_bool_key", - &k_handle, - "map", - "js_map_set_string_bool", - ); - new_handle - } else if use_string_string_map { - let v_box = lower_expr(ctx, value)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, v_handle, new_handle) = { - let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let v_handle = unbox_str_handle(blk, &v_box); - let new_handle = blk.call( - I64, + record_collection_string_key_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_bool_key", + &k_handle, + "map", + "js_map_set_string_bool", + ); + new_handle + } else if use_string_string_map { + let v_box = lower_expr(ctx, value)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, v_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let v_handle = unbox_str_handle(blk, &v_box); + let new_handle = blk.call( + I64, + "js_map_set_string_string", + &[(I64, &m_handle), (I64, &k_handle), (I64, &v_handle)], + ); + (k_handle, v_handle, new_handle) + }; + let lowered_value = crate::native_value::LoweredValue::string_ref(&v_handle); + record_collection_string_key_value_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_string", + &lowered_value, + "map", + "string_value_helper", "js_map_set_string_string", - &[(I64, &m_handle), (I64, &k_handle), (I64, &v_handle)], ); - (k_handle, v_handle, new_handle) - }; - let lowered_value = crate::native_value::LoweredValue::string_ref(&v_handle); - record_collection_string_key_value_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_string", - &lowered_value, - "map", - "string_value_helper", - "js_map_set_string_string", - ); - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_string_key", - &k_handle, - "map", - "js_map_set_string_string", - ); - new_handle - } else if has_string_key_map { - let v_box = lower_expr(ctx, value)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (k_handle, new_handle) = { - let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let new_handle = blk.call( - I64, - "js_map_set_string_key", - &[(I64, &m_handle), (I64, &k_handle), (DOUBLE, &v_box)], + record_collection_string_key_selected( + ctx, + "MapSet", + "collection_string_key.map_set_string_string_key", + &k_handle, + "map", + "js_map_set_string_string", ); - (k_handle, new_handle) - }; - record_collection_string_key_selected( - ctx, - "MapSet", - "collection_string_key.map_set_string_key", - &k_handle, - "map", - "js_map_set_string_key", - ); - if static_string_boolean_map { - record_collection_typed_value_fallback( + new_handle + } else if has_string_key_map { + let v_box = lower_expr(ctx, value)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (k_handle, new_handle) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let new_handle = blk.call( + I64, + "js_map_set_string_key", + &[(I64, &m_handle), (I64, &k_handle), (DOUBLE, &v_box)], + ); + (k_handle, new_handle) + }; + record_collection_string_key_selected( ctx, "MapSet", - "collection_typed_value.map_set_string_bool_generic", - &v_box, + "collection_string_key.map_set_string_key", + &k_handle, "map", - "boolean_value_helper", "js_map_set_string_key", - "value_expr_not_native_i1", ); - } - new_handle - } else if use_number_string_map { - let v_box = lower_expr(ctx, value)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let (v_handle, v_slot_box) = { - let blk = ctx.block(); - let v_handle = unbox_str_handle(blk, &v_box); - let v_slot_box = nanbox_string_inline(blk, &v_handle); - (v_handle, v_slot_box) - }; - let lowered_value = crate::native_value::LoweredValue::string_ref(&v_handle); - record_collection_typed_value_selected( - ctx, - "MapSet", - "collection_typed_value.map_set_number_string", - &lowered_value, - "map", - "string_value_helper", - "js_map_set_number_key", - "map_slot", - ); - guarded_map_number_key_set(ctx, &m_handle, &k_box, &v_slot_box) - } else if use_number_key_map { - let v_box = lower_expr(ctx, value)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - if static_number_string_map { - record_collection_typed_value_fallback( + if static_string_boolean_map { + record_collection_typed_value_fallback( + ctx, + "MapSet", + "collection_typed_value.map_set_string_bool_generic", + &v_box, + "map", + "boolean_value_helper", + "js_map_set_string_key", + "value_expr_not_native_i1", + ); + } + new_handle + } else if use_number_string_map { + let v_box = lower_expr(ctx, value)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let (v_handle, v_slot_box) = { + let blk = ctx.block(); + let v_handle = unbox_str_handle(blk, &v_box); + let v_slot_box = nanbox_string_inline(blk, &v_handle); + (v_handle, v_slot_box) + }; + let lowered_value = crate::native_value::LoweredValue::string_ref(&v_handle); + record_collection_typed_value_selected( ctx, "MapSet", - "collection_typed_value.map_set_number_string_generic", - &v_box, + "collection_typed_value.map_set_number_string", + &lowered_value, "map", "string_value_helper", "js_map_set_number_key", - "value_expr_not_definitely_string", + "map_slot", ); - } - guarded_map_number_key_set(ctx, &m_handle, &k_box, &v_box) - } else { - let v_box = lower_expr(ctx, value)?; - let (m_handle, k_box) = reread_map_set_receiver_and_key( - ctx, - &roots, - &map_key_operands, - &m_handle_unrooted, - )?; - let new_handle = { - let blk = ctx.block(); - blk.call( - I64, + guarded_map_number_key_set(ctx, &m_handle, &k_box, &v_slot_box) + } else if use_number_key_map { + let v_box = lower_expr(ctx, value)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + if static_number_string_map { + record_collection_typed_value_fallback( + ctx, + "MapSet", + "collection_typed_value.map_set_number_string_generic", + &v_box, + "map", + "string_value_helper", + "js_map_set_number_key", + "value_expr_not_definitely_string", + ); + } + guarded_map_number_key_set(ctx, &m_handle, &k_box, &v_box) + } else { + let v_box = lower_expr(ctx, value)?; + let (m_handle, k_box) = + reread_map_set_receiver_and_key(ctx, group, &m_handle_unrooted)?; + let new_handle = { + let blk = ctx.block(); + blk.call( + I64, + "js_map_set", + &[(I64, &m_handle), (DOUBLE, &k_box), (DOUBLE, &v_box)], + ) + }; + record_collection_string_key_fallback( + ctx, + "MapSet", + "collection_string_key.map_set_generic", + &k_box, + "map", "js_map_set", - &[(I64, &m_handle), (DOUBLE, &k_box), (DOUBLE, &v_box)], - ) + "receiver_or_key_not_static_string", + ); + new_handle }; - record_collection_string_key_fallback( - ctx, - "MapSet", - "collection_string_key.map_set_generic", - &k_box, - "map", - "js_map_set", - "receiver_or_key_not_static_string", - ); - new_handle - }; - // Released only now: the runtime call above allocates while it - // reads the key, so the group has to stay rooted across it. - roots.release(ctx); + // Released by `with_rooted_group` on every path out, including the + // `?` of a value lowering above: the runtime call allocates while + // it reads the key, so the group stays rooted across it. + Ok(new_handle) + })?; // map.set returns the (possibly-realloc'd) map. Re-NaN-box // and return. The caller may need to write this back to a // local; that's the caller's problem if Map is held in a @@ -922,50 +901,52 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { && is_numeric_expr(ctx, key); // #6970: `key` is lowered after the receiver and can collect, so the // receiver would otherwise sit unrooted in an SSA register across it. - let (m_box, k_box, guard) = temp_root::lower_operand_pair_rooted(ctx, map, key)?; - let m_handle = { - let blk = ctx.block(); - unbox_to_i64(blk, &m_box) - }; - let value = if use_string_key_map { - let (k_handle, value) = { + let value = with_operands_rooted(ctx, &[map, key], |ctx, values| { + let (m_box, k_box) = (values[0].clone(), values[1].clone()); + let m_handle = { let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let value = blk.call( - DOUBLE, + unbox_to_i64(blk, &m_box) + }; + let value = if use_string_key_map { + let (k_handle, value) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let value = blk.call( + DOUBLE, + "js_map_get_string_key", + &[(I64, &m_handle), (I64, &k_handle)], + ); + (k_handle, value) + }; + record_collection_string_key_selected( + ctx, + "MapGet", + "collection_string_key.map_get", + &k_handle, + "map", "js_map_get_string_key", - &[(I64, &m_handle), (I64, &k_handle)], ); - (k_handle, value) - }; - record_collection_string_key_selected( - ctx, - "MapGet", - "collection_string_key.map_get", - &k_handle, - "map", - "js_map_get_string_key", - ); - value - } else if use_number_key_map { - guarded_map_number_key_get(ctx, &m_handle, &k_box) - } else { - let value = { - let blk = ctx.block(); - blk.call(DOUBLE, "js_map_get", &[(I64, &m_handle), (DOUBLE, &k_box)]) + value + } else if use_number_key_map { + guarded_map_number_key_get(ctx, &m_handle, &k_box) + } else { + let value = { + let blk = ctx.block(); + blk.call(DOUBLE, "js_map_get", &[(I64, &m_handle), (DOUBLE, &k_box)]) + }; + record_collection_string_key_fallback( + ctx, + "MapGet", + "collection_string_key.map_get_generic", + &k_box, + "map", + "js_map_get", + "receiver_or_key_not_static_string", + ); + value }; - record_collection_string_key_fallback( - ctx, - "MapGet", - "collection_string_key.map_get_generic", - &k_box, - "map", - "js_map_get", - "receiver_or_key_not_static_string", - ); - value - }; - temp_root::temp_root_release(ctx, guard); + Ok(value) + })?; Ok(value) } Expr::MapHas { map, key } => { @@ -976,50 +957,52 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { && is_numeric_expr(ctx, key); // #6970: same hazard as `MapGet` — the key's lowering can collect // while the receiver is live only in an SSA register. - let (m_box, k_box, guard) = temp_root::lower_operand_pair_rooted(ctx, map, key)?; - let m_handle = { - let blk = ctx.block(); - unbox_to_i64(blk, &m_box) - }; - let i32_v = if use_string_key_map { - let (k_handle, i32_v) = { + let i32_v = with_operands_rooted(ctx, &[map, key], |ctx, values| { + let (m_box, k_box) = (values[0].clone(), values[1].clone()); + let m_handle = { let blk = ctx.block(); - let k_handle = unbox_str_handle(blk, &k_box); - let i32_v = blk.call( - I32, + unbox_to_i64(blk, &m_box) + }; + let i32_v = if use_string_key_map { + let (k_handle, i32_v) = { + let blk = ctx.block(); + let k_handle = unbox_str_handle(blk, &k_box); + let i32_v = blk.call( + I32, + "js_map_has_string_key", + &[(I64, &m_handle), (I64, &k_handle)], + ); + (k_handle, i32_v) + }; + record_collection_string_key_selected( + ctx, + "MapHas", + "collection_string_key.map_has", + &k_handle, + "map", "js_map_has_string_key", - &[(I64, &m_handle), (I64, &k_handle)], ); - (k_handle, i32_v) - }; - record_collection_string_key_selected( - ctx, - "MapHas", - "collection_string_key.map_has", - &k_handle, - "map", - "js_map_has_string_key", - ); - i32_v - } else if use_number_key_map { - guarded_map_number_key_has(ctx, &m_handle, &k_box) - } else { - let i32_v = { - let blk = ctx.block(); - blk.call(I32, "js_map_has", &[(I64, &m_handle), (DOUBLE, &k_box)]) + i32_v + } else if use_number_key_map { + guarded_map_number_key_has(ctx, &m_handle, &k_box) + } else { + let i32_v = { + let blk = ctx.block(); + blk.call(I32, "js_map_has", &[(I64, &m_handle), (DOUBLE, &k_box)]) + }; + record_collection_string_key_fallback( + ctx, + "MapHas", + "collection_string_key.map_has_generic", + &k_box, + "map", + "js_map_has", + "receiver_or_key_not_static_string", + ); + i32_v }; - record_collection_string_key_fallback( - ctx, - "MapHas", - "collection_string_key.map_has_generic", - &k_box, - "map", - "js_map_has", - "receiver_or_key_not_static_string", - ); - i32_v - }; - temp_root::temp_root_release(ctx, guard); + Ok(i32_v) + })?; // NaN-tagged boolean for "true"/"false" printing. let blk = ctx.block(); let bit = blk.icmp_ne(I32, &i32_v, "0"); diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 1417a83c58..7cb7d8916a 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -136,11 +136,15 @@ mod record_value; mod repsel_gates; mod scalar_slot_root; pub(crate) mod shadow_inline; -mod shadow_slot; +// `pub(crate)` since #7615 slice 8: `rooting/temp_root.rs` binds a pooled +// temp alloca through the same shadow-slot emission every named local uses, +// and it now lives outside `crate::expr`. +pub(crate) mod shadow_slot; #[cfg(test)] mod slice7_rooting_tests; +#[cfg(test)] +mod slice8_rooting_tests; mod slot_rep; -pub(crate) mod temp_root; // #7128: the env-knob table and the pure `gates -> context flags` derivation. // Every `FnCtx` construction site goes through `RepselContextFlags` so that a // knob cannot silently acquire a second representation's sites again. @@ -726,9 +730,9 @@ pub(crate) struct FnCtx<'a> { pub shadow_slots_bound: std::collections::HashSet, /// #7469: pooled frame-rooted allocas for expression temporaries — see - /// [`temp_root::TempRootPool`]. Starts empty; grows on the first + /// [`crate::rooting::TempRootPool`]. Starts empty; grows on the first /// protected temporary this function lowers. - pub temp_roots: temp_root::TempRootPool, + pub temp_roots: crate::rooting::TempRootPool, /// Cached pointer to this function's `InlineArenaState` slot — /// allocated lazily on the first `new ClassName()` site that uses diff --git a/crates/perry-codegen/src/expr/proxy_reflect.rs b/crates/perry-codegen/src/expr/proxy_reflect.rs index cdb2131d9a..f89f2f353f 100644 --- a/crates/perry-codegen/src/expr/proxy_reflect.rs +++ b/crates/perry-codegen/src/expr/proxy_reflect.rs @@ -1252,7 +1252,7 @@ fn try_lower_process_env_put_value_set( let property_key = ctx .block() .call(DOUBLE, "js_to_property_key", &[(DOUBLE, &key_box)]); - let key_slot = g.adopt_emitted(ctx, Repr::Boxed, &property_key); + let key_slot = g.adopt_emitted(ctx, Repr::Boxed, &property_key, true); let val_double = lower_expr(ctx, value)?; // The strip happens BELOW the window, never above it. let key_box = g.reread_emitted(ctx, key_slot); diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index b7c32c7978..a339cf6365 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -210,7 +210,7 @@ pub(crate) fn emit_shadow_slot_bind_for_local(ctx: &mut FnCtx<'_>, local_id: u32 /// Bind frame slot `slot_idx` to the root alloca `slot_ptr` — the raw form of /// [`emit_shadow_slot_bind_for_local`], for roots that are not named locals -/// (#7469: the pooled temp-root allocas in `temp_root.rs`). +/// (#7469: the pooled temp-root allocas in `rooting/temp_root.rs`). /// /// The caller owns the pairing of `slot_idx` and `slot_ptr`; everything else /// — the stack-map textual marker, the #7088 inline frame write, the FFI diff --git a/crates/perry-codegen/src/expr/slice7_rooting_tests.rs b/crates/perry-codegen/src/expr/slice7_rooting_tests.rs index 7f8d9c8cf5..5db43fca74 100644 --- a/crates/perry-codegen/src/expr/slice7_rooting_tests.rs +++ b/crates/perry-codegen/src/expr/slice7_rooting_tests.rs @@ -54,7 +54,7 @@ fn compile_body(name: &str, body: Vec) -> String { } /// A heap value whose lowering allocates, so the window it sits in collects. -fn allocating(tag: &str) -> Expr { +pub(super) fn allocating(tag: &str) -> Expr { Expr::Object(vec![(tag.to_string(), Expr::Number(1.0))]) } @@ -69,7 +69,7 @@ fn call_line(ir: &str, callee: &str) -> Option { .position(|l| l.contains(&needle) && !l.trim_start().starts_with("declare")) } -fn require_call_line(ir: &str, callee: &str) -> usize { +pub(super) fn require_call_line(ir: &str, callee: &str) -> usize { call_line(ir, callee).unwrap_or_else(|| panic!("no call to {callee} in:\n{ir}")) } @@ -120,7 +120,7 @@ fn last_alloc(ir: &str) -> usize { /// Temp-root traffic, excluding the `declare` lines that name the helpers /// whether or not anything calls them. -fn temp_root_calls(ir: &str) -> usize { +pub(super) fn temp_root_calls(ir: &str) -> usize { ir.lines() .filter(|l| !l.trim_start().starts_with("declare")) .filter(|l| l.contains("js_gc_temp_root")) @@ -130,7 +130,7 @@ fn temp_root_calls(ir: &str) -> usize { /// Assert that the register `callee` reads as operand `n` is defined below the /// last allocation above the call — i.e. that the value was re-read after the /// window rather than carried across it. -fn assert_operand_survives_the_window(ir: &str, callee: &str, n: usize, what: &str) { +pub(super) fn assert_operand_survives_the_window(ir: &str, callee: &str, n: usize, what: &str) { let call = require_call_line(ir, callee); let reg = call_operand(ir, callee, n); let def = require_definition_line(ir, ®); diff --git a/crates/perry-codegen/src/expr/slice8_rooting_tests.rs b/crates/perry-codegen/src/expr/slice8_rooting_tests.rs new file mode 100644 index 0000000000..4b4dee8f0a --- /dev/null +++ b/crates/perry-codegen/src/expr/slice8_rooting_tests.rs @@ -0,0 +1,342 @@ +//! Rooting coverage for the windows #7615 slice 8 closed. +//! +//! # What is asserted, and why it cannot pass vacuously +//! +//! The harness is slice 7's, reused rather than copied — the same +//! "the register the consuming call reads must be DEFINED BELOW the last +//! allocation above the call" ordering property, and the same rule that every +//! test first proves by callee name that the arm under test was reached at all. +//! +//! **Ordering, never slot counts, and the reason is sharper here than in slice +//! 7.** A slot-width assertion silently measures nothing on the DEFAULT build: +//! under statepoints `reserve_shadow_slot` hands back a stack-map index and no +//! `js_shadow_frame_enter` is emitted at all, so a test that counts frame slots +//! reads zero and passes. The definition-line ordering is visible in all three +//! lowerings — pooled alloca, shadow frame and FFI fallback — because all three +//! must produce the re-read below the window or they are not rooting anything. +//! +//! **The zero-cost arm is a test too.** `[1,2].map(f)` where `f` is inert +//! cannot collect between the receiver and `js_array_map`, so +//! `operand_protection` must answer `Reuse` and emit nothing. Without that pin +//! a future "root everything" change would tax every call site unnoticed. +//! +//! # ★ The receiver is a PROPERTY READ, and that is load-bearing +//! +//! The first version of these tests used an array-typed LOCAL as the receiver, +//! and the sabotage arm — restore the pre-fix lowering, require red — came back +//! green. The reason is worth more than the tests: for a local with a shadow +//! slot, codegen's `ptr addrspace(1)` retype pass **rematerialises the load +//! from that slot at the use site**, so the pre-fix code re-read the receiver +//! by accident and had no window at all. Measured on the perry-dev A/B: the +//! baseline arm emits `%rN.rs4p = load ptr addrspace(1), ptr %slot` BELOW the +//! callback's allocation, all by itself. +//! +//! The windows that are real — verified by A/B on emitted IR — are the +//! receivers with no slot to rematerialise from: a MODULE GLOBAL, a CLASS +//! FIELD read, and a CLOSURE CAPTURE (`js_closure_get_capture_bits`, a raw +//! `i64`, #7280 taxonomy (a)). These tests use the field read, because it is +//! the one a single HIR function can express. + +use perry_hir::types::Type; +use perry_hir::{Expr, Function, Module as HirModule, Stmt}; + +use super::slice7_rooting_tests::{allocating, require_call_line, temp_root_calls}; + +/// Instructions that move bits around without reading memory, calling anything +/// or joining control flow — so a register defined by one of these is only as +/// fresh as ITS input. +const PURE_OPS: &[&str] = &[ + "bitcast", + "and ", + "or ", + "xor ", + "shl ", + "lshr", + "ashr", + "ptrtoint", + "inttoptr", + "zext", + "sext", + "trunc", + "add ", + "sub ", + "getelementptr", + "select", +]; + +/// Walk `reg`'s definition chain up through [`PURE_OPS`] and return the line of +/// the first instruction that actually PRODUCES a value — a call, a load, a phi +/// or an argument. +/// +/// ★ **This is the whole reason this file does not reuse slice 7's +/// `assert_operand_survives_the_window`.** That helper compares the operand +/// register's own definition line against the window, and for +/// `Expr::ArrayMap` that is a `and i64 %stale, POINTER_MASK` — which the +/// pre-fix lowering emitted BELOW the window while masking a register loaded +/// ABOVE it. Measured, not reasoned: the first version of these tests used the +/// shallow helper, and the sabotage arm (restore the pre-fix lowering, require +/// red) came back GREEN on all four. "The unbox sits below its own window" is +/// exactly the shape a one-level check cannot see, which is #7280 taxonomy (c) +/// stated as a property of the instrument instead of the bug. +fn producer_line(ir: &str, reg: &str) -> usize { + let lines: Vec<&str> = ir.lines().collect(); + let mut current = reg.to_string(); + for _ in 0..32 { + let prefix = format!("{current} = "); + let Some(idx) = lines + .iter() + .position(|l| l.trim_start().starts_with(&prefix)) + else { + panic!("no definition for {current} in:\n{ir}"); + }; + let rhs = lines[idx].split_once(" = ").expect("matched on ' = '").1; + let Some(op) = PURE_OPS + .iter() + .find(|op| rhs.trim_start().starts_with(**op)) + else { + return idx; + }; + // Follow the FIRST register operand of the pure op. + let Some(next) = rhs[op.len()..] + .split(&[',', ' '][..]) + .find(|t| t.starts_with('%')) + else { + return idx; // constant-only pure op: it is its own producer + }; + current = next.trim_end_matches(')').to_string(); + } + panic!("definition chain for {reg} did not terminate in:\n{ir}"); +} + +/// Line index of a call to `callee` that is not a `declare`. +fn call_line_of(ir: &str, callee: &str) -> usize { + let needle = format!("@{callee}("); + ir.lines() + .position(|l| l.contains(&needle) && !l.trim_start().starts_with("declare")) + .unwrap_or_else(|| panic!("no call to {callee} in:\n{ir}")) +} + +/// The `n`-th SSA operand of the call to `callee`. +fn call_operand_of(ir: &str, callee: &str, n: usize) -> String { + let idx = call_line_of(ir, callee); + let line = ir.lines().nth(idx).expect("index came from this IR"); + let args = line + .rsplit_once('(') + .unwrap_or_else(|| panic!("{callee} call has no argument list: {line}")) + .1; + args.split(',') + .nth(n) + .unwrap_or_else(|| panic!("{callee} has no operand {n}: {line}")) + .trim() + .rsplit(' ') + .next() + .expect("an operand is a type followed by a register") + .trim_end_matches(')') + .to_string() +} + +/// Assert that the value `consumer` reads as operand `consumer_n` was PRODUCED +/// **below** the value `window` reads as operand `window_n`. +/// +/// The window is named by the LATER OPERAND'S OWN PRODUCER rather than by "the +/// last object allocation above the call". Which helper an `Expr::Object` +/// lowering reaches for is not a property of this module — +/// `js_object_alloc_with_shape`, an inline bump and +/// `js_object_alloc_class_inline_keys` are all reachable, and the last two emit +/// no `@js_object_alloc` line at all — so an allocation anchor is a guess about +/// somebody else's lowering. The operand's own producer is not: whatever the +/// callback's lowering emitted, the register it hands to the call is produced +/// by it, and the receiver must be re-read below that. +fn assert_reread_below_operand( + ir: &str, + consumer: &str, + consumer_n: usize, + window: &str, + window_n: usize, + what: &str, +) { + let reg = call_operand_of(ir, consumer, consumer_n); + let producer = producer_line(ir, ®); + let window_reg = call_operand_of(ir, window, window_n); + let window_line = producer_line(ir, &window_reg); + assert!( + producer > window_line, + "{what}: {consumer} reads {reg}, whose value is PRODUCED at line {producer} — at or \ + above line {window_line}, where {window}'s operand {window_n} ({window_reg}) is \ + produced. That lowering is the window. Masking or bit-casting the receiver below it \ + does not repair it: the bits are still the pre-move address. It must be rooted above \ + the window and re-read below it.\n{ir}" + ); +} + +/// Compile a one-function module and return its LLVM IR. +/// +/// A local copy rather than slice 7's, because these arms need a *local* — the +/// receiver of `arr.map(cb)` has to be something `lower_expr` can lower, and a +/// `LocalGet` needs the slot the declaration creates. +fn compile_body(name: &str, body: Vec) -> String { + let mut hir = HirModule::new(name); + hir.functions.push(Function { + id: 0, + name: "build".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body, + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }); + let opts = crate::CompileOptions { + emit_ir_only: true, + ..Default::default() + }; + let bytes = crate::compile_module(&hir, opts).expect("test module compiles"); + String::from_utf8(bytes).expect("LLVM IR is UTF-8") +} + +/// `let o = { items: [...] }; ` — the receiver reached through a FIELD +/// read, which is a call result with no slot for the retype pass to +/// rematerialise from. See the module header for why a local will not do. +fn with_object_local(id: u32, tail: Stmt) -> Vec { + vec![ + Stmt::Let { + id, + name: "o".to_string(), + ty: Type::Any, + init: Some(Expr::Object(vec![( + "items".to_string(), + Expr::Array(vec![Expr::Number(1.0), Expr::Number(2.0)]), + )])), + mutable: false, + }, + tail, + ] +} + +/// `o.items` — the receiver expression these tests use. +fn field_receiver(id: u32) -> Expr { + Expr::PropertyGet { + byte_offset: 0, + object: Box::new(Expr::LocalGet(id)), + property: "items".to_string(), + } +} + +// --------------------------------------------------------------------------- +// expr/math_simple.rs — Expr::ArrayMap +// --------------------------------------------------------------------------- + +/// The live bug slice 8 fixed. +/// +/// `Expr::ArrayMap` lowered the receiver, lowered the callback, and only THEN +/// unboxed the receiver — so `unbox_to_i64` sat BELOW its own window and masked +/// a stale box rather than repairing it (#7280 taxonomy (c): an +/// operand-to-operand window). The receiver handle `js_array_map` reads must +/// therefore be defined below the callback's allocation. +#[test] +fn array_map_receiver_is_reread_below_the_callback_lowering() { + let ir = compile_body( + "array_map_window", + with_object_local( + 1, + Stmt::Expr(Expr::ArrayMap { + array: Box::new(field_receiver(1)), + callback: Box::new(allocating("cb")), + }), + ), + ); + assert_reread_below_operand( + &ir, + "js_array_map", + 0, + "js_validate_array_map_callback", + 1, + "arr.map(cb) evaluates the receiver first and the callback second, and the \ + callback's lowering allocates", + ); +} + +/// The same window, one instruction earlier: `js_validate_array_map_callback` +/// takes the receiver too, and it is emitted from the same re-read. Asserting +/// both is what stops a "fix" that re-reads for `js_array_map` only and leaves +/// the validator dereferencing from-space. +#[test] +fn array_map_validator_reads_the_same_reread_receiver() { + let ir = compile_body( + "array_map_validator", + with_object_local( + 1, + Stmt::Expr(Expr::ArrayMap { + array: Box::new(field_receiver(1)), + callback: Box::new(allocating("cb")), + }), + ), + ); + assert_reread_below_operand( + &ir, + "js_validate_array_map_callback", + 0, + "js_validate_array_map_callback", + 1, + "the non-callable validator dereferences the receiver as well", + ); +} + +/// The zero-cost arm: an inert callback cannot collect, so nothing is rooted +/// and the IR is what it was before the fix. +#[test] +fn array_map_with_an_inert_callback_emits_no_rooting_traffic() { + let ir = compile_body( + "array_map_cold", + with_object_local( + 1, + Stmt::Expr(Expr::ArrayMap { + array: Box::new(field_receiver(1)), + callback: Box::new(Expr::Undefined), + }), + ), + ); + require_call_line(&ir, "js_array_map"); + assert_eq!( + temp_root_calls(&ir), + 0, + "an inert callback cannot collect between the receiver and js_array_map, so \ + operand_protection must route the receiver to Reuse\n{ir}" + ); +} + +// --------------------------------------------------------------------------- +// expr/math_simple.rs — Expr::MapGet / Expr::MapHas +// --------------------------------------------------------------------------- + +/// `m.get(k)` lowers the receiver before the key, and an allocating key is a +/// window the receiver must survive. This is the `with_operands_rooted` +/// translation of what `lower_operand_pair_rooted` did, pinned so the +/// translation cannot quietly drop the root. +#[test] +fn map_get_receiver_survives_an_allocating_key() { + let ir = compile_body( + "map_get_window", + with_object_local( + 1, + Stmt::Expr(Expr::MapGet { + map: Box::new(field_receiver(1)), + key: Box::new(allocating("k")), + }), + ), + ); + assert_reread_below_operand( + &ir, + "js_map_get", + 0, + "js_map_get", + 1, + "Map.get evaluates the receiver first and the key second", + ); +} diff --git a/crates/perry-codegen/src/expr/static_field_meta.rs b/crates/perry-codegen/src/expr/static_field_meta.rs index cb77254c3e..8638ce0f72 100644 --- a/crates/perry-codegen/src/expr/static_field_meta.rs +++ b/crates/perry-codegen/src/expr/static_field_meta.rs @@ -8,6 +8,10 @@ use anyhow::Result; use perry_hir::Expr; use crate::nanbox::double_literal; +use crate::rooting::{ + any_operand_may_collect, with_operands_rooted, with_rooted_accumulator, with_rooted_group, Arg, + Repr, +}; use crate::types::{DOUBLE, I32, I64, PTR}; use super::{emit_root_nanbox_store_on_block, lower_expr, nanbox_pointer_inline, FnCtx}; @@ -504,118 +508,147 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { || !captured_args.is_empty() || !symbol_statics.is_empty() || !block_fns.is_empty(); - let rooted = super::temp_root::rooted_handle_begin(ctx, &obj, protect_handle); - for (name, init) in named_statics { - let key_idx = ctx.strings.intern(name); - let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); - let v = lower_expr(ctx, init)?; - let obj = super::temp_root::rooted_handle_get(ctx, &rooted); - let blk = ctx.block(); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, crate::nanbox::POINTER_MASK_I64); - blk.call_void( - "js_object_set_field_by_name", - &[(I64, &obj), (I64, &key_raw), (DOUBLE, &v)], - ); - } - // #1787: snapshot the captured outer-scope values onto the class - // object as the `__perry_ctor_caps` own array (in the constructor's - // capture-param order). `new ()` reads it back - // in `js_new_function_construct` and replays the constructor with the - // right captured environment — which the static `new ClassName()` - // inlining can't do once the class escapes its defining scope. - if !captured_args.is_empty() { - let cap_len = captured_args.len().to_string(); - let mut caps_arr = ctx.block().call(I64, "js_array_alloc", &[(I32, &cap_len)]); - // #6523: these capture loads are Perry-internal materialization - // at the class's DEFINITION site, same as the - // `RegisterClassCaptures` snapshot loads above (#6052). A - // captured `const` declared AFTER the class (bundled semver's - // `class Comparator` + trailing debug/require consts) is still - // in its dead zone here — legal JS, since TDZ applies at - // method-call time. Without the suppression window the checked - // box read threw "Cannot access undefined before - // initialization" while merely DEFINING the class. Suppressed - // loads snapshot `undefined`; the #6037 refresh statements - // re-register the live values right after each captured - // refresh the evaluated object's array right after each - // captured binding's initializer runs. - ctx.block().call_void("js_tdz_suppress_begin", &[]); - for arg in captured_args { - let v = lower_expr(ctx, arg)?; - caps_arr = ctx.block().call( - I64, - "js_array_push_f64", - &[(I64, &caps_arr), (DOUBLE, &v)], + with_rooted_group(ctx, 1, |ctx, group| { + let rooted = group.adopt_emitted(ctx, Repr::Ptr, &obj, protect_handle); + for (name, init) in named_statics { + let key_idx = ctx.strings.intern(name); + let key_handle_global = + format!("@{}", ctx.strings.entry(key_idx).handle_global); + let v = lower_expr(ctx, init)?; + let obj = group.reread_emitted(ctx, rooted); + let blk = ctx.block(); + let key_box = blk.load(DOUBLE, &key_handle_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + let key_raw = blk.and(I64, &key_bits, crate::nanbox::POINTER_MASK_I64); + blk.call_void( + "js_object_set_field_by_name", + &[(I64, &obj), (I64, &key_raw), (DOUBLE, &v)], ); } - ctx.block().call_void("js_tdz_suppress_end", &[]); - let caps_box = nanbox_pointer_inline(ctx.block(), &caps_arr); - let key_idx = ctx.strings.intern("__perry_ctor_caps"); - let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); - // #7154: re-read the class object — the capture lowerings above - // are arbitrary expressions and may have moved it. - let obj = super::temp_root::rooted_handle_get(ctx, &rooted); - let blk = ctx.block(); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, crate::nanbox::POINTER_MASK_I64); - blk.call_void( - "js_object_set_field_by_name", - &[(I64, &obj), (I64, &key_raw), (DOUBLE, &caps_box)], - ); - } - for (key, init) in symbol_statics { - let k = lower_expr(ctx, key)?; - // #7154: `key` is lowered before `init`, so the Symbol sits in - // an SSA register across an arbitrary initializer — the same - // exposure the receiver has, one operand over. Root it. - let key_guard = super::temp_root::guard_store_operand(ctx, key, &k, init); - let v = lower_expr(ctx, init)?; - let k = super::temp_root::reread_store_operand(ctx, &key_guard, key, &k)?; - // #7154: both lowerings above can collect; re-derive the - // receiver from the root rather than reusing the register. - let obj = super::temp_root::rooted_handle_get(ctx, &rooted); - let obj_box = nanbox_pointer_inline(ctx.block(), &obj); - ctx.block().call( - DOUBLE, - "js_object_set_symbol_property", - &[(DOUBLE, &obj_box), (DOUBLE, &k), (DOUBLE, &v)], - ); - // Cut per iteration rather than letting `rooted`'s release do it - // at the end: the setter this call may invoke is user code, and - // N statics would otherwise hold N slots across all of them. - super::temp_root::release_store_operand(ctx, key_guard); - } - // #685: run the class's `static { … }` blocks NOW — at the class - // expression's evaluation, with `this` = THIS fresh class object. - // The `ClassExprFresh` fast path previously never invoked them - // (they are also skipped by the module-init fallback when another - // evaluation site invokes them inline), so `return class { static - // { this.viaBlock = tag } }` factories produced objects whose - // blocks simply never ran. Arm the one-shot static-`this` - // override before each call so the compiled body's - // `js_static_this_resolve` prologue binds `this` to the fresh - // object (writes land as own properties of this evaluation's - // object, not the shared template). Blocks run after the named - // static fields above — the source interleaving of fields and - // blocks is not reproduced on this path (pre-existing limitation). - // - // `block_fns` is computed above, next to `protect_handle`. - for fn_name in block_fns { - // #7154: a static block runs arbitrary user code, so re-derive - // the receiver from the root before each one. - let obj = super::temp_root::rooted_handle_get(ctx, &rooted); + // #1787: snapshot the captured outer-scope values onto the class + // object as the `__perry_ctor_caps` own array (in the constructor's + // capture-param order). `new ()` reads it back + // in `js_new_function_construct` and replays the constructor with the + // right captured environment — which the static `new ClassName()` + // inlining can't do once the class escapes its defining scope. + if !captured_args.is_empty() { + let cap_len = captured_args.len().to_string(); + let caps_arr = ctx.block().call(I64, "js_array_alloc", &[(I32, &cap_len)]); + // #7615 slice 8: the capture snapshot is an ACCUMULATOR — the + // array holds the only reference to everything pushed so far + // while the next element is lowered — and it was threaded + // through a bare SSA register, which is #6951's shape exactly. + // + // The window is EMPTY on today's HIR, and the flag says so + // rather than the code assuming it: `captured_args` is built at + // exactly one site (`lower/lower_expr/arm_class.rs`) as + // `ids.iter().map(|id| Expr::LocalGet(*id))`, and + // `expr_may_trigger_gc` answers `false` for every `LocalGet`. + // So `protect_caps` is false today, `advance` threads the same + // register the old code threaded, and the emitted IR is byte + // for byte what it was. What changes is that the day a + // non-inert expression reaches this list it is rooted by + // construction instead of silently entering the window. + let protect_caps = any_operand_may_collect(ctx, captured_args.iter()); + // #6523: these capture loads are Perry-internal materialization + // at the class's DEFINITION site, same as the + // `RegisterClassCaptures` snapshot loads above (#6052). A + // captured `const` declared AFTER the class (bundled semver's + // `class Comparator` + trailing debug/require consts) is still + // in its dead zone here — legal JS, since TDZ applies at + // method-call time. Without the suppression window the checked + // box read threw "Cannot access undefined before + // initialization" while merely DEFINING the class. Suppressed + // loads snapshot `undefined`; the #6037 refresh statements + // re-register the live values right after each captured + // refresh the evaluated object's array right after each + // captured binding's initializer runs. + let caps_box = with_rooted_accumulator( + ctx, + Repr::Ptr, + &caps_arr, + protect_caps, + |ctx, acc| { + ctx.block().call_void("js_tdz_suppress_begin", &[]); + for arg in captured_args { + let v = lower_expr(ctx, arg)?; + acc.advance(ctx, "js_array_push_f64", &[Arg::Plain(DOUBLE, &v)]); + } + ctx.block().call_void("js_tdz_suppress_end", &[]); + Ok(()) + }, + |ctx, arr| Ok(nanbox_pointer_inline(ctx.block(), arr)), + )?; + let key_idx = ctx.strings.intern("__perry_ctor_caps"); + let key_handle_global = + format!("@{}", ctx.strings.entry(key_idx).handle_global); + // #7154: re-read the class object — the capture lowerings above + // are arbitrary expressions and may have moved it. + let obj = group.reread_emitted(ctx, rooted); + let blk = ctx.block(); + let key_box = blk.load(DOUBLE, &key_handle_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + let key_raw = blk.and(I64, &key_bits, crate::nanbox::POINTER_MASK_I64); + blk.call_void( + "js_object_set_field_by_name", + &[(I64, &obj), (I64, &key_raw), (DOUBLE, &caps_box)], + ); + } + for (key, init) in symbol_statics { + // #7154: `key` is lowered before `init`, so the Symbol sits in + // an SSA register across an arbitrary initializer — the same + // exposure the receiver has, one operand over. Root it. + // + // The inner scope is cut per iteration rather than by the + // group's own release: the setter this call may invoke is user + // code, and N statics would otherwise hold N slots across all + // of them. A release is a stack CUT, so the inner scope drops + // only what it pushed above the class object. + with_operands_rooted(ctx, &[key, init], |ctx, values| { + // #7154: both lowerings above can collect; re-derive the + // receiver from the root rather than reusing the register. + let obj = group.reread_emitted(ctx, rooted); + let obj_box = nanbox_pointer_inline(ctx.block(), &obj); + ctx.block().call( + DOUBLE, + "js_object_set_symbol_property", + &[ + (DOUBLE, &obj_box), + (DOUBLE, &values[0]), + (DOUBLE, &values[1]), + ], + ); + Ok(()) + })?; + } + // #685: run the class's `static { … }` blocks NOW — at the class + // expression's evaluation, with `this` = THIS fresh class object. + // The `ClassExprFresh` fast path previously never invoked them + // (they are also skipped by the module-init fallback when another + // evaluation site invokes them inline), so `return class { static + // { this.viaBlock = tag } }` factories produced objects whose + // blocks simply never ran. Arm the one-shot static-`this` + // override before each call so the compiled body's + // `js_static_this_resolve` prologue binds `this` to the fresh + // object (writes land as own properties of this evaluation's + // object, not the shared template). Blocks run after the named + // static fields above — the source interleaving of fields and + // blocks is not reproduced on this path (pre-existing limitation). + // + // `block_fns` is computed above, next to `protect_handle`. + for fn_name in block_fns { + // #7154: a static block runs arbitrary user code, so re-derive + // the receiver from the root before each one. + let obj = group.reread_emitted(ctx, rooted); + let obj_box = nanbox_pointer_inline(ctx.block(), &obj); + ctx.block() + .call_void("js_static_this_arm_value", &[(DOUBLE, &obj_box)]); + ctx.block().call(DOUBLE, &fn_name, &[]); + } + let obj = group.reread_emitted(ctx, rooted); let obj_box = nanbox_pointer_inline(ctx.block(), &obj); - ctx.block() - .call_void("js_static_this_arm_value", &[(DOUBLE, &obj_box)]); - ctx.block().call(DOUBLE, &fn_name, &[]); - } - let obj = super::temp_root::rooted_handle_get(ctx, &rooted); - let obj_box = nanbox_pointer_inline(ctx.block(), &obj); - super::temp_root::rooted_handle_release(ctx, rooted); - Ok(obj_box) + Ok(obj_box) + }) } // Issue #711 part 2: `.prototype = ` pattern. // Calls `js_set_function_prototype(func, proto)`, which (when diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 0306c1b9c5..88db6ad4c3 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -24,6 +24,7 @@ pub(crate) mod loop_purity; pub(crate) mod lower_array_method; pub(crate) mod lower_call; pub(crate) mod lower_conditional; +pub(crate) mod lower_string_concat; pub(crate) mod lower_string_method; pub mod module; pub mod nanbox; diff --git a/crates/perry-codegen/src/loop_purity.rs b/crates/perry-codegen/src/loop_purity.rs index 5fdc25a335..1212d817d9 100644 --- a/crates/perry-codegen/src/loop_purity.rs +++ b/crates/perry-codegen/src/loop_purity.rs @@ -67,7 +67,7 @@ pub(crate) fn body_needs_asm_barrier(body: &[Stmt]) -> bool { /// /// `is_inert` answers "can evaluating this expression run user code or /// allocate?" for the coercing operators. In production it is -/// [`crate::expr::temp_root::expr_is_inert_primitive`] — the predicate #6975 +/// [`crate::rooting::expr_is_inert_primitive`] — the predicate #6975 /// introduced for the argument-rooting decision, because it answers exactly /// this question and two copies of it would drift. It is injected rather than /// called directly so this module stays free of `FnCtx` and both directions of diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index 9103273921..4c4b967f5a 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -56,6 +56,7 @@ mod native; mod native_module_dispatch; mod native_table; mod new; +mod new_alloc; mod new_ctor_args; mod new_helpers; mod omitted_native_params; diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index 1e54795344..fc5615dcda 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -20,9 +20,10 @@ use super::new_helpers::{ ctor_body_has_value_return, ctor_body_uses_this, ctor_chain_uses_new_target, emit_promise_subclass_init, local_constructor_symbol_exists, node_stream_parent_kind, }; -use crate::expr::{lower_expr, lower_js_args_array, nanbox_pointer_inline, temp_root, FnCtx}; +use crate::expr::{lower_expr, lower_js_args_array, nanbox_pointer_inline, FnCtx}; use crate::nanbox::{double_literal, POINTER_MASK_I64}; -use crate::types::{DOUBLE, I32, I64, I8, PTR}; +use crate::rooting::{open_rooted_group, EmittedValue, Repr, RootedGroup}; +use crate::types::{DOUBLE, I32, I64, PTR}; /// Does `new (…)` run user code — an own or inherited constructor /// body, or field initializers — between the instance allocation and the value @@ -46,7 +47,7 @@ fn construction_runs_user_code(ctx: &FnCtx<'_>, class_name: &str) -> bool { // `ctx.imported_class_ctors[class_name]` (its `has_imported_ctor` arm, and // the `Stmt::Return` writer at the tail of this file). That left BOTH // consumers of this predicate unprotected across a real constructor body: - // #7192's `instance_root`, and the `this`-slot bind added for #7202. + // #7192's instance root, and the `this`-slot bind added for #7202. // // Keeping it ONE predicate rather than two is the point — the consumers // have to agree by construction, which is what stops the divergence @@ -73,47 +74,34 @@ fn construction_runs_user_code(ctx: &FnCtx<'_>, class_name: &str) -> bool { /// keep their old IR byte for byte. fn reload_instance( ctx: &mut FnCtx<'_>, - instance_root: &Option, + group: &RootedGroup<'_>, + instance: &Instance, obj_handle: &str, obj_box: &str, ) -> (String, String) { - let Some(idx) = instance_root.clone() else { + if !instance.protected { return (obj_handle.to_string(), obj_box.to_string()); - }; - let handle = temp_root::temp_root_get_i64(ctx, &idx); + } + let handle = group.reread_emitted(ctx, instance.root); let boxed = nanbox_pointer_inline(ctx.block(), &handle); (handle, boxed) } -pub(crate) use super::capture_writeback::emit_class_capture_writeback; -use super::typed_shape_init::{emit_typed_shape_layout_declare, emit_typed_shape_layout_init}; - -/// #7469: is the `new` site being lowered inside a **loop body**? -/// -/// This is the gate on the inline bump allocator. Inlining removes the -/// `js_object_alloc_class_inline_keys` FFI call and, with it, the thread-local -/// resolutions that call performs — measured 1.81× on `churn_alloc`, 1.78× on -/// `push_cls`. But it costs ~268 bytes of machine code **per site** (measured -/// over 10 / 200 / 800-site programs: +0, +49,536, +214,656 bytes), so it -/// cannot be the unconditional default against this repo's binary-size -/// campaign — a 5,000-site application would pay ~1.3 MB. +/// The freshly-allocated instance's place in the `new` scope. /// -/// Loop membership is the cheapest sound proxy for "this site runs many -/// times", and it bounds the size cost to loop bodies. A `new` executed once -/// keeps the outlined call and contributes nothing to binary growth. -/// -/// `loop_targets` is reused rather than a new counter added, because it is -/// already maintained by all three loop lowerings. It also carries `switch` -/// frames, which push an EMPTY continue label (`switch_stmt.rs`) while every -/// loop pushes a real one — the same discriminator `Stmt::Continue`'s -/// scan-outward-past-switch-frames logic uses. A `new` inside a bare `switch` -/// is therefore correctly treated as not-in-a-loop. -fn new_site_is_in_loop(ctx: &FnCtx<'_>) -> bool { - ctx.loop_targets - .iter() - .any(|(continue_label, _, _)| !continue_label.is_empty()) +/// `protected` is `construction_runs_user_code`, taken ONCE. It gates three +/// things that must agree — the temp root, the `this`-slot bind (#7202), and +/// whether [`reload_instance`] re-reads at all — and the reason it is a field +/// rather than three calls is the same reason `construction_runs_user_code` is +/// one predicate rather than two: a fork here is how #7114's pair diverged. +struct Instance { + root: EmittedValue, + protected: bool, } +pub(crate) use super::capture_writeback::emit_class_capture_writeback; +use super::typed_shape_init::{emit_typed_shape_layout_declare, emit_typed_shape_layout_init}; + /// Lower `new ClassName(args…)` — Phase C.1. /// /// Strategy: allocate an anonymous object via `js_object_alloc(0, N)` @@ -174,8 +162,8 @@ pub(crate) fn lower_new_member_captured( /// *mutable* root that an evacuating cycle rewrites in place, leaving the /// register pushed beforehand stale; /// - an argument that was NOT rooted because it reads an *immutable* registered -/// root — a string literal, the only `temp_root::operand_is_reloadable` case -/// — is **re-loaded**. It is never swept, but evacuation rewrote its handle +/// root — a string literal, the only `operand_is_reloadable` case — is +/// **re-loaded**. It is never swept, but evacuation rewrote its handle /// global too, so the cached register points at where the string used to be. /// Re-lowering emits the load again and costs no runtime call. (A /// shadow-slotted local or a module global is a registered root as well, but @@ -185,25 +173,21 @@ pub(crate) fn lower_new_member_captured( /// Called after the instance allocation and again before the late consumers /// that sit behind further arbitrary lowering (field initializers, an inlined /// constructor body) — each of those is another chance to relocate. -fn refresh_rooted_args( - ctx: &mut FnCtx<'_>, - args: &[Expr], - lowered_args: &mut [String], - arg_roots: &[Option], -) -> Result<()> { - for (i, (value, slot)) in lowered_args.iter_mut().zip(arg_roots.iter()).enumerate() { - match slot { - Some(idx) => { - let idx = idx.clone(); - *value = temp_root::temp_root_get_double(ctx, &idx); - } - None if temp_root::operand_is_reloadable(&args[i]) => { - *value = lower_constructor_arg(ctx, &args[i])?; - } - None => {} - } - } - Ok(()) +fn refresh_rooted_args(ctx: &mut FnCtx<'_>, group: &RootedGroup<'_>) -> Result> { + // `RootedGroup`'s re-read re-lowers a `Reload` operand through + // `crate::expr::lower_expr`, while the ORIGINAL lowering of every + // constructor argument went through `lower_constructor_arg` — which is + // `lower_expr` with `discard_expr_value` forced false (#7590: the flag + // means "this STATEMENT's value is discarded" and is not cleared on + // recursion). Re-lowering under a different flag would be free to pick + // `materialize_js_value_without_record`, so the re-read is wrapped in the + // same suppression the first lowering had. A no-op for `Root` and `Reuse` + // operands, which emit no lowering at all. + let prev_discard = ctx.discard_expr_value; + ctx.discard_expr_value = false; + let out = group.reread_all(ctx); + ctx.discard_expr_value = prev_discard; + out } fn lower_new_impl( @@ -212,30 +196,35 @@ fn lower_new_impl( args: &[Expr], caps_absent_from_args: bool, ) -> Result { - // #6969: expression-scope temp-root barrier. The body below roots its + // #6969: one expression-scope temp-root barrier. The body below roots its // constructor arguments across the instance allocation, and it has ~20 // return paths with `lowered_args` consumed at a dozen of them — one cut - // here releases the group whichever path ran, instead of a - // `temp_root_release` at each that reviewers and future edits must keep - // balanced. + // here releases the group whichever path ran, instead of a release at each + // that reviewers and future edits must keep balanced. + // + // #7615 slice 8: this is [`open_rooted_group`], and the escaping form is + // the right one for exactly the reason its doc names — the release has to + // post-dominate every one of those return paths, which no closure form can + // own without swallowing the whole 1,000-line dispatch. // - // #7154: the body also roots the freshly-allocated instance across the - // constructor body, so the marker is required whenever construction runs - // user code — not only when an argument needs a root. `new C()` with no - // arguments is exactly the shape that would otherwise push a slot with no - // marker above it to cut. - let scope = - temp_root::temp_root_scope_begin(ctx, args, construction_runs_user_code(ctx, class_name)); - let result = lower_new_impl_inner(ctx, class_name, args, caps_absent_from_args); - temp_root::temp_root_scope_end(ctx, scope); + // The null MARKER slot `temp_root_scope_begin` used to push is gone with + // it. It existed only because a raw truncate needs a base index even when + // nothing else was pushed; `RootedGroup::release` truncates at the group's + // own lowest slot and emits nothing at all when the group is empty, so the + // marker has no work left to do. One fewer slot and one fewer push per + // `new` site that roots anything. + let mut group = open_rooted_group(args.len() + 1); + let result = lower_new_impl_inner(ctx, class_name, args, caps_absent_from_args, &mut group); + group.release(ctx); result } -fn lower_new_impl_inner( +fn lower_new_impl_inner<'a>( ctx: &mut FnCtx<'_>, class_name: &str, - args: &[Expr], + args: &'a [Expr], caps_absent_from_args: bool, + group: &mut RootedGroup<'a>, ) -> Result { // Built-in Web classes that the runtime provides constructors for. // These are checked BEFORE the ctx.classes lookup because the user @@ -434,478 +423,19 @@ fn lower_new_impl_inner( // collects; the re-read is immediately after it (see `obj_box`), and the // scope cut in `lower_new_impl` is the release. let mut lowered_args: Vec = Vec::with_capacity(args.len()); - let mut arg_roots: Vec> = Vec::with_capacity(args.len()); for a in args { let value = lower_constructor_arg(ctx, a)?; - let slot = if temp_root::operand_needs_root(ctx, a) { - Some(temp_root::temp_root_push_double(ctx, &value)) - } else { - None - }; + // `collects` is unconditionally true: the instance allocation below + // always collects, so every argument is live across it. That is the + // same answer the pre-migration code gave by consulting + // `operand_needs_root` with no window test at all. + group.adopt(ctx, a, &value, true); lowered_args.push(value); - arg_roots.push(slot); } - // Compute total field count including inherited parent fields. - // The runtime allocates at least 8 inline slots regardless, so this - // mostly matters for shapes >8 fields. - let mut field_count = class.fields.len() as u32; - // Imported classes now carry their real field_names from the source - // module. If the field count is still 0 (no fields info available), - // use a generous default as a safety net. - if field_count == 0 && class.constructor.is_none() { - field_count = 32; - } - let mut parent = class.extends_name.as_deref(); - while let Some(parent_name) = parent { - if let Some(p) = ctx.classes.get(parent_name).copied() { - field_count += p.fields.len() as u32; - parent = p.extends_name.as_deref(); - } else { - break; - } - } - // Issue #26 / #321: prefer the authoritative per-class field count computed - // by the source-prefix-disambiguated keys-global builder. The walk above - // resolves parents via `ctx.classes` — a name-keyed map that holds only - // ONE same-named stub — so when a cross-module parent name collides - // (effect's `Type` in SchemaAST.ts vs ParseResult.ts) it counts the wrong - // parent's fields. Using the keys-global's count keeps the allocated slot - // count and the header `field_count` in lockstep with the keys array, - // which `Object.keys()` walks. Falls back to the computed walk when this - // class has no keys global (anonymous / no-keys path). - if let Some(&authoritative) = ctx.class_field_counts.get(class_name) { - field_count = authoritative; - } - // #6812 (w16): a per-site empty-literal anon-shape class may carry a - // compile-time proven builder width. Allocate that many inline slots so - // the FIRST instance of the site is as wide as the runtime-learned - // resizes make every later one — a lone under-sized first instance - // permanently vetoes whole-loop clone eligibility for arrays built at - // the site. Capacity only: the keys array stays authoritative for - // enumeration, and the runtime treats header field_count as alloc_limit. - if class.alloc_width_hint > field_count { - field_count = class.alloc_width_hint; - } - - // Allocate the object with the per-class id and (if applicable) - // parent class id, so the runtime registers the inheritance - // chain for instanceof / virtual dispatch lookups. - // - // Use `js_object_alloc_class_with_keys`, which pre-populates the - // `keys_array` with the class's field names in declaration order - // (parent fields first, walking from the deepest ancestor down, - // then own fields). This is REQUIRED so the LLVM PropertyGet/Set - // fast path's slot indices match the runtime's by-name dispatch - // (which walks `keys_array`). Mixing the two access patterns on - // the same object — e.g. constructor writes via the fast path, - // PropertyUpdate reads via the runtime helper — only produces - // consistent results when both agree on the slot mapping. - // - // The packed-keys constant is interned via the StringPool. Two - // classes with the same field-name set + order share one constant. - let cid = ctx.class_ids.get(class_name).copied().unwrap_or(0); - let parent_cid = class - .extends_name - .as_deref() - .and_then(|p| ctx.class_ids.get(p).copied()) - .unwrap_or(0); - let cid_str = cid.to_string(); - let parent_cid_str = parent_cid.to_string(); - let n_str = field_count.to_string(); - - // Fast path: if the class has a per-class keys global (built once - // at module init via `js_build_class_keys_array`), emit INLINE - // bump-allocator IR — no function call into the runtime at all on - // the hot path. The runtime exposes a `InlineArenaState` struct - // (data ptr at offset 0, current bump offset at offset 8, current - // block size at offset 16) via `js_inline_arena_state()`. We call - // that ONCE per JS function entry (cached in `arena_state_slot`) - // and then emit a 5-instruction bump check + GcHeader/ObjectHeader - // store sequence at every `new ClassName()` site. The slow path - // (block overflow) calls `js_inline_arena_slow_alloc` which syncs - // the inline state back to the underlying arena, allocates a new - // block, and updates the inline state. - // - // Cycles per inlined alloc on the M-series fast path: - // load offset (1) - // add+and align (2) - // add new_offset (1) - // load size + cmp (2) - // cond br (predicted, 0) - // store offset (1) - // load data + gep (2) - // write GcHeader (1) — packed i64 store - // write ObjectHeader×2 (2) — packed i64 stores - // write keys_ptr (1) - // total: ~13 cycles vs ~140 cycles for the function-call path. - // - // Layout assumption: GcHeader is 8 bytes - // {obj_type:u8, gc_flags:u8, _reserved:u16, size:u32} - // and ObjectHeader is 24 bytes - // {object_type:u32, class_id:u32, parent_class_id:u32, - // field_count:u32, keys_array:*ptr} - // followed by `max(field_count, 8)` 8-byte field slots. The user - // pointer the rest of the codegen sees is `raw + 8` (i.e. the - // ObjectHeader address) — same as what - // `js_object_alloc_class_inline_keys` returns. - // - // Layout constants are duplicated here from the runtime; if - // `GcHeader` or `ObjectHeader` ever change in - // `crates/perry-runtime/src/{gc,object}.rs`, update both sides. - let obj_handle = if class.extends_expr.is_some() { - // Wall 45: dynamic-parent subclass (`class X extends _mod.default`). - // The parent's field layout is unknown at this compile time (the - // `extends` target is an unresolvable cross-module value, so the - // parent-chain walk above contributed 0 fields and `field_count` / - // `packed_keys` cover only X's OWN fields). Allocating with that - // own-only layout under-sizes and mis-lays-out the instance: the - // parent's constructor and inherited methods address the inherited - // fields at the PARENT's slot indices (parent fields first), which fall - // past X's own slots → OOB heap reads (captures read as garbage). - // Route to `js_object_alloc_class_dynamic_parent`, which resolves the - // runtime-registered parent edge + keys-array (both established at - // module init by `js_register_class_parent_dynamic` / - // `js_build_class_keys_array`, before any `new X()`) and allocates with - // the merged `[parent keys..] ++ [own keys..]` layout. Bypasses the - // inline bump-alloc fast path (which would bake the wrong layout). - let mut packed_keys = String::new(); - for f in &class.fields { - if f.key_expr.is_some() { - continue; - } - packed_keys.push_str(&f.name); - packed_keys.push('\0'); - } - let keys_idx = ctx.strings.intern(&packed_keys); - let keys_entry = ctx.strings.entry(keys_idx); - let keys_global = format!("@{}", keys_entry.bytes_global); - let keys_len_str = keys_entry.byte_len.to_string(); - ctx.block().call( - I64, - "js_object_alloc_class_dynamic_parent", - &[ - (I32, &cid_str), - (I32, &n_str), - (PTR, &keys_global), - (I32, &keys_len_str), - ], - ) - } else if let Some(keys_global_name) = ctx.class_keys_globals.get(class_name).cloned() { - // [#bloat] Outline the per-`new`-site allocator EXCEPT inside a loop. - // - // Outlining collapses ~145 lines of per-class-constant IR per site into - // a single `js_object_alloc_class_inline_keys` call (~3 lines), which - // performs the identical bump alloc + header init + slot zero-fill and - // returns the same user pointer. That is the right default for code - // size, and the size half of the original measurement still holds: - // ~268 bytes of machine code per site, +214,656 bytes over an 800-site - // program. - // - // The SPEED half of that measurement has since inverted. It read - // "~17% faster on an 8M-allocation loop (the inline bump bloated the - // hot loop, hurting icache/regalloc more than the saved call)"; today - // the outlined form is **1.81× SLOWER** on `churn_alloc` and 1.78× on - // `push_cls`. Nothing about the inline bump changed — everything - // *around* the allocation got cheaper (#7474, #7486, #7487, #7501, - // #7525, #7532, #7535, #7536, #7552), so the surviving FFI call and - // the thread-local resolutions it performs now dominate what the - // inline bump's code bloat costs. On Darwin those resolutions cannot - // be made cheaper — Mach-O has no local-exec TLS model, and building - // the runtime with `-Ztls-model=local-exec` leaves the `blr` through - // the TLV descriptor byte-identical (measured: 1.02×). Only their - // COUNT can be reduced, and inlining removes them outright. - // - // So the choice is per site, not global: a `new` inside a loop takes - // the inline bump (it runs many times, and the size cost is bounded to - // loop bodies); everything else keeps the outlined call and - // contributes nothing to binary growth. `PERRY_INLINE_NEW=1` still - // forces the inline form everywhere, for A/B measurement. - // - // NOTE the env test is `is_none()`: `PERRY_INLINE_NEW=""` *enables* - // the inline path, because an empty string is `Some("")`. - let force_inline_new = std::env::var_os("PERRY_INLINE_NEW").is_some(); - if !force_inline_new && !new_site_is_in_loop(ctx) { - let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { - s - } else { - let s = ctx.func.entry_init_load_global(&keys_global_name, I64); - ctx.class_keys_slots - .insert(class_name.to_string(), s.clone()); - s - }; - let keys_ptr = ctx.block().load(I64, &keys_slot); - ctx.pending_declares.push(( - "js_object_alloc_class_inline_keys".to_string(), - I64, - vec![I32, I32, I32, I64], - )); - ctx.block().call( - I64, - "js_object_alloc_class_inline_keys", - &[ - (I32, &cid_str), - (I32, &parent_cid_str), - (I32, &field_count.to_string()), - (I64, &keys_ptr), - ], - ) - } else { - // Compile-time layout constants. - const GC_HEADER_SIZE: u64 = 8; - // arm64_32 watchOS: `size_of::()` is 24 on 64-bit but - // 20 on ILP32 (4-byte `keys_array` pointer). Derive from the target - // triple so the inline alloc size and field-region base match the - // target-compiled runtime (no-op on 64-bit; see `target_layout`). - let object_header_size: u64 = - crate::target_layout::object_header_size_bytes(ctx.target_triple); - // #6759 Phase B: pointer width for the trailing `meta` header - // field (computed here, before `ctx.block()` mutably borrows). - let meta_ptr_size: u64 = if crate::target_layout::target_is_ilp32(ctx.target_triple) { - 4 - } else { - 8 - }; - const FIELD_SLOT_SIZE: u64 = 8; - // Inline-slot floor — MUST match perry-runtime `object::INLINE_SLOT_FLOOR` - // (they independently pad `new` objects to the same minimum; a mismatch - // where codegen allocs fewer slots than the runtime's get/set bound-check - // assumes is heap corruption). Lowered 8->4 to shrink small-object footprint. - const MIN_FIELD_SLOTS: u64 = 4; - const GC_TYPE_OBJECT: u64 = 2; - const GC_FLAG_ARENA: u64 = 0x02; - // PR #1146: pointer-free hint for inline-allocated regular - // objects. The field-store sites issue per-slot - // `js_gc_note_slot_layout` so the GC sees real pointer-bearing - // slots regardless of this initial tag. - const GC_LAYOUT_POINTER_FREE: u64 = 0x4000; - const OBJECT_TYPE_REGULAR: u64 = 1; - - let alloc_field_count = std::cmp::max(field_count as u64, MIN_FIELD_SLOTS); - let payload_size = object_header_size + alloc_field_count * FIELD_SLOT_SIZE; - // Round the whole allocation up to FIELD_SLOT_SIZE (8). The inline - // bump allocator's offset invariant (below) requires every - // allocation to be a multiple of 8; on ILP32 `object_header_size` - // is 20, so an unpadded total is 4-skewed (e.g. 92) and would - // misalign the next bump. No-op on 64-bit (8 + 24 + 8·n is already - // 8-aligned → 96 for ≤8 fields). - let total_size = (GC_HEADER_SIZE + payload_size).next_multiple_of(FIELD_SLOT_SIZE); - let total_size_str = total_size.to_string(); - - // Lazy: allocate the per-function arena-state slot on the - // first `new` we see. The slot init (`call @js_inline_arena_state` - // + store) lives in the entry block via `entry_init_call_ptr`, - // so it dominates every reachable use. - let arena_state_slot = if let Some(slot) = ctx.arena_state_slot.clone() { - slot - } else { - let slot = ctx.func.entry_init_call_ptr("js_inline_arena_state"); - ctx.arena_state_slot = Some(slot.clone()); - slot - }; - - // Hoist the per-class `keys_array` global load to the function - // entry block (cached in a stack slot per class). Without this - // hoisting, LLVM would reload `@perry_class_keys_` on - // every loop iteration, because the loop body's `call - // @js_inline_arena_slow_alloc` blocks LICM — LLVM can't prove - // the call doesn't modify the global. - let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { - s - } else { - let s = ctx.func.entry_init_load_global(&keys_global_name, I64); - ctx.class_keys_slots - .insert(class_name.to_string(), s.clone()); - s - }; - let keys_ptr = ctx.block().load(I64, &keys_slot); - - // Inline bump-allocator IR. - let blk = ctx.block(); - let state_ptr = blk.load(PTR, &arena_state_slot); - - // offset = state.offset (at byte offset 8 in InlineArenaState). - // The offset is invariant 8-aligned: arena blocks start at offset 0 - // (8-aligned), every allocation is a multiple of 8 (`total_size` - // includes the 8-byte GcHeader and `MIN_FIELD_SLOTS=4` slots × - // 8 bytes), and `js_inline_arena_slow_alloc` only ever swings the - // state to `block.offset` which is also always 8-aligned. So we - // skip the `(offset + 7) & -8` align-up step entirely — saves - // 2 instructions per iter on the hot path. - let offset_field_ptr = blk.gep(I8, &state_ptr, &[(I64, "8")]); - let offset_val = blk.load(I64, &offset_field_ptr); - let aligned_off = offset_val.clone(); - - // new_offset = aligned + total_size - let new_offset = blk.add(I64, &aligned_off, &total_size_str); - - // size = state.size (at byte offset 16) - let size_field_ptr = blk.gep(I8, &state_ptr, &[(I64, "16")]); - let size_val = blk.load(I64, &size_field_ptr); - - // fits = new_offset <= size - let fits = blk.icmp_ule(I64, &new_offset, &size_val); - - // Set up fast/slow/merge basic blocks. - let fast_idx = ctx.new_block("alloc.fast"); - let slow_idx = ctx.new_block("alloc.slow"); - let merge_idx = ctx.new_block("alloc.merge"); - let fast_label = ctx.block_label(fast_idx); - let slow_label = ctx.block_label(slow_idx); - let merge_label = ctx.block_label(merge_idx); - - ctx.block().cond_br(&fits, &fast_label, &slow_label); - - // ---- Fast path: bump and return data + aligned ---- - ctx.current_block = fast_idx; - let blk = ctx.block(); - // GC_STORE_AUDIT(INIT): inline arena bump offset is allocator metadata, not a JS heap edge. - blk.store(I64, &new_offset, &offset_field_ptr); - // data ptr is at byte offset 0 in InlineArenaState - let data_ptr = blk.load(PTR, &state_ptr); - let raw_fast = blk.gep(I8, &data_ptr, &[(I64, &aligned_off)]); - let fast_pred_label = blk.label.clone(); - blk.br(&merge_label); - - // ---- Slow path: call into the runtime ---- - ctx.current_block = slow_idx; - let raw_slow = ctx.block().call( - PTR, - "js_inline_arena_slow_alloc", - &[(PTR, &state_ptr), (I64, &total_size_str), (I64, "8")], - ); - let slow_pred_label = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - // ---- Merge: phi the raw pointer, write headers, NaN-box ---- - ctx.current_block = merge_idx; - let blk = ctx.block(); - let raw = blk.phi( - PTR, - &[(&raw_fast, &fast_pred_label), (&raw_slow, &slow_pred_label)], - ); - - // Write GcHeader (8 bytes) as a single i64 store. Field - // packing (little-endian): - // bits 0..7 = obj_type (u8) - // bits 8..15 = gc_flags (u8) - // bits 16..31 = _reserved (u16) - // bits 32..63 = size (u32) - let gc_packed: u64 = GC_TYPE_OBJECT - | (GC_FLAG_ARENA << 8) - | (GC_LAYOUT_POINTER_FREE << 16) - | ((total_size as u64) << 32); - // GC_STORE_AUDIT(INIT): inline headers initialize freshly allocated unpublished object storage. - blk.store(I64, &gc_packed.to_string(), &raw); - - // Write ObjectHeader at raw + 8. - // First 8 bytes: object_type (u32, low) | class_id (u32, high) - let oh_addr_1 = blk.gep(I8, &raw, &[(I64, "8")]); - let oh_word_1: u64 = OBJECT_TYPE_REGULAR | ((cid as u64) << 32); - blk.store(I64, &oh_word_1.to_string(), &oh_addr_1); - - // Second 8 bytes: parent_class_id (u32, low) | field_count (u32, high) - let oh_addr_2 = blk.gep(I8, &raw, &[(I64, "16")]); - let oh_word_2: u64 = (parent_cid as u64) | ((field_count as u64) << 32); - blk.store(I64, &oh_word_2.to_string(), &oh_addr_2); - - // Third 8 bytes: keys_array pointer. The keys_ptr we loaded - // above is an i64 (carries the ArrayHeader address); store as - // i64 since the underlying memory is 8 bytes either way. - let oh_addr_3 = blk.gep(I8, &raw, &[(I64, "24")]); - // GC_STORE_AUDIT(INIT): keys_array edge is installed before publishing the new object. - blk.store(I64, &keys_ptr, &oh_addr_3); - - // #6759 Phase B: null the `meta` record pointer — the LAST header - // field, at header offset (object_header_size - pointer_size). - // Pointer-width store: on ILP32 the field is 4 bytes at a - // 4-aligned offset, and an i64 store there would violate the - // arm64_32 `i64:64` ABI alignment (and spill into slot 0). - let meta_off = GC_HEADER_SIZE + object_header_size - meta_ptr_size; - let meta_addr = blk.gep(I8, &raw, &[(I64, &meta_off.to_string())]); - // GC_STORE_AUDIT(INIT): fresh inline object starts with no per-object meta record (#6759 B). - let meta_store_ty = if meta_ptr_size == 4 { I32 } else { I64 }; - blk.store(meta_store_ty, "0", &meta_addr); - - // PerryTS/perry#4717: zero-fill the field slots with `undefined`, mirroring - // `js_object_alloc_with_parent` (runtime object/alloc.rs), which deliberately - // initializes ALL `max(field_count, 8)` slots "to prevent stale data from - // previously freed GC objects from bleeding through." This inline bump path - // wrote only the headers and left the slots uninitialized, so a field - // read-before-write — or a GC that scans the still-constructing instance — - // observed stale arena bytes. When those bytes were a previously-freed - // `undefined`/pointer (e.g. `marked`'s `this.defaults`), the constructor - // crashed with "Cannot read properties of undefined". Slots start at - // raw + GcHeader(8) + ObjectHeader(24) = raw + 32. - for i in 0..alloc_field_count { - let slot_off = GC_HEADER_SIZE + object_header_size + i * FIELD_SLOT_SIZE; - let slot_ptr = blk.gep(I8, &raw, &[(I64, &slot_off.to_string())]); - // GC_STORE_AUDIT(INIT): freshly allocated inline object slot initialized to undefined. - blk.store(I64, crate::nanbox::TAG_UNDEFINED_I64, &slot_ptr); - } - - // User pointer = raw + 8 (the ObjectHeader address — what the - // function-call path returned). Convert to i64 to match what - // the existing nanbox_pointer_inline expects. - let user_ptr = blk.gep(I8, &raw, &[(I64, "8")]); - blk.ptrtoint(&user_ptr, I64) - } - } else { - // Fallback: build the packed-keys string at this site and - // call the slower SHAPE_CACHE-aware allocator. Used when the - // class isn't in `class_keys_globals` (e.g. anonymous / - // synthetic classes that compile_module doesn't pre-emit a - // global for). - let mut packed_keys = String::new(); - let mut parent_chain: Vec<&perry_hir::Class> = Vec::new(); - let mut p = class.extends_name.as_deref(); - while let Some(parent_name) = p { - if let Some(pc) = ctx.classes.get(parent_name).copied() { - parent_chain.push(pc); - p = pc.extends_name.as_deref(); - } else { - break; - } - } - // Skip computed-key fields: their key is an expression evaluated at - // construction time, not a stable string, so they don't get an inline - // slot. The runtime stores them via IndexSet → js_object_set_field / - // js_object_set_symbol_property paths in `apply_field_initializers_recursive`. - // Including their synthetic `__computed_field_*` names in packed_keys - // would surface them as enumerable own properties on Object.keys(). - for pc in parent_chain.iter().rev() { - for f in &pc.fields { - if f.key_expr.is_some() { - continue; - } - packed_keys.push_str(&f.name); - packed_keys.push('\0'); - } - } - for f in &class.fields { - if f.key_expr.is_some() { - continue; - } - packed_keys.push_str(&f.name); - packed_keys.push('\0'); - } - let keys_idx = ctx.strings.intern(&packed_keys); - let keys_entry = ctx.strings.entry(keys_idx); - let keys_global = format!("@{}", keys_entry.bytes_global); - let keys_len_str = keys_entry.byte_len.to_string(); - - ctx.block().call( - I64, - "js_object_alloc_class_with_keys", - &[ - (I32, &cid_str), - (I32, &parent_cid_str), - (I32, &n_str), - (PTR, &keys_global), - (I32, &keys_len_str), - ], - ) - }; + // #7615 slice 8: the field-count computation and the three-arm instance + // allocation moved verbatim to `new_alloc.rs` (see its header for why). + let obj_handle = super::new_alloc::emit_instance_alloc(ctx, class_name, class); // #7154: root the instance for the duration of the constructor body. // // Until now the instance existed ONLY as an SSA register while that body @@ -935,15 +465,20 @@ fn lower_new_impl_inner( // intact-bit guard (#7512). Gated and suppressed as one — see // `layout_declared_at_allocation`. // - // Before the temp-root push, so the handle this names is the one the + // Before the instance root's push, so the handle this names is the one the // allocator returned: nothing between here and there can collect. emit_typed_shape_layout_declare(ctx, class_name, &obj_handle); - let instance_root = construction_runs_user_code(ctx, class_name) - .then(|| temp_root::temp_root_push_i64(ctx, &obj_handle)); + let instance = { + let protected = construction_runs_user_code(ctx, class_name); + Instance { + root: group.adopt_emitted(ctx, Repr::Ptr, &obj_handle, protected), + protected, + } + }; let obj_box = nanbox_pointer_inline(ctx.block(), &obj_handle); // #6969: the instance allocation has run, so refresh every argument before // the constructor consumes them. - refresh_rooted_args(ctx, args, &mut lowered_args, &arg_roots)?; + lowered_args = refresh_rooted_args(ctx, group)?; // Constructor bodies may contain terminating recursive construction // shapes such as `if (typeof opts === "function") return new C(...)`. @@ -1043,7 +578,8 @@ fn lower_new_impl_inner( // would otherwise install the layout descriptor on the abandoned // from-space copy, and `js_ctor_return_override` would hand the // caller that copy's address. - let (obj_handle, obj_box) = reload_instance(ctx, &instance_root, &obj_handle, &obj_box); + let (obj_handle, obj_box) = + reload_instance(ctx, group, &instance, &obj_handle, &obj_box); // The constructor body has run and set the declared fields; register // the typed raw-f64/pointer slot layout so class-field accesses hit // the slot-direct fast path instead of the by-name hashmap fallback. @@ -1134,7 +670,7 @@ fn lower_new_impl_inner( // it, and every `this.x = …` after that collection stores into abandoned // from-space memory. // - // #7192 rooted the instance for the *caller* (`instance_root` above, + // #7192 rooted the instance for the *caller* (`instance.root` above, // re-read by `reload_instance` at the tail) precisely because this window // collects — so the object survives and MOVES. That made the caller's copy // correct and left this one behind: the same address, taken one line later, @@ -1144,7 +680,7 @@ fn lower_new_impl_inner( // Reachability is the default, not an opt-in: `force_ctor_call` requires // `class.constructor.is_some()`, so `class C { payload = mk() }` and // `class C extends B {}` take this path with `PERRY_INLINE_CTOR` unset — - // and `construction_runs_user_code` (which gates `instance_root`) is true + // and `construction_runs_user_code` (which gates `instance.root`) is true // for exactly those, i.e. the code already asserts this window collects. // // Binding it — rather than routing `Expr::This` through a temp root — @@ -1154,14 +690,14 @@ fn lower_new_impl_inner( // contract: the bind is hoisted to entry setup, so the slot is live to the // collector before this store executes. // - // Gated on `instance_root.is_some()`, i.e. on the very same + // Gated on `instance.protected`, i.e. on the very same // `construction_runs_user_code` predicate that decided the instance needed // a temp root at all. When it is false no user code runs between this store // and the pop, so nothing in the window can collect and the slot cannot go // stale — and a class with no constructor, no fields and no heritage keeps // its previous IR exactly, frame size included. One predicate, one place: // forking a second one here is how #7114's two predicates diverged. - if instance_root.is_some() { + if instance.protected { let undef = crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); ctx.func .entry_allocas_push_store(DOUBLE, &undef, &this_slot); @@ -1731,7 +1267,7 @@ fn lower_new_impl_inner( // for the final slot (mirrors method_has_rest, #672). // Field initializers / an inlined constructor body were lowered // between the instance allocation and here, so refresh again. - refresh_rooted_args(ctx, args, &mut lowered_args, &arg_roots)?; + lowered_args = refresh_rooted_args(ctx, group)?; let marshalled = marshal_imported_ctor_args(ctx, &ctor, &lowered_args); let mut ctor_args: Vec<(crate::types::LlvmType, &str)> = Vec::with_capacity(1 + marshalled.len()); @@ -1768,7 +1304,7 @@ fn lower_new_impl_inner( // slot into an array when the ctor's last param is `...rest`. // Field initializers / an inlined constructor body were lowered // between the instance allocation and here, so refresh again. - refresh_rooted_args(ctx, args, &mut lowered_args, &arg_roots)?; + lowered_args = refresh_rooted_args(ctx, group)?; let marshalled = marshal_imported_ctor_args(ctx, &ctor, &lowered_args); // Pass `this` as NaN-boxed double (same as compile_method's this_arg). let mut ctor_args: Vec<(crate::types::LlvmType, &str)> = @@ -1854,7 +1390,7 @@ fn lower_new_impl_inner( ); // Same here: the dynamic-parent `super(...)` buffer is filled long // after the allocation, behind further lowering. - refresh_rooted_args(ctx, args, &mut lowered_args, &arg_roots)?; + lowered_args = refresh_rooted_args(ctx, group)?; let (args_ptr, args_len) = if lowered_args.is_empty() { ("null".to_string(), "0".to_string()) } else { @@ -1944,7 +1480,7 @@ fn lower_new_impl_inner( // constructor body (field initializers, `super(...)`, nested `new`s) can // reach a back-edge poll, and the evacuating minor there relocates the // instance out from under `obj_handle`/`obj_box`. - let (obj_handle, obj_box) = reload_instance(ctx, &instance_root, &obj_handle, &obj_box); + let (obj_handle, obj_box) = reload_instance(ctx, group, &instance, &obj_handle, &obj_box); emit_typed_shape_layout_init(ctx, class_name, &obj_handle); // Close the inline-constructor return: fall through (or branch) to the diff --git a/crates/perry-codegen/src/lower_call/new_alloc.rs b/crates/perry-codegen/src/lower_call/new_alloc.rs new file mode 100644 index 0000000000..2ba0e0057d --- /dev/null +++ b/crates/perry-codegen/src/lower_call/new_alloc.rs @@ -0,0 +1,526 @@ +//! The `new ClassName(...)` **instance allocation**, split out of `new.rs` +//! (#7615 slice 8). +//! +//! A pure move: this is `lower_new_impl_inner`'s field-count computation and +//! its three-arm object allocation, verbatim, wrapped in one function that +//! returns the raw instance handle. Nothing else in the `new` lowering reads +//! any of the locals it defines (`field_count`, `cid_str`, `parent_cid_str`, +//! `n_str`, `packed_keys`, `alloc_field_count`), which is what made the +//! boundary a boundary rather than a cut. +//! +//! The split exists because `new.rs` reached 1,988 lines against +//! `scripts/check_file_size.sh`'s 2,000-line cap, and the Layer 1 rooting +//! migration (#7615) has to ADD lines to it — `refresh_rooted_args` and the +//! `temp_root_scope_*` marker become a `RootedGroup`. Doing the move as its +//! own commit keeps that diff readable: this file has no rooting decision in +//! it at all, because everything it emits sits ABOVE the instance root (the +//! push is `new.rs`'s first act on the returned handle). + +use perry_hir::Class; + +use crate::expr::FnCtx; +use crate::types::{I32, I64, I8, PTR}; + +/// #7469: is the `new` site being lowered inside a **loop body**? +/// +/// This is the gate on the inline bump allocator. Inlining removes the +/// `js_object_alloc_class_inline_keys` FFI call and, with it, the thread-local +/// resolutions that call performs — measured 1.81× on `churn_alloc`, 1.78× on +/// `push_cls`. But it costs ~268 bytes of machine code **per site** (measured +/// over 10 / 200 / 800-site programs: +0, +49,536, +214,656 bytes), so it +/// cannot be the unconditional default against this repo's binary-size +/// campaign — a 5,000-site application would pay ~1.3 MB. +/// +/// Loop membership is the cheapest sound proxy for "this site runs many +/// times", and it bounds the size cost to loop bodies. A `new` executed once +/// keeps the outlined call and contributes nothing to binary growth. +/// +/// `loop_targets` is reused rather than a new counter added, because it is +/// already maintained by all three loop lowerings. It also carries `switch` +/// frames, which push an EMPTY continue label (`switch_stmt.rs`) while every +/// loop pushes a real one — the same discriminator `Stmt::Continue`'s +/// scan-outward-past-switch-frames logic uses. A `new` inside a bare `switch` +/// is therefore correctly treated as not-in-a-loop. +fn new_site_is_in_loop(ctx: &FnCtx<'_>) -> bool { + ctx.loop_targets + .iter() + .any(|(continue_label, _, _)| !continue_label.is_empty()) +} + +/// Emit the instance allocation for `new (...)` and return the raw +/// object handle (an `i64` user pointer, NOT NaN-boxed). +/// +/// Three arms, in the order the original `if`/`else if`/`else` had them: +/// a dynamic-parent subclass (`class X extends _mod.default`), a class with a +/// per-class keys global (inline bump allocator or the outlined +/// `js_object_alloc_class_inline_keys` call, chosen per site by +/// [`new_site_is_in_loop`]), and the `js_object_alloc_class_with_keys` +/// fallback. +/// +/// **No rooting decision is made here and none is possible.** The returned +/// handle is live in an SSA register only until the caller's very next +/// emission, which is the `RootedGroup::adopt_emitted` push that roots it for +/// the constructor body; nothing between the allocator call and that push can +/// collect. +pub(super) fn emit_instance_alloc(ctx: &mut FnCtx<'_>, class_name: &str, class: &Class) -> String { + // Compute total field count including inherited parent fields. + // The runtime allocates at least 8 inline slots regardless, so this + // mostly matters for shapes >8 fields. + let mut field_count = class.fields.len() as u32; + // Imported classes now carry their real field_names from the source + // module. If the field count is still 0 (no fields info available), + // use a generous default as a safety net. + if field_count == 0 && class.constructor.is_none() { + field_count = 32; + } + let mut parent = class.extends_name.as_deref(); + while let Some(parent_name) = parent { + if let Some(p) = ctx.classes.get(parent_name).copied() { + field_count += p.fields.len() as u32; + parent = p.extends_name.as_deref(); + } else { + break; + } + } + // Issue #26 / #321: prefer the authoritative per-class field count computed + // by the source-prefix-disambiguated keys-global builder. The walk above + // resolves parents via `ctx.classes` — a name-keyed map that holds only + // ONE same-named stub — so when a cross-module parent name collides + // (effect's `Type` in SchemaAST.ts vs ParseResult.ts) it counts the wrong + // parent's fields. Using the keys-global's count keeps the allocated slot + // count and the header `field_count` in lockstep with the keys array, + // which `Object.keys()` walks. Falls back to the computed walk when this + // class has no keys global (anonymous / no-keys path). + if let Some(&authoritative) = ctx.class_field_counts.get(class_name) { + field_count = authoritative; + } + // #6812 (w16): a per-site empty-literal anon-shape class may carry a + // compile-time proven builder width. Allocate that many inline slots so + // the FIRST instance of the site is as wide as the runtime-learned + // resizes make every later one — a lone under-sized first instance + // permanently vetoes whole-loop clone eligibility for arrays built at + // the site. Capacity only: the keys array stays authoritative for + // enumeration, and the runtime treats header field_count as alloc_limit. + if class.alloc_width_hint > field_count { + field_count = class.alloc_width_hint; + } + + // Allocate the object with the per-class id and (if applicable) + // parent class id, so the runtime registers the inheritance + // chain for instanceof / virtual dispatch lookups. + // + // Use `js_object_alloc_class_with_keys`, which pre-populates the + // `keys_array` with the class's field names in declaration order + // (parent fields first, walking from the deepest ancestor down, + // then own fields). This is REQUIRED so the LLVM PropertyGet/Set + // fast path's slot indices match the runtime's by-name dispatch + // (which walks `keys_array`). Mixing the two access patterns on + // the same object — e.g. constructor writes via the fast path, + // PropertyUpdate reads via the runtime helper — only produces + // consistent results when both agree on the slot mapping. + // + // The packed-keys constant is interned via the StringPool. Two + // classes with the same field-name set + order share one constant. + let cid = ctx.class_ids.get(class_name).copied().unwrap_or(0); + let parent_cid = class + .extends_name + .as_deref() + .and_then(|p| ctx.class_ids.get(p).copied()) + .unwrap_or(0); + let cid_str = cid.to_string(); + let parent_cid_str = parent_cid.to_string(); + let n_str = field_count.to_string(); + + // Fast path: if the class has a per-class keys global (built once + // at module init via `js_build_class_keys_array`), emit INLINE + // bump-allocator IR — no function call into the runtime at all on + // the hot path. The runtime exposes a `InlineArenaState` struct + // (data ptr at offset 0, current bump offset at offset 8, current + // block size at offset 16) via `js_inline_arena_state()`. We call + // that ONCE per JS function entry (cached in `arena_state_slot`) + // and then emit a 5-instruction bump check + GcHeader/ObjectHeader + // store sequence at every `new ClassName()` site. The slow path + // (block overflow) calls `js_inline_arena_slow_alloc` which syncs + // the inline state back to the underlying arena, allocates a new + // block, and updates the inline state. + // + // Cycles per inlined alloc on the M-series fast path: + // load offset (1) + // add+and align (2) + // add new_offset (1) + // load size + cmp (2) + // cond br (predicted, 0) + // store offset (1) + // load data + gep (2) + // write GcHeader (1) — packed i64 store + // write ObjectHeader×2 (2) — packed i64 stores + // write keys_ptr (1) + // total: ~13 cycles vs ~140 cycles for the function-call path. + // + // Layout assumption: GcHeader is 8 bytes + // {obj_type:u8, gc_flags:u8, _reserved:u16, size:u32} + // and ObjectHeader is 24 bytes + // {object_type:u32, class_id:u32, parent_class_id:u32, + // field_count:u32, keys_array:*ptr} + // followed by `max(field_count, 8)` 8-byte field slots. The user + // pointer the rest of the codegen sees is `raw + 8` (i.e. the + // ObjectHeader address) — same as what + // `js_object_alloc_class_inline_keys` returns. + // + // Layout constants are duplicated here from the runtime; if + // `GcHeader` or `ObjectHeader` ever change in + // `crates/perry-runtime/src/{gc,object}.rs`, update both sides. + if class.extends_expr.is_some() { + // Wall 45: dynamic-parent subclass (`class X extends _mod.default`). + // The parent's field layout is unknown at this compile time (the + // `extends` target is an unresolvable cross-module value, so the + // parent-chain walk above contributed 0 fields and `field_count` / + // `packed_keys` cover only X's OWN fields). Allocating with that + // own-only layout under-sizes and mis-lays-out the instance: the + // parent's constructor and inherited methods address the inherited + // fields at the PARENT's slot indices (parent fields first), which fall + // past X's own slots → OOB heap reads (captures read as garbage). + // Route to `js_object_alloc_class_dynamic_parent`, which resolves the + // runtime-registered parent edge + keys-array (both established at + // module init by `js_register_class_parent_dynamic` / + // `js_build_class_keys_array`, before any `new X()`) and allocates with + // the merged `[parent keys..] ++ [own keys..]` layout. Bypasses the + // inline bump-alloc fast path (which would bake the wrong layout). + let mut packed_keys = String::new(); + for f in &class.fields { + if f.key_expr.is_some() { + continue; + } + packed_keys.push_str(&f.name); + packed_keys.push('\0'); + } + let keys_idx = ctx.strings.intern(&packed_keys); + let keys_entry = ctx.strings.entry(keys_idx); + let keys_global = format!("@{}", keys_entry.bytes_global); + let keys_len_str = keys_entry.byte_len.to_string(); + ctx.block().call( + I64, + "js_object_alloc_class_dynamic_parent", + &[ + (I32, &cid_str), + (I32, &n_str), + (PTR, &keys_global), + (I32, &keys_len_str), + ], + ) + } else if let Some(keys_global_name) = ctx.class_keys_globals.get(class_name).cloned() { + // [#bloat] Outline the per-`new`-site allocator EXCEPT inside a loop. + // + // Outlining collapses ~145 lines of per-class-constant IR per site into + // a single `js_object_alloc_class_inline_keys` call (~3 lines), which + // performs the identical bump alloc + header init + slot zero-fill and + // returns the same user pointer. That is the right default for code + // size, and the size half of the original measurement still holds: + // ~268 bytes of machine code per site, +214,656 bytes over an 800-site + // program. + // + // The SPEED half of that measurement has since inverted. It read + // "~17% faster on an 8M-allocation loop (the inline bump bloated the + // hot loop, hurting icache/regalloc more than the saved call)"; today + // the outlined form is **1.81× SLOWER** on `churn_alloc` and 1.78× on + // `push_cls`. Nothing about the inline bump changed — everything + // *around* the allocation got cheaper (#7474, #7486, #7487, #7501, + // #7525, #7532, #7535, #7536, #7552), so the surviving FFI call and + // the thread-local resolutions it performs now dominate what the + // inline bump's code bloat costs. On Darwin those resolutions cannot + // be made cheaper — Mach-O has no local-exec TLS model, and building + // the runtime with `-Ztls-model=local-exec` leaves the `blr` through + // the TLV descriptor byte-identical (measured: 1.02×). Only their + // COUNT can be reduced, and inlining removes them outright. + // + // So the choice is per site, not global: a `new` inside a loop takes + // the inline bump (it runs many times, and the size cost is bounded to + // loop bodies); everything else keeps the outlined call and + // contributes nothing to binary growth. `PERRY_INLINE_NEW=1` still + // forces the inline form everywhere, for A/B measurement. + // + // NOTE the env test is `is_none()`: `PERRY_INLINE_NEW=""` *enables* + // the inline path, because an empty string is `Some("")`. + let force_inline_new = std::env::var_os("PERRY_INLINE_NEW").is_some(); + if !force_inline_new && !new_site_is_in_loop(ctx) { + let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { + s + } else { + let s = ctx.func.entry_init_load_global(&keys_global_name, I64); + ctx.class_keys_slots + .insert(class_name.to_string(), s.clone()); + s + }; + let keys_ptr = ctx.block().load(I64, &keys_slot); + ctx.pending_declares.push(( + "js_object_alloc_class_inline_keys".to_string(), + I64, + vec![I32, I32, I32, I64], + )); + ctx.block().call( + I64, + "js_object_alloc_class_inline_keys", + &[ + (I32, &cid_str), + (I32, &parent_cid_str), + (I32, &field_count.to_string()), + (I64, &keys_ptr), + ], + ) + } else { + // Compile-time layout constants. + const GC_HEADER_SIZE: u64 = 8; + // arm64_32 watchOS: `size_of::()` is 24 on 64-bit but + // 20 on ILP32 (4-byte `keys_array` pointer). Derive from the target + // triple so the inline alloc size and field-region base match the + // target-compiled runtime (no-op on 64-bit; see `target_layout`). + let object_header_size: u64 = + crate::target_layout::object_header_size_bytes(ctx.target_triple); + // #6759 Phase B: pointer width for the trailing `meta` header + // field (computed here, before `ctx.block()` mutably borrows). + let meta_ptr_size: u64 = if crate::target_layout::target_is_ilp32(ctx.target_triple) { + 4 + } else { + 8 + }; + const FIELD_SLOT_SIZE: u64 = 8; + // Inline-slot floor — MUST match perry-runtime `object::INLINE_SLOT_FLOOR` + // (they independently pad `new` objects to the same minimum; a mismatch + // where codegen allocs fewer slots than the runtime's get/set bound-check + // assumes is heap corruption). Lowered 8->4 to shrink small-object footprint. + const MIN_FIELD_SLOTS: u64 = 4; + const GC_TYPE_OBJECT: u64 = 2; + const GC_FLAG_ARENA: u64 = 0x02; + // PR #1146: pointer-free hint for inline-allocated regular + // objects. The field-store sites issue per-slot + // `js_gc_note_slot_layout` so the GC sees real pointer-bearing + // slots regardless of this initial tag. + const GC_LAYOUT_POINTER_FREE: u64 = 0x4000; + const OBJECT_TYPE_REGULAR: u64 = 1; + + let alloc_field_count = std::cmp::max(field_count as u64, MIN_FIELD_SLOTS); + let payload_size = object_header_size + alloc_field_count * FIELD_SLOT_SIZE; + // Round the whole allocation up to FIELD_SLOT_SIZE (8). The inline + // bump allocator's offset invariant (below) requires every + // allocation to be a multiple of 8; on ILP32 `object_header_size` + // is 20, so an unpadded total is 4-skewed (e.g. 92) and would + // misalign the next bump. No-op on 64-bit (8 + 24 + 8·n is already + // 8-aligned → 96 for ≤8 fields). + let total_size = (GC_HEADER_SIZE + payload_size).next_multiple_of(FIELD_SLOT_SIZE); + let total_size_str = total_size.to_string(); + + // Lazy: allocate the per-function arena-state slot on the + // first `new` we see. The slot init (`call @js_inline_arena_state` + // + store) lives in the entry block via `entry_init_call_ptr`, + // so it dominates every reachable use. + let arena_state_slot = if let Some(slot) = ctx.arena_state_slot.clone() { + slot + } else { + let slot = ctx.func.entry_init_call_ptr("js_inline_arena_state"); + ctx.arena_state_slot = Some(slot.clone()); + slot + }; + + // Hoist the per-class `keys_array` global load to the function + // entry block (cached in a stack slot per class). Without this + // hoisting, LLVM would reload `@perry_class_keys_` on + // every loop iteration, because the loop body's `call + // @js_inline_arena_slow_alloc` blocks LICM — LLVM can't prove + // the call doesn't modify the global. + let keys_slot = if let Some(s) = ctx.class_keys_slots.get(class_name).cloned() { + s + } else { + let s = ctx.func.entry_init_load_global(&keys_global_name, I64); + ctx.class_keys_slots + .insert(class_name.to_string(), s.clone()); + s + }; + let keys_ptr = ctx.block().load(I64, &keys_slot); + + // Inline bump-allocator IR. + let blk = ctx.block(); + let state_ptr = blk.load(PTR, &arena_state_slot); + + // offset = state.offset (at byte offset 8 in InlineArenaState). + // The offset is invariant 8-aligned: arena blocks start at offset 0 + // (8-aligned), every allocation is a multiple of 8 (`total_size` + // includes the 8-byte GcHeader and `MIN_FIELD_SLOTS=4` slots × + // 8 bytes), and `js_inline_arena_slow_alloc` only ever swings the + // state to `block.offset` which is also always 8-aligned. So we + // skip the `(offset + 7) & -8` align-up step entirely — saves + // 2 instructions per iter on the hot path. + let offset_field_ptr = blk.gep(I8, &state_ptr, &[(I64, "8")]); + let offset_val = blk.load(I64, &offset_field_ptr); + let aligned_off = offset_val.clone(); + + // new_offset = aligned + total_size + let new_offset = blk.add(I64, &aligned_off, &total_size_str); + + // size = state.size (at byte offset 16) + let size_field_ptr = blk.gep(I8, &state_ptr, &[(I64, "16")]); + let size_val = blk.load(I64, &size_field_ptr); + + // fits = new_offset <= size + let fits = blk.icmp_ule(I64, &new_offset, &size_val); + + // Set up fast/slow/merge basic blocks. + let fast_idx = ctx.new_block("alloc.fast"); + let slow_idx = ctx.new_block("alloc.slow"); + let merge_idx = ctx.new_block("alloc.merge"); + let fast_label = ctx.block_label(fast_idx); + let slow_label = ctx.block_label(slow_idx); + let merge_label = ctx.block_label(merge_idx); + + ctx.block().cond_br(&fits, &fast_label, &slow_label); + + // ---- Fast path: bump and return data + aligned ---- + ctx.current_block = fast_idx; + let blk = ctx.block(); + // GC_STORE_AUDIT(INIT): inline arena bump offset is allocator metadata, not a JS heap edge. + blk.store(I64, &new_offset, &offset_field_ptr); + // data ptr is at byte offset 0 in InlineArenaState + let data_ptr = blk.load(PTR, &state_ptr); + let raw_fast = blk.gep(I8, &data_ptr, &[(I64, &aligned_off)]); + let fast_pred_label = blk.label.clone(); + blk.br(&merge_label); + + // ---- Slow path: call into the runtime ---- + ctx.current_block = slow_idx; + let raw_slow = ctx.block().call( + PTR, + "js_inline_arena_slow_alloc", + &[(PTR, &state_ptr), (I64, &total_size_str), (I64, "8")], + ); + let slow_pred_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + // ---- Merge: phi the raw pointer, write headers, NaN-box ---- + ctx.current_block = merge_idx; + let blk = ctx.block(); + let raw = blk.phi( + PTR, + &[(&raw_fast, &fast_pred_label), (&raw_slow, &slow_pred_label)], + ); + + // Write GcHeader (8 bytes) as a single i64 store. Field + // packing (little-endian): + // bits 0..7 = obj_type (u8) + // bits 8..15 = gc_flags (u8) + // bits 16..31 = _reserved (u16) + // bits 32..63 = size (u32) + let gc_packed: u64 = GC_TYPE_OBJECT + | (GC_FLAG_ARENA << 8) + | (GC_LAYOUT_POINTER_FREE << 16) + | ((total_size as u64) << 32); + // GC_STORE_AUDIT(INIT): inline headers initialize freshly allocated unpublished object storage. + blk.store(I64, &gc_packed.to_string(), &raw); + + // Write ObjectHeader at raw + 8. + // First 8 bytes: object_type (u32, low) | class_id (u32, high) + let oh_addr_1 = blk.gep(I8, &raw, &[(I64, "8")]); + let oh_word_1: u64 = OBJECT_TYPE_REGULAR | ((cid as u64) << 32); + blk.store(I64, &oh_word_1.to_string(), &oh_addr_1); + + // Second 8 bytes: parent_class_id (u32, low) | field_count (u32, high) + let oh_addr_2 = blk.gep(I8, &raw, &[(I64, "16")]); + let oh_word_2: u64 = (parent_cid as u64) | ((field_count as u64) << 32); + blk.store(I64, &oh_word_2.to_string(), &oh_addr_2); + + // Third 8 bytes: keys_array pointer. The keys_ptr we loaded + // above is an i64 (carries the ArrayHeader address); store as + // i64 since the underlying memory is 8 bytes either way. + let oh_addr_3 = blk.gep(I8, &raw, &[(I64, "24")]); + // GC_STORE_AUDIT(INIT): keys_array edge is installed before publishing the new object. + blk.store(I64, &keys_ptr, &oh_addr_3); + + // #6759 Phase B: null the `meta` record pointer — the LAST header + // field, at header offset (object_header_size - pointer_size). + // Pointer-width store: on ILP32 the field is 4 bytes at a + // 4-aligned offset, and an i64 store there would violate the + // arm64_32 `i64:64` ABI alignment (and spill into slot 0). + let meta_off = GC_HEADER_SIZE + object_header_size - meta_ptr_size; + let meta_addr = blk.gep(I8, &raw, &[(I64, &meta_off.to_string())]); + // GC_STORE_AUDIT(INIT): fresh inline object starts with no per-object meta record (#6759 B). + let meta_store_ty = if meta_ptr_size == 4 { I32 } else { I64 }; + blk.store(meta_store_ty, "0", &meta_addr); + + // PerryTS/perry#4717: zero-fill the field slots with `undefined`, mirroring + // `js_object_alloc_with_parent` (runtime object/alloc.rs), which deliberately + // initializes ALL `max(field_count, 8)` slots "to prevent stale data from + // previously freed GC objects from bleeding through." This inline bump path + // wrote only the headers and left the slots uninitialized, so a field + // read-before-write — or a GC that scans the still-constructing instance — + // observed stale arena bytes. When those bytes were a previously-freed + // `undefined`/pointer (e.g. `marked`'s `this.defaults`), the constructor + // crashed with "Cannot read properties of undefined". Slots start at + // raw + GcHeader(8) + ObjectHeader(24) = raw + 32. + for i in 0..alloc_field_count { + let slot_off = GC_HEADER_SIZE + object_header_size + i * FIELD_SLOT_SIZE; + let slot_ptr = blk.gep(I8, &raw, &[(I64, &slot_off.to_string())]); + // GC_STORE_AUDIT(INIT): freshly allocated inline object slot initialized to undefined. + blk.store(I64, crate::nanbox::TAG_UNDEFINED_I64, &slot_ptr); + } + + // User pointer = raw + 8 (the ObjectHeader address — what the + // function-call path returned). Convert to i64 to match what + // the existing nanbox_pointer_inline expects. + let user_ptr = blk.gep(I8, &raw, &[(I64, "8")]); + blk.ptrtoint(&user_ptr, I64) + } + } else { + // Fallback: build the packed-keys string at this site and + // call the slower SHAPE_CACHE-aware allocator. Used when the + // class isn't in `class_keys_globals` (e.g. anonymous / + // synthetic classes that compile_module doesn't pre-emit a + // global for). + let mut packed_keys = String::new(); + let mut parent_chain: Vec<&perry_hir::Class> = Vec::new(); + let mut p = class.extends_name.as_deref(); + while let Some(parent_name) = p { + if let Some(pc) = ctx.classes.get(parent_name).copied() { + parent_chain.push(pc); + p = pc.extends_name.as_deref(); + } else { + break; + } + } + // Skip computed-key fields: their key is an expression evaluated at + // construction time, not a stable string, so they don't get an inline + // slot. The runtime stores them via IndexSet → js_object_set_field / + // js_object_set_symbol_property paths in `apply_field_initializers_recursive`. + // Including their synthetic `__computed_field_*` names in packed_keys + // would surface them as enumerable own properties on Object.keys(). + for pc in parent_chain.iter().rev() { + for f in &pc.fields { + if f.key_expr.is_some() { + continue; + } + packed_keys.push_str(&f.name); + packed_keys.push('\0'); + } + } + for f in &class.fields { + if f.key_expr.is_some() { + continue; + } + packed_keys.push_str(&f.name); + packed_keys.push('\0'); + } + let keys_idx = ctx.strings.intern(&packed_keys); + let keys_entry = ctx.strings.entry(keys_idx); + let keys_global = format!("@{}", keys_entry.bytes_global); + let keys_len_str = keys_entry.byte_len.to_string(); + + ctx.block().call( + I64, + "js_object_alloc_class_with_keys", + &[ + (I32, &cid_str), + (I32, &parent_cid_str), + (I32, &n_str), + (PTR, &keys_global), + (I32, &keys_len_str), + ], + ) + } +} diff --git a/crates/perry-codegen/src/lower_string_concat.rs b/crates/perry-codegen/src/lower_string_concat.rs new file mode 100644 index 0000000000..e77deb57bc --- /dev/null +++ b/crates/perry-codegen/src/lower_string_concat.rs @@ -0,0 +1,646 @@ +//! String CONCATENATION and self-append lowering. +//! +//! Split out of `lower_string_method.rs` (#7615 slice 8). That file was 1,957 +//! lines against `scripts/check_file_size.sh`'s 2,000-line cap, and the Layer 1 +//! rooting migration has to add closure scopes to five of the functions here — +//! `with_operands_rooted` and `with_rooted_accumulator` both re-indent the body +//! they own, which is line growth on a file with 43 lines of headroom. +//! +//! Pure move, no behaviour change. The boundary is the one the file already +//! had: everything above it dispatches a `str.(...)` call, everything +//! here lowers `a + b` / `s += x` on strings. `str_operand_handle_tag_dispatched` +//! is `pub(crate)` rather than private because three of the dispatch arms above +//! call it. + +use anyhow::{anyhow, Result}; +use perry_hir::Expr; + +use crate::expr::{lower_expr, nanbox_string_inline, unbox_str_handle, FnCtx}; +use crate::type_analysis::is_string_expr; +use crate::types::{DOUBLE, I1, I32, I64}; + +use crate::rooting::{with_operands_rooted, with_rooted_group, Repr}; + +/// Lower the `str = str + rhs` self-append pattern. Uses the in-place +/// `js_string_append` runtime function (refcount=1 → mutate in place, +/// otherwise allocate). The returned pointer is stored back to the local +/// slot — `js_string_append` may realloc when growing past capacity. +/// +/// This is the load-bearing optimization for the canonical `let str = ""; +/// for (...) str = str + "a"` string-build pattern. +pub(crate) fn lower_string_self_append( + ctx: &mut FnCtx<'_>, + local_id: u32, + rhs: &Expr, +) -> Result { + let slot = ctx + .locals + .get(&local_id) + .ok_or_else(|| anyhow!("string self-append: local {} not in scope", local_id))? + .clone(); + + // Representation-selection Phase 3a: canonical-Str destination — + // tag-dispatch on the slot bits inline instead of paying the two opaque + // `js_get_string_pointer_unified` calls per iteration. + if crate::expr::local_is_canonical_str(ctx, local_id) { + return lower_canonical_str_self_append(ctx, local_id, rhs, &slot); + } + + // Lower the RHS first (might be a string literal, a local, or a + // computed expression). For non-string RHS we'd need to coerce, but + // the bench_string_ops case always uses a string literal, so for the + // first slice we require the RHS to be a known string. + if !is_string_expr(ctx, rhs) { + // Fall back to the slower concat path: load the local, do a + // generic concat-coerce, store back. + let lhs_val = ctx.block().load(DOUBLE, &slot); + let _lhs = lhs_val.clone(); + let rhs_val = lower_expr(ctx, rhs)?; + let blk = ctx.block(); + // Issue #214: SSO-safe unbox. + let l_handle = unbox_str_handle(blk, &lhs_val); + // Coerce non-string RHS to a string handle. + let r_handle = blk.call(I64, "js_jsvalue_to_string", &[(DOUBLE, &rhs_val)]); + let result = blk.call( + I64, + "js_string_append", + &[(I64, &l_handle), (I64, &r_handle)], + ); + let new_box = nanbox_string_inline(blk, &result); + blk.store(DOUBLE, &new_box, &slot); + return Ok(new_box); + } + + let rhs_box = lower_expr(ctx, rhs)?; + let blk = ctx.block(); + let lhs_box = blk.load(DOUBLE, &slot); + // Issue #214: SSO-safe unbox. + let l_handle = unbox_str_handle(blk, &lhs_box); + let r_handle = unbox_str_handle(blk, &rhs_box); + let new_handle = blk.call( + I64, + "js_string_append", + &[(I64, &l_handle), (I64, &r_handle)], + ); + let new_box = nanbox_string_inline(blk, &new_handle); + blk.store(DOUBLE, &new_box, &slot); + Ok(new_box) +} + +/// Repsel Phase 3a: is this expression PROVEN to lower to a heap-tagged +/// (`STRING_TAG`) NaN-box — never SSO bits, never a non-string? String +/// literals load the interned pool handle (`@.str.N.handle`, always a heap +/// `StringHeader` from `js_string_from_bytes`); `String(x)` routes through +/// `js_string_coerce`, which always allocates a heap header. Deliberately +/// NOT included: `Binary Add` string results — the pairwise concat lowering +/// returns `js_string_concat_box`, which assembles ≤5-byte ASCII results as +/// SSO bits. +fn proven_heap_string_operand(_ctx: &FnCtx<'_>, e: &Expr) -> bool { + match e { + Expr::String(_) | Expr::WtfString(_) | Expr::StringCoerce(_) => true, + Expr::Conditional { + then_expr, + else_expr, + .. + } => { + proven_heap_string_operand(_ctx, then_expr) + && proven_heap_string_operand(_ctx, else_expr) + } + _ => false, + } +} + +/// Repsel Phase 3a: operand → raw `StringHeader*` handle for the string +/// helpers, tag-dispatched: +/// +/// - proven heap-tagged operand (see `proven_heap_string_operand`) → inline +/// `bitcast; and POINTER_MASK` — zero calls; +/// - canonical-Str `LocalGet` → 2-arm dispatch: heap `STRING_TAG` bits → +/// bare `and POINTER_MASK` (hot arm, no call); anything else (SSO bits, +/// annotation lie) → the legacy `js_get_string_pointer_unified` (which +/// materializes SSO — cold); +/// - everything else (or flag off) → the legacy unified call, unchanged. +/// +/// #7128: the two arms are on separate knobs, because only the second one is +/// about a selected representation. The proven-heap arm keys on the operand's +/// static type and fires with zero canonical-`Str` locals in the program. +pub(crate) fn str_operand_handle_tag_dispatched( + ctx: &mut FnCtx<'_>, + object: &Expr, + recv_box: &str, +) -> String { + use crate::nanbox::POINTER_MASK_I64; + if crate::expr::static_string_lowering_enabled() && proven_heap_string_operand(ctx, object) { + let bits = ctx.block().bitcast_double_to_i64(recv_box); + return ctx.block().and(I64, &bits, POINTER_MASK_I64); + } + let canonical = crate::expr::canonical_str_locals_enabled() + && matches!( + object, Expr::LocalGet(id) if crate::expr::local_is_canonical_str(ctx, *id) + ); + if !canonical { + return unbox_str_handle(ctx.block(), recv_box); + } + let bits = ctx.block().bitcast_double_to_i64(recv_box); + let tag = ctx.block().lshr(I64, &bits, "48"); + let is_heap = ctx + .block() + .icmp_eq(I64, &tag, crate::nanbox::STRING_TAG_TOP16_I64); + + let heap_idx = ctx.new_block("strrecv.heap"); + let cold_idx = ctx.new_block("strrecv.cold"); + let merge_idx = ctx.new_block("strrecv.merge"); + let heap_label = ctx.block_label(heap_idx); + let cold_label = ctx.block_label(cold_idx); + let merge_label = ctx.block_label(merge_idx); + ctx.block().cond_br(&is_heap, &heap_label, &cold_label); + + ctx.current_block = heap_idx; + let h_heap = ctx.block().and(I64, &bits, POINTER_MASK_I64); + let heap_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = cold_idx; + let h_cold = unbox_str_handle(ctx.block(), recv_box); + let cold_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = merge_idx; + ctx.block() + .phi(I64, &[(&h_heap, &heap_pred), (&h_cold, &cold_pred)]) +} + +/// Representation-selection Phase 3a: `s += rhs` for a canonical-Str +/// destination (`SlotRep::Str` — the `ctx.locals` slot provably holds +/// NaN-box string bits at rest). Replaces the two opaque +/// `js_get_string_pointer_unified` calls per iteration with an inline tag +/// dispatch on the slot bits: +/// +/// - **both heap** (`STRING_TAG` on both sides): `and POINTER_MASK` → +/// `js_string_append(h, h)` → `or STRING_TAG` — the hot accumulator-loop +/// arm; keeps the refcount==1 in-place append (every alias demote site is +/// untouched by this phase, so `let b = a` still demotes first). +/// - **both strings, SSO involved**: `js_string_concat_box(box, box)` — +/// SSO-aware pairwise concat, assembles ≤5-byte ASCII results inline and +/// never mutates in place. No per-op heap materialization of SSO bits +/// (RFC §4 "short-string values stay by-value"). +/// - **anything else** (a lying `string` annotation): the exact pre-phase +/// sequence — `js_get_string_pointer_unified` ×2 (SSO materialize + +/// number coercion included) → `js_string_append` — so acceptance +/// behavior is bit-identical to today's on non-string bits (RFC §5.5: +/// mismatches route to the legacy path, never a new coercion). +fn lower_canonical_str_self_append( + ctx: &mut FnCtx<'_>, + _local_id: u32, + rhs: &Expr, + slot: &str, +) -> Result { + use crate::nanbox::{ + POINTER_MASK_I64, SHORT_STRING_TAG_TOP16_I64 as TAG_SSO_STR, + STRING_TAG_TOP16_I64 as TAG_HEAP_STR, + }; + + if !is_string_expr(ctx, rhs) { + // Non-string rhs: mirror the legacy fallback's evaluation order + // (lhs slot load, then rhs), coerce the rhs once (heap handle + // guaranteed), then 2-arm on the destination tag only. + let lhs_box = ctx.block().load(DOUBLE, slot); + // #6951: the load must happen before `rhs` per `s += rhs` evaluation + // order (a `rhs` that reassigns `s` must not be observed here), so the + // pre-rhs value has to be carried across `rhs`'s evaluation and the + // `js_jsvalue_to_string` coercion — both of which allocate. Re-reading + // the slot would take the wrong value; re-read the root instead. + // + // #7615 slice 8: both values are produced by an EMITTED step rather + // than by lowering an `Expr` (a slot load and a coercion call), which + // is what `RootedGroup::adopt_emitted` is for, and both are re-read at + // more than one caller-chosen point — the `lhs` once below the + // coercion, the coerced `rhs` once per arm of the tag diamond. That + // multi-point re-read is why this is a group and not + // `with_operands_rooted`. + return with_rooted_group(ctx, 0, |ctx, group| { + let lhs_root = group.adopt_emitted(ctx, Repr::Boxed, &lhs_box, true); + let rhs_val = lower_expr(ctx, rhs)?; + let r_handle = ctx + .block() + .call(I64, "js_jsvalue_to_string", &[(DOUBLE, &rhs_val)]); + // The coerced rhs is a bare string handle that has to survive the cold + // arm's `unbox_str_handle`, which materializes an SSO destination onto + // the heap — another allocation. Root it too and re-read it per arm. + let r_root = group.adopt_emitted(ctx, Repr::Ptr, &r_handle, true); + let lhs_box = group.reread_emitted(ctx, lhs_root); + let bits_d = ctx.block().bitcast_double_to_i64(&lhs_box); + let tag_d = ctx.block().lshr(I64, &bits_d, "48"); + let is_heap = ctx.block().icmp_eq(I64, &tag_d, TAG_HEAP_STR); + + let heap_idx = ctx.new_block("strapp.heap"); + let cold_idx = ctx.new_block("strapp.cold"); + let merge_idx = ctx.new_block("strapp.merge"); + let heap_label = ctx.block_label(heap_idx); + let cold_label = ctx.block_label(cold_idx); + let merge_label = ctx.block_label(merge_idx); + ctx.block().cond_br(&is_heap, &heap_label, &cold_label); + + ctx.current_block = heap_idx; + let h_d = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); + let r_heap = group.reread_emitted(ctx, r_root); + let h_heap = ctx + .block() + .call(I64, "js_string_append", &[(I64, &h_d), (I64, &r_heap)]); + let heap_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = cold_idx; + let h_d2 = unbox_str_handle(ctx.block(), &lhs_box); + let r_cold = group.reread_emitted(ctx, r_root); + let h_cold = ctx + .block() + .call(I64, "js_string_append", &[(I64, &h_d2), (I64, &r_cold)]); + let cold_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = merge_idx; + let handle = ctx + .block() + .phi(I64, &[(&h_heap, &heap_pred), (&h_cold, &cold_pred)]); + let new_box = nanbox_string_inline(ctx.block(), &handle); + ctx.block().store(DOUBLE, &new_box, slot); + // The group's release is one truncate at the LOWEST slot it holds — + // `lhs_root` — which drops the coerced-rhs slot with it, because a + // truncate is a stack cut. It runs in the merge block, which the slot + // registers (defined in the entry block) dominate. + Ok(new_box) + }); + } + + // Proven-string rhs: mirror the legacy fast path's evaluation order + // (rhs first, then the lhs slot load). + // + // Arm layout — the load-bearing property is that a HEAP destination + // ALWAYS reaches `js_string_append` (whose refcount==1 in-place path is + // what makes accumulator loops amortized O(n)). Routing a heap-dest / + // SSO-rhs iteration through `js_string_concat_box` instead would copy + // the whole accumulator every time a ≤5-byte part arrives — O(n²). + // + // dest heap, rhs heap → append(h, h) (hot, no calls) + // dest heap, rhs other → append(h, unified(rhs)) (legacy-exact: + // unified materializes SSO / coerces a lie) + // dest SSO → js_string_concat_box (SSO-aware, + // nothing to mutate in place; result may stay + // SSO — no per-op heap materialization) + // dest other (lie) → unified ×2 + append (legacy-exact) + let rhs_box = lower_expr(ctx, rhs)?; + let lhs_box = ctx.block().load(DOUBLE, slot); + let bits_d = ctx.block().bitcast_double_to_i64(&lhs_box); + let bits_r = ctx.block().bitcast_double_to_i64(&rhs_box); + let tag_d = ctx.block().lshr(I64, &bits_d, "48"); + let tag_r = ctx.block().lshr(I64, &bits_r, "48"); + let d_heap = ctx.block().icmp_eq(I64, &tag_d, TAG_HEAP_STR); + + let dheap_idx = ctx.new_block("strapp.dheap"); + let heap_idx = ctx.new_block("strapp.heap"); + let rcold_idx = ctx.new_block("strapp.rcold"); + let dother_idx = ctx.new_block("strapp.dother"); + let sso_idx = ctx.new_block("strapp.sso"); + let cold_idx = ctx.new_block("strapp.cold"); + let merge_idx = ctx.new_block("strapp.merge"); + let dheap_label = ctx.block_label(dheap_idx); + let heap_label = ctx.block_label(heap_idx); + let rcold_label = ctx.block_label(rcold_idx); + let dother_label = ctx.block_label(dother_idx); + let sso_label = ctx.block_label(sso_idx); + let cold_label = ctx.block_label(cold_idx); + let merge_label = ctx.block_label(merge_idx); + ctx.block().cond_br(&d_heap, &dheap_label, &dother_label); + + // dest heap: split on the rhs tag. + ctx.current_block = dheap_idx; + let r_heap = ctx.block().icmp_eq(I64, &tag_r, TAG_HEAP_STR); + ctx.block().cond_br(&r_heap, &heap_label, &rcold_label); + + ctx.current_block = heap_idx; + let h_d = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); + let h_r = ctx.block().and(I64, &bits_r, POINTER_MASK_I64); + let h_new = ctx + .block() + .call(I64, "js_string_append", &[(I64, &h_d), (I64, &h_r)]); + let box_heap = nanbox_string_inline(ctx.block(), &h_new); + let heap_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = rcold_idx; + let h_d1 = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); + let r_h1 = unbox_str_handle(ctx.block(), &rhs_box); + let h_rc = ctx + .block() + .call(I64, "js_string_append", &[(I64, &h_d1), (I64, &r_h1)]); + let box_rcold = nanbox_string_inline(ctx.block(), &h_rc); + let rcold_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + // dest not heap: an SSO dest with a real-string rhs takes the SSO-aware + // pairwise concat; a lie on EITHER side keeps the exact legacy sequence + // (`js_string_concat_box` treats a non-string operand as empty, but the + // legacy unified path ToString-coerces it — `"ab" += 42` must stay + // `"ab42"`). + ctx.current_block = dother_idx; + let d_sso = ctx.block().icmp_eq(I64, &tag_d, TAG_SSO_STR); + let r_heap2 = ctx.block().icmp_eq(I64, &tag_r, TAG_HEAP_STR); + let r_sso = ctx.block().icmp_eq(I64, &tag_r, TAG_SSO_STR); + let r_str = ctx.block().or(I1, &r_heap2, &r_sso); + let take_sso = ctx.block().and(I1, &d_sso, &r_str); + ctx.block().cond_br(&take_sso, &sso_label, &cold_label); + + ctx.current_block = sso_idx; + let box_sso = ctx.block().call( + DOUBLE, + "js_string_concat_box", + &[(DOUBLE, &lhs_box), (DOUBLE, &rhs_box)], + ); + let sso_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = cold_idx; + let l_h = unbox_str_handle(ctx.block(), &lhs_box); + let r_h = unbox_str_handle(ctx.block(), &rhs_box); + let h_cold = ctx + .block() + .call(I64, "js_string_append", &[(I64, &l_h), (I64, &r_h)]); + let box_cold = nanbox_string_inline(ctx.block(), &h_cold); + let cold_pred = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = merge_idx; + let new_box = ctx.block().phi( + DOUBLE, + &[ + (&box_heap, &heap_pred), + (&box_rcold, &rcold_pred), + (&box_sso, &sso_pred), + (&box_cold, &cold_pred), + ], + ); + ctx.block().store(DOUBLE, &new_box, slot); + Ok(new_box) +} + +/// Lower `string + non_string` (or vice versa) concat with runtime +/// coercion of the non-string side. The non-string operand passes through +/// `js_jsvalue_to_string` which inspects its NaN tag and produces the +/// canonical JS string form (numbers via the formatter at +/// `crates/perry-runtime/src/value.rs:825`, booleans → `"true"`/`"false"`, +/// objects → `"[object Object]"`, etc.). +/// +/// The string-typed side still uses the fast inline `bitcast double → i64; +/// and POINTER_MASK_I64` unbox; only the non-string side pays the function +/// call. Both operand handles then feed `js_string_concat`. +pub(crate) fn lower_string_coerce_concat( + ctx: &mut FnCtx<'_>, + left: &Expr, + right: &Expr, + l_is_string: bool, + r_is_string: bool, +) -> Result { + // #6951: `l_box` is a heap string in an SSA register while `right` is + // lowered. If `right` allocates (`"tag" + f()`), a collection sweeps the + // left operand and the concat reads freed memory — a segfault, not a + // dropped character. `with_operands_rooted` emits nothing at all when + // `right` provably cannot collect, which is the common `"user_" + i` case, + // and it owns the release on all three exits — including the two early + // returns below, which is where #7462's "released on one arm" lived. + with_operands_rooted(ctx, &[left, right], |ctx, values| { + coerce_concat_body( + ctx, + left, + right, + &values[0], + &values[1], + l_is_string, + r_is_string, + ) + }) +} + +/// The body of [`lower_string_coerce_concat`], below its operand roots. +/// +/// A separate function rather than a closure body so the three arms keep their +/// indentation — and so the ONE place that still needs a nested scope (the +/// both-non-string fallback, whose left coercion has to survive the right +/// coercion) reads as the exception it is. +#[allow(clippy::too_many_arguments)] +fn coerce_concat_body( + ctx: &mut FnCtx<'_>, + left: &Expr, + right: &Expr, + l_box: &str, + r_box: &str, + l_is_string: bool, + r_is_string: bool, +) -> Result { + // Issue #58: fused string+value concat — when one side is a string + // and the other is not, use the fused runtime call that collapses + // js_jsvalue_to_string + js_string_concat into a single allocation + // for number operands (the common `"item_" + i` pattern). + if l_is_string && !r_is_string { + // Issue #214: SSO-safe unbox; repsel Phase 3a: inline `bitcast+and` + // for proven-heap operands (string literals — the `"user_" + i` + // shape) and tag-dispatch for canonical-Str locals. + let l_handle = str_operand_handle_tag_dispatched(ctx, left, l_box); + let blk = ctx.block(); + let result_handle = blk.call( + I64, + "js_string_concat_value", + &[(I64, &l_handle), (DOUBLE, r_box)], + ); + return Ok(nanbox_string_inline(blk, &result_handle)); + } + + if !l_is_string && r_is_string { + // Issue #214: SSO-safe unbox; repsel Phase 3a: see above. + let r_handle = str_operand_handle_tag_dispatched(ctx, right, r_box); + let blk = ctx.block(); + let result_handle = blk.call( + I64, + "js_value_concat_string", + &[(DOUBLE, l_box), (I64, &r_handle)], + ); + return Ok(nanbox_string_inline(blk, &result_handle)); + } + + // Both non-string (shouldn't normally reach here) — fall back to + // the generic path. + let l_handle = ctx + .block() + .call(I64, "js_jsvalue_to_string", &[(DOUBLE, l_box)]); + // The coercion of the right operand allocates, and `l_handle` is a bare + // string address in an SSA register — root it across that call (#6951). + // It is an EMITTED value (a coercion result, not a lowered `Expr`), so the + // group is the only form that can take it. + with_rooted_group(ctx, 0, |ctx, group| { + let l_root = group.adopt_emitted(ctx, Repr::Ptr, &l_handle, true); + let r_handle = ctx + .block() + .call(I64, "js_jsvalue_to_string", &[(DOUBLE, r_box)]); + let l_handle = group.reread_emitted(ctx, l_root); + let blk = ctx.block(); + let result_handle = blk.call( + I64, + "js_string_concat", + &[(I64, &l_handle), (I64, &r_handle)], + ); + Ok(nanbox_string_inline(blk, &result_handle)) + }) +} + +/// Lower a static `s1 + s2` string concatenation. Both operands must +/// already be statically string-typed (caller's responsibility — see +/// `is_string_expr`). +/// +/// Pattern: +/// ```llvm +/// ; %l_box and %r_box are NaN-boxed strings (double values with STRING_TAG) +/// %l_bits = bitcast double %l_box to i64 +/// %l_handle = and i64 %l_bits, 281474976710655 ; POINTER_MASK_I64 +/// %r_bits = bitcast double %r_box to i64 +/// %r_handle = and i64 %r_bits, 281474976710655 +/// %result_handle = call i64 @js_string_concat(i64 %l_handle, i64 %r_handle) +/// %result_box = call double @js_nanbox_string(i64 %result_handle) +/// ``` +/// +/// The bitcast+and is the inline-fast unboxing pattern. We avoid calling +/// the slower `js_nanbox_get_pointer` (which does the same thing in Rust) +/// to keep concat hot-path overhead minimal. +pub(crate) fn lower_string_concat( + ctx: &mut FnCtx<'_>, + left: &Expr, + right: &Expr, +) -> Result { + // #6951: same hazard as `lower_string_coerce_concat` — the left operand is + // a heap string in an SSA register across the right operand's evaluation. + with_operands_rooted(ctx, &[left, right], |ctx, values| { + let blk = ctx.block(); + // SSO-aware fast path: pass operands as NaN-boxed f64s directly to + // `js_string_concat_sso`, which keeps SSO operands inline (no + // materialise-to-heap defeat) and returns the result NaN-boxed — + // SSO when the total fits 5 bytes, heap-pointer otherwise. Saves up + // to 3 heap allocations per concat on hot paths like ABC451D's + // recursive `before + after` (1.4M concats with 1-9 byte operands). + Ok(blk.call( + DOUBLE, + "js_string_concat_box", + &[(DOUBLE, &values[0]), (DOUBLE, &values[1])], + )) + }) +} + +/// Cap the per-call part count for the n-way fold. Must match the +/// runtime's `MAX_PARTS` in `js_string_concat_chain`. 32 covers every +/// realistic CSV / log-line / template chain in user code. +const CONCAT_CHAIN_MAX_PARTS: usize = 32; + +/// Try to flatten a left-spine of `Binary { Add }` nodes where every Add +/// has at least one statically-string operand. Returns the parts in +/// left-to-right (source-order) order. Returns `None` if the chain is +/// shorter than the existing pairwise fast path's preference, has too +/// many parts, or contains an Add node where neither side is statically +/// string (which would risk numeric semantics under JS spec). +/// +/// Caller passes the OUTERMOST Add's children. If the outermost Add's +/// left child is itself a string-shaped Add, we recurse into it; right +/// children are always leaves in our flat representation. +pub(crate) fn flatten_string_add_chain<'a>( + ctx: &FnCtx<'_>, + left: &'a Expr, + right: &'a Expr, +) -> Option> { + use perry_hir::BinaryOp; + + let mut parts: Vec<&Expr> = Vec::with_capacity(8); + parts.push(right); + + // Walk down the left spine. At each step, the current `cur` was the + // left child of an Add we already accepted — so we know `cur + ...` + // is string-shaped at the level above. We need each Add we descend + // INTO to itself be string-shaped (≥1 statically-string operand), so + // the entire chain has unambiguous string semantics. + let mut cur: &Expr = left; + loop { + match cur { + Expr::Binary { + op: BinaryOp::Add, + left: l, + right: r, + } => { + let l_str = crate::type_analysis::is_definitely_string_expr(ctx, l); + let r_str = crate::type_analysis::is_definitely_string_expr(ctx, r); + if !l_str && !r_str { + // Stop the descent — this Add isn't unambiguously + // string-shaped. Treat the entire `cur` subtree as + // one opaque part. + parts.push(cur); + break; + } + parts.push(r); + cur = l; + if parts.len() >= CONCAT_CHAIN_MAX_PARTS { + return None; + } + } + _ => { + parts.push(cur); + break; + } + } + } + + parts.reverse(); + Some(parts) +} + +/// Lower a flat parts list to a single `js_string_concat_chain` call. +/// Each part is lowered to its NaN-boxed value, then stored into a +/// stack-allocated `[CONCAT_CHAIN_MAX_PARTS x double]` buffer; we pass +/// the base pointer + N to the runtime helper, which produces a single +/// allocation containing the entire concatenated result. +/// +/// The buffer is fixed-size (always sized to MAX_PARTS) and hoisted to +/// the function entry block via `alloca_entry_array`. A non-entry-block +/// alloca lowers to a runtime `sub %rsp, N` with no matching restore; +/// inside a loop body that's a stack leak (issue #167 — same shape that +/// blew up `buf.readInt32BE` in tight loops). Function-entry allocas +/// run once at prologue and the slot dominates every reachable use. +/// One per-function buffer is shared across all chain call sites — fine +/// because each chain call writes its parts and immediately calls into +/// the runtime helper before any other call site can clobber the slots. +pub(crate) fn lower_string_concat_chain(ctx: &mut FnCtx<'_>, parts: &[&Expr]) -> Result { + debug_assert!(parts.len() >= 2); + debug_assert!(parts.len() <= CONCAT_CHAIN_MAX_PARTS); + + // Lower each part first (in source order); side effects must fire + // left-to-right per JS spec. #6951: that ordering is exactly what makes + // every earlier part a heap value in an SSA register across every later + // part's evaluation — this is the template-literal / log-line shape, and + // one allocating interpolation was enough to sweep the parts already + // lowered. Parts that nothing allocating follows emit no rooting calls. + with_operands_rooted(ctx, parts, |ctx, lowered| { + let n = lowered.len(); + // Hoist the buffer to the function entry block. Issue #167. + let buf_reg = ctx.func.alloca_entry_array(DOUBLE, CONCAT_CHAIN_MAX_PARTS); + let blk = ctx.block(); + for (i, val) in lowered.iter().enumerate() { + let slot = blk.gep(DOUBLE, &buf_reg, &[(I64, &format!("{}", i))]); + blk.store(DOUBLE, val, &slot); + } + // Pass the array's base pointer as i64 (codegen ABI uses i64 for + // raw pointer args matching the existing `js_string_concat` shape). + let base_i64 = blk.next_reg(); + blk.emit_raw(format!("{} = ptrtoint ptr {} to i64", base_i64, buf_reg)); + + let result_handle = blk.call( + I64, + "js_string_concat_chain", + &[(I64, &base_i64), (I32, &format!("{}", n))], + ); + Ok(nanbox_string_inline(blk, &result_handle)) + }) +} diff --git a/crates/perry-codegen/src/lower_string_method.rs b/crates/perry-codegen/src/lower_string_method.rs index f34db80620..900d20ec73 100644 --- a/crates/perry-codegen/src/lower_string_method.rs +++ b/crates/perry-codegen/src/lower_string_method.rs @@ -3,19 +3,19 @@ //! Contains `lower_string_method`, `lower_string_self_append`, //! `lower_string_coerce_concat`, and `lower_string_concat`. -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Result}; use perry_hir::types::Type as HirType; use perry_hir::Expr; -use crate::expr::temp_root::{ - self, lower_exprs_rooted, lower_operand_pair_rooted, temp_root_get_double, temp_root_get_i64, - temp_root_push_double, temp_root_push_i64, temp_root_release, temp_root_set_i64, - temp_root_truncate, -}; use crate::expr::{ i32_bool_to_nanbox, lower_expr, nanbox_pointer_inline, nanbox_string_inline, unbox_str_handle, FnCtx, }; +use crate::lower_string_concat::str_operand_handle_tag_dispatched; +use crate::rooting::{ + open_rooted_group, operand_may_collect, with_rooted_accumulator, Arg, EmittedValue, Repr, + RootedGroup, +}; use crate::type_analysis::is_string_expr; mod char_code_at; @@ -166,9 +166,17 @@ pub(crate) fn lower_string_method( // `gc::root_words` bare form covers. Root it across the whole dispatch; // the truncate below is the single release point for every one of the // match's ~60 return paths. - let args_can_collect = args.iter().any(|a| temp_root::expr_may_trigger_gc(ctx, a)); - let recv_root = args_can_collect.then(|| temp_root_push_double(ctx, &recv_box)); - let result = lower_string_method_dispatch(ctx, object, property, args, &recv_box, &recv_root); + let args_can_collect = args.iter().any(|a| operand_may_collect(ctx, a)); + // #7615 slice 8: the ESCAPING group form. The release has to post-dominate + // ~60 return paths inside the dispatch, which no closure form can own + // without swallowing the whole 1,100-line match — the same argument + // `open_rooted_group`'s doc makes for `func_ref.rs`'s dispatch diamonds. + // What the group buys over the raw slot index it replaces is that a caller + // holding it cannot truncate at the wrong slot, in the wrong order, or + // twice: it never sees an index. + let mut group = open_rooted_group(1); + let recv = group.adopt_emitted(ctx, Repr::Boxed, &recv_box, args_can_collect); + let result = lower_string_method_dispatch(ctx, object, property, args, &recv_box, &group, recv); // Released only after the dispatch's consuming runtime call has run: that // call allocates while it reads the receiver. // @@ -190,10 +198,13 @@ pub(crate) fn lower_string_method( // defense-in-depth: emitting after a terminator is never correct, and the // check is free. Reachability of the arm from a TypeScript source remains // unproven; see the PR body. - if let Some(idx) = &recv_root { - if !ctx.block().is_terminated() { - temp_root_truncate(ctx, idx); - } + // + // Not releasing on the terminated path is the SAFE half of a mis-managed + // guard (`open_rooted_group`'s doc): over-retention, not a dangling + // pointer. `release` on an empty group — the `args_can_collect == false` + // case — emits nothing, so the guard alone decides. + if !ctx.block().is_terminated() { + group.release(ctx); } result } @@ -204,14 +215,8 @@ pub(crate) fn lower_string_method( /// evacuating cycle rewrites it and the register pushed beforehand is stale. /// Returns the original register when nothing was rooted, emitting no IR — so a /// method whose arguments cannot collect keeps its previous code byte for byte. -fn reread_recv(ctx: &mut FnCtx<'_>, recv_root: &Option, recv_box: &str) -> String { - match recv_root { - Some(idx) => { - let idx = idx.clone(); - temp_root_get_double(ctx, &idx) - } - None => recv_box.to_string(), - } +fn reread_recv(ctx: &mut FnCtx<'_>, group: &RootedGroup<'_>, recv: EmittedValue) -> String { + group.reread_emitted(ctx, recv) } #[allow(clippy::too_many_lines)] @@ -221,7 +226,8 @@ fn lower_string_method_dispatch( property: &str, args: &[Expr], recv_box: &str, - recv_root: &Option, + group: &RootedGroup<'_>, + recv: EmittedValue, ) -> Result { let recv_box = recv_box.to_string(); match property { @@ -250,7 +256,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let needle_handle = if needle_is_str { @@ -299,7 +305,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // String length (i32 at header offset 0). Used as the default end @@ -352,7 +358,7 @@ fn lower_string_method_dispatch( // its coercion/undefined/RegExp checks for this common hot path. if args.len() == 1 && matches!(&args[0], Expr::String(_) | Expr::WtfString(_)) { let delim_box = lower_expr(ctx, &args[0])?; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let delim_handle = unbox_str_handle(blk, &delim_box); @@ -381,7 +387,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // No separator → pass `undefined`, which `js_string_split_value` @@ -426,7 +432,7 @@ fn lower_string_method_dispatch( } else { Some(lower_expr(ctx, &args[0])?) }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let locales_box = match locales_box { Some(v) => v, @@ -453,7 +459,7 @@ fn lower_string_method_dispatch( for extra in args.iter() { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let runtime_fn = match property { @@ -473,7 +479,7 @@ fn lower_string_method_dispatch( for extra in args.iter() { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let runtime_fn = match property { @@ -502,7 +508,7 @@ fn lower_string_method_dispatch( for extra in args.iter().skip(1) { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let value_handle = blk.call(I64, "js_string_coerce", &[(DOUBLE, &value_d)]); @@ -535,7 +541,7 @@ fn lower_string_method_dispatch( for extra in args.iter().skip(1) { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let idx_i32 = blk.call(I32, "js_string_index_to_i32", &[(DOUBLE, &idx_d)]); @@ -554,7 +560,7 @@ fn lower_string_method_dispatch( ); } let count_d = lower_expr(ctx, &args[0])?; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let result = blk.call( @@ -596,7 +602,7 @@ fn lower_string_method_dispatch( let repl_is_str = is_string_expr(ctx, &args[1]); let needle_box = lower_expr(ctx, &args[0])?; let repl_box = lower_expr(ctx, &args[1])?; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // #4871: a `searchValue` codegen can't type (an object-property @@ -720,7 +726,7 @@ fn lower_string_method_dispatch( for extra in args.iter().skip(1) { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let recv_handle = str_operand_handle_tag_dispatched(ctx, object, &recv_box); let blk = ctx.block(); let idx_i32 = blk.call(I32, "js_string_index_to_i32", &[(DOUBLE, &idx_d)]); @@ -742,7 +748,7 @@ fn lower_string_method_dispatch( for extra in args.iter().skip(1) { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let recv_handle = str_operand_handle_tag_dispatched(ctx, object, &recv_box); let blk = ctx.block(); let idx_i32 = blk.call(I32, "js_string_index_to_i32", &[(DOUBLE, &idx_d)]); @@ -764,7 +770,7 @@ fn lower_string_method_dispatch( for extra in args.iter().skip(1) { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); if let Some(value) = lower_char_code_at_inline(ctx, object, &recv_box, &idx_d) { return Ok(value); } @@ -805,7 +811,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let needle_handle = if needle_is_str { @@ -874,7 +880,7 @@ fn lower_string_method_dispatch( let sp_box = blk.load(DOUBLE, &sp_global); unbox_str_handle(blk, &sp_box) }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // Pass `target_length` as raw DOUBLE — the runtime does the @@ -911,7 +917,7 @@ fn lower_string_method_dispatch( } form }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let result = blk.call( @@ -972,7 +978,7 @@ fn lower_string_method_dispatch( &[(DOUBLE, loc), (DOUBLE, opts_ref)], ); } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // A non-string `that` (undefined/number/object) must be @@ -1015,7 +1021,7 @@ fn lower_string_method_dispatch( } else { crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let i32_v = blk.call( @@ -1040,7 +1046,7 @@ fn lower_string_method_dispatch( } else { crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let result = blk.call( @@ -1070,7 +1076,7 @@ fn lower_string_method_dispatch( } else { crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let result = blk.call( @@ -1086,7 +1092,7 @@ fn lower_string_method_dispatch( for extra in args.iter() { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // Returns a NaN-tagged boolean directly. @@ -1097,7 +1103,7 @@ fn lower_string_method_dispatch( for extra in args.iter() { let _ = lower_expr(ctx, extra)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let result = blk.call(I64, "js_string_to_well_formed", &[(I64, &recv_handle)]); @@ -1116,45 +1122,43 @@ fn lower_string_method_dispatch( // `reread_recv` above cannot help: it has to be rooted in its own // right and written back after every concat, because each iteration // produces a NEW address. - let mut acc_handle = { + let acc_handle = { let blk = ctx.block(); unbox_str_handle(blk, &recv_box) }; - let args_can_collect = args.iter().any(|a| temp_root::expr_may_trigger_gc(ctx, a)); - let acc_root = args_can_collect.then(|| temp_root_push_i64(ctx, &acc_handle)); - for a in args { - let a_is_str = is_string_expr(ctx, a); - let s_box = lower_expr(ctx, a)?; - // The ToString coercion allocates too, so re-read only after it. - let s_handle = { - let blk = ctx.block(); - if a_is_str { - unbox_str_handle(blk, &s_box) - } else { - blk.call(I64, "js_string_coerce", &[(DOUBLE, &s_box)]) + let args_can_collect = args.iter().any(|a| operand_may_collect(ctx, a)); + // #7615 slice 8: `RootedAcc::advance` IS the "re-read, call, write + // the new address back" triple this arm spelled out by hand, and it + // exists for this exact reason (its doc names `js_string_concat`'s + // sibling `js_array_push_f64`). The accumulator never becomes a + // register the loop holds: `advance` materialises argument 0 from + // the slot as part of emitting the call. + with_rooted_accumulator( + ctx, + Repr::Ptr, + &acc_handle, + args_can_collect, + |ctx, acc| { + for a in args { + let a_is_str = is_string_expr(ctx, a); + let s_box = lower_expr(ctx, a)?; + // The ToString coercion allocates too, so re-read only after it. + let s_handle = { + let blk = ctx.block(); + if a_is_str { + unbox_str_handle(blk, &s_box) + } else { + blk.call(I64, "js_string_coerce", &[(DOUBLE, &s_box)]) + } + }; + // Write the new accumulator back, so the NEXT argument's + // lowering keeps *this* string alive rather than its input. + acc.advance(ctx, "js_string_concat", &[Arg::Plain(I64, &s_handle)]); } - }; - if let Some(idx) = &acc_root { - let idx = idx.clone(); - acc_handle = temp_root_get_i64(ctx, &idx); - } - acc_handle = ctx.block().call( - I64, - "js_string_concat", - &[(I64, &acc_handle), (I64, &s_handle)], - ); - // Write the new accumulator back, so the NEXT argument's - // lowering keeps *this* string alive rather than its input. - if let Some(idx) = &acc_root { - let idx = idx.clone(); - temp_root_set_i64(ctx, &idx, &acc_handle); - } - } - let boxed = nanbox_string_inline(ctx.block(), &acc_handle); - if let Some(idx) = &acc_root { - temp_root_truncate(ctx, idx); - } - Ok(boxed) + Ok(()) + }, + |ctx, handle| Ok(nanbox_string_inline(ctx.block(), handle)), + ) } "substr" => { // Legacy substr(start, length) — distinct from substring/slice: @@ -1175,7 +1179,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); // Pass the raw NaN-boxed args straight through: `js_string_substr` @@ -1211,7 +1215,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let method_id = regexp_search_method_id(property); @@ -1272,7 +1276,7 @@ fn lower_string_method_dispatch( } else { None }; - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let recv_handle = unbox_str_handle(blk, &recv_box); let method_id = regexp_search_method_id(property); @@ -1313,7 +1317,7 @@ fn lower_string_method_dispatch( for a in args { let _ = lower_expr(ctx, a)?; } - let recv_box = reread_recv(ctx, recv_root, &recv_box); + let recv_box = reread_recv(ctx, group, recv); let blk = ctx.block(); let handle = blk.call(I64, "js_jsvalue_to_string", &[(DOUBLE, &recv_box)]); Ok(nanbox_string_inline(blk, &handle)) @@ -1368,590 +1372,3 @@ fn lower_string_method_dispatch( } } } - -/// Lower the `str = str + rhs` self-append pattern. Uses the in-place -/// `js_string_append` runtime function (refcount=1 → mutate in place, -/// otherwise allocate). The returned pointer is stored back to the local -/// slot — `js_string_append` may realloc when growing past capacity. -/// -/// This is the load-bearing optimization for the canonical `let str = ""; -/// for (...) str = str + "a"` string-build pattern. -pub(crate) fn lower_string_self_append( - ctx: &mut FnCtx<'_>, - local_id: u32, - rhs: &Expr, -) -> Result { - let slot = ctx - .locals - .get(&local_id) - .ok_or_else(|| anyhow!("string self-append: local {} not in scope", local_id))? - .clone(); - - // Representation-selection Phase 3a: canonical-Str destination — - // tag-dispatch on the slot bits inline instead of paying the two opaque - // `js_get_string_pointer_unified` calls per iteration. - if crate::expr::local_is_canonical_str(ctx, local_id) { - return lower_canonical_str_self_append(ctx, local_id, rhs, &slot); - } - - // Lower the RHS first (might be a string literal, a local, or a - // computed expression). For non-string RHS we'd need to coerce, but - // the bench_string_ops case always uses a string literal, so for the - // first slice we require the RHS to be a known string. - if !is_string_expr(ctx, rhs) { - // Fall back to the slower concat path: load the local, do a - // generic concat-coerce, store back. - let lhs_val = ctx.block().load(DOUBLE, &slot); - let _lhs = lhs_val.clone(); - let rhs_val = lower_expr(ctx, rhs)?; - let blk = ctx.block(); - // Issue #214: SSO-safe unbox. - let l_handle = unbox_str_handle(blk, &lhs_val); - // Coerce non-string RHS to a string handle. - let r_handle = blk.call(I64, "js_jsvalue_to_string", &[(DOUBLE, &rhs_val)]); - let result = blk.call( - I64, - "js_string_append", - &[(I64, &l_handle), (I64, &r_handle)], - ); - let new_box = nanbox_string_inline(blk, &result); - blk.store(DOUBLE, &new_box, &slot); - return Ok(new_box); - } - - let rhs_box = lower_expr(ctx, rhs)?; - let blk = ctx.block(); - let lhs_box = blk.load(DOUBLE, &slot); - // Issue #214: SSO-safe unbox. - let l_handle = unbox_str_handle(blk, &lhs_box); - let r_handle = unbox_str_handle(blk, &rhs_box); - let new_handle = blk.call( - I64, - "js_string_append", - &[(I64, &l_handle), (I64, &r_handle)], - ); - let new_box = nanbox_string_inline(blk, &new_handle); - blk.store(DOUBLE, &new_box, &slot); - Ok(new_box) -} - -/// Repsel Phase 3a: is this expression PROVEN to lower to a heap-tagged -/// (`STRING_TAG`) NaN-box — never SSO bits, never a non-string? String -/// literals load the interned pool handle (`@.str.N.handle`, always a heap -/// `StringHeader` from `js_string_from_bytes`); `String(x)` routes through -/// `js_string_coerce`, which always allocates a heap header. Deliberately -/// NOT included: `Binary Add` string results — the pairwise concat lowering -/// returns `js_string_concat_box`, which assembles ≤5-byte ASCII results as -/// SSO bits. -fn proven_heap_string_operand(_ctx: &FnCtx<'_>, e: &Expr) -> bool { - match e { - Expr::String(_) | Expr::WtfString(_) | Expr::StringCoerce(_) => true, - Expr::Conditional { - then_expr, - else_expr, - .. - } => { - proven_heap_string_operand(_ctx, then_expr) - && proven_heap_string_operand(_ctx, else_expr) - } - _ => false, - } -} - -/// Repsel Phase 3a: operand → raw `StringHeader*` handle for the string -/// helpers, tag-dispatched: -/// -/// - proven heap-tagged operand (see `proven_heap_string_operand`) → inline -/// `bitcast; and POINTER_MASK` — zero calls; -/// - canonical-Str `LocalGet` → 2-arm dispatch: heap `STRING_TAG` bits → -/// bare `and POINTER_MASK` (hot arm, no call); anything else (SSO bits, -/// annotation lie) → the legacy `js_get_string_pointer_unified` (which -/// materializes SSO — cold); -/// - everything else (or flag off) → the legacy unified call, unchanged. -/// -/// #7128: the two arms are on separate knobs, because only the second one is -/// about a selected representation. The proven-heap arm keys on the operand's -/// static type and fires with zero canonical-`Str` locals in the program. -fn str_operand_handle_tag_dispatched(ctx: &mut FnCtx<'_>, object: &Expr, recv_box: &str) -> String { - use crate::nanbox::POINTER_MASK_I64; - if crate::expr::static_string_lowering_enabled() && proven_heap_string_operand(ctx, object) { - let bits = ctx.block().bitcast_double_to_i64(recv_box); - return ctx.block().and(I64, &bits, POINTER_MASK_I64); - } - let canonical = crate::expr::canonical_str_locals_enabled() - && matches!( - object, Expr::LocalGet(id) if crate::expr::local_is_canonical_str(ctx, *id) - ); - if !canonical { - return unbox_str_handle(ctx.block(), recv_box); - } - let bits = ctx.block().bitcast_double_to_i64(recv_box); - let tag = ctx.block().lshr(I64, &bits, "48"); - let is_heap = ctx - .block() - .icmp_eq(I64, &tag, crate::nanbox::STRING_TAG_TOP16_I64); - - let heap_idx = ctx.new_block("strrecv.heap"); - let cold_idx = ctx.new_block("strrecv.cold"); - let merge_idx = ctx.new_block("strrecv.merge"); - let heap_label = ctx.block_label(heap_idx); - let cold_label = ctx.block_label(cold_idx); - let merge_label = ctx.block_label(merge_idx); - ctx.block().cond_br(&is_heap, &heap_label, &cold_label); - - ctx.current_block = heap_idx; - let h_heap = ctx.block().and(I64, &bits, POINTER_MASK_I64); - let heap_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = cold_idx; - let h_cold = unbox_str_handle(ctx.block(), recv_box); - let cold_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = merge_idx; - ctx.block() - .phi(I64, &[(&h_heap, &heap_pred), (&h_cold, &cold_pred)]) -} - -/// Representation-selection Phase 3a: `s += rhs` for a canonical-Str -/// destination (`SlotRep::Str` — the `ctx.locals` slot provably holds -/// NaN-box string bits at rest). Replaces the two opaque -/// `js_get_string_pointer_unified` calls per iteration with an inline tag -/// dispatch on the slot bits: -/// -/// - **both heap** (`STRING_TAG` on both sides): `and POINTER_MASK` → -/// `js_string_append(h, h)` → `or STRING_TAG` — the hot accumulator-loop -/// arm; keeps the refcount==1 in-place append (every alias demote site is -/// untouched by this phase, so `let b = a` still demotes first). -/// - **both strings, SSO involved**: `js_string_concat_box(box, box)` — -/// SSO-aware pairwise concat, assembles ≤5-byte ASCII results inline and -/// never mutates in place. No per-op heap materialization of SSO bits -/// (RFC §4 "short-string values stay by-value"). -/// - **anything else** (a lying `string` annotation): the exact pre-phase -/// sequence — `js_get_string_pointer_unified` ×2 (SSO materialize + -/// number coercion included) → `js_string_append` — so acceptance -/// behavior is bit-identical to today's on non-string bits (RFC §5.5: -/// mismatches route to the legacy path, never a new coercion). -fn lower_canonical_str_self_append( - ctx: &mut FnCtx<'_>, - _local_id: u32, - rhs: &Expr, - slot: &str, -) -> Result { - use crate::nanbox::{ - POINTER_MASK_I64, SHORT_STRING_TAG_TOP16_I64 as TAG_SSO_STR, - STRING_TAG_TOP16_I64 as TAG_HEAP_STR, - }; - - if !is_string_expr(ctx, rhs) { - // Non-string rhs: mirror the legacy fallback's evaluation order - // (lhs slot load, then rhs), coerce the rhs once (heap handle - // guaranteed), then 2-arm on the destination tag only. - let lhs_box = ctx.block().load(DOUBLE, slot); - // #6951: the load must happen before `rhs` per `s += rhs` evaluation - // order (a `rhs` that reassigns `s` must not be observed here), so the - // pre-rhs value has to be carried across `rhs`'s evaluation and the - // `js_jsvalue_to_string` coercion — both of which allocate. Re-reading - // the slot would take the wrong value; re-read the temp root instead. - let lhs_root = temp_root_push_double(ctx, &lhs_box); - let rhs_val = lower_expr(ctx, rhs)?; - let r_handle = ctx - .block() - .call(I64, "js_jsvalue_to_string", &[(DOUBLE, &rhs_val)]); - // The coerced rhs is a bare string handle that has to survive the cold - // arm's `unbox_str_handle`, which materializes an SSO destination onto - // the heap — another allocation. Root it too and re-read it per arm. - let r_root = temp_root_push_i64(ctx, &r_handle); - let lhs_box = temp_root_get_double(ctx, &lhs_root); - let bits_d = ctx.block().bitcast_double_to_i64(&lhs_box); - let tag_d = ctx.block().lshr(I64, &bits_d, "48"); - let is_heap = ctx.block().icmp_eq(I64, &tag_d, TAG_HEAP_STR); - - let heap_idx = ctx.new_block("strapp.heap"); - let cold_idx = ctx.new_block("strapp.cold"); - let merge_idx = ctx.new_block("strapp.merge"); - let heap_label = ctx.block_label(heap_idx); - let cold_label = ctx.block_label(cold_idx); - let merge_label = ctx.block_label(merge_idx); - ctx.block().cond_br(&is_heap, &heap_label, &cold_label); - - ctx.current_block = heap_idx; - let h_d = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); - let r_heap = temp_root_get_i64(ctx, &r_root); - let h_heap = ctx - .block() - .call(I64, "js_string_append", &[(I64, &h_d), (I64, &r_heap)]); - let heap_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = cold_idx; - let h_d2 = unbox_str_handle(ctx.block(), &lhs_box); - let r_cold = temp_root_get_i64(ctx, &r_root); - let h_cold = ctx - .block() - .call(I64, "js_string_append", &[(I64, &h_d2), (I64, &r_cold)]); - let cold_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = merge_idx; - let handle = ctx - .block() - .phi(I64, &[(&h_heap, &heap_pred), (&h_cold, &cold_pred)]); - let new_box = nanbox_string_inline(ctx.block(), &handle); - ctx.block().store(DOUBLE, &new_box, slot); - // `lhs_root` is the base of the pair, so one truncate drops both. The - // index register is defined in the entry block and dominates the merge. - temp_root_truncate(ctx, &lhs_root); - return Ok(new_box); - } - - // Proven-string rhs: mirror the legacy fast path's evaluation order - // (rhs first, then the lhs slot load). - // - // Arm layout — the load-bearing property is that a HEAP destination - // ALWAYS reaches `js_string_append` (whose refcount==1 in-place path is - // what makes accumulator loops amortized O(n)). Routing a heap-dest / - // SSO-rhs iteration through `js_string_concat_box` instead would copy - // the whole accumulator every time a ≤5-byte part arrives — O(n²). - // - // dest heap, rhs heap → append(h, h) (hot, no calls) - // dest heap, rhs other → append(h, unified(rhs)) (legacy-exact: - // unified materializes SSO / coerces a lie) - // dest SSO → js_string_concat_box (SSO-aware, - // nothing to mutate in place; result may stay - // SSO — no per-op heap materialization) - // dest other (lie) → unified ×2 + append (legacy-exact) - let rhs_box = lower_expr(ctx, rhs)?; - let lhs_box = ctx.block().load(DOUBLE, slot); - let bits_d = ctx.block().bitcast_double_to_i64(&lhs_box); - let bits_r = ctx.block().bitcast_double_to_i64(&rhs_box); - let tag_d = ctx.block().lshr(I64, &bits_d, "48"); - let tag_r = ctx.block().lshr(I64, &bits_r, "48"); - let d_heap = ctx.block().icmp_eq(I64, &tag_d, TAG_HEAP_STR); - - let dheap_idx = ctx.new_block("strapp.dheap"); - let heap_idx = ctx.new_block("strapp.heap"); - let rcold_idx = ctx.new_block("strapp.rcold"); - let dother_idx = ctx.new_block("strapp.dother"); - let sso_idx = ctx.new_block("strapp.sso"); - let cold_idx = ctx.new_block("strapp.cold"); - let merge_idx = ctx.new_block("strapp.merge"); - let dheap_label = ctx.block_label(dheap_idx); - let heap_label = ctx.block_label(heap_idx); - let rcold_label = ctx.block_label(rcold_idx); - let dother_label = ctx.block_label(dother_idx); - let sso_label = ctx.block_label(sso_idx); - let cold_label = ctx.block_label(cold_idx); - let merge_label = ctx.block_label(merge_idx); - ctx.block().cond_br(&d_heap, &dheap_label, &dother_label); - - // dest heap: split on the rhs tag. - ctx.current_block = dheap_idx; - let r_heap = ctx.block().icmp_eq(I64, &tag_r, TAG_HEAP_STR); - ctx.block().cond_br(&r_heap, &heap_label, &rcold_label); - - ctx.current_block = heap_idx; - let h_d = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); - let h_r = ctx.block().and(I64, &bits_r, POINTER_MASK_I64); - let h_new = ctx - .block() - .call(I64, "js_string_append", &[(I64, &h_d), (I64, &h_r)]); - let box_heap = nanbox_string_inline(ctx.block(), &h_new); - let heap_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = rcold_idx; - let h_d1 = ctx.block().and(I64, &bits_d, POINTER_MASK_I64); - let r_h1 = unbox_str_handle(ctx.block(), &rhs_box); - let h_rc = ctx - .block() - .call(I64, "js_string_append", &[(I64, &h_d1), (I64, &r_h1)]); - let box_rcold = nanbox_string_inline(ctx.block(), &h_rc); - let rcold_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - // dest not heap: an SSO dest with a real-string rhs takes the SSO-aware - // pairwise concat; a lie on EITHER side keeps the exact legacy sequence - // (`js_string_concat_box` treats a non-string operand as empty, but the - // legacy unified path ToString-coerces it — `"ab" += 42` must stay - // `"ab42"`). - ctx.current_block = dother_idx; - let d_sso = ctx.block().icmp_eq(I64, &tag_d, TAG_SSO_STR); - let r_heap2 = ctx.block().icmp_eq(I64, &tag_r, TAG_HEAP_STR); - let r_sso = ctx.block().icmp_eq(I64, &tag_r, TAG_SSO_STR); - let r_str = ctx.block().or(I1, &r_heap2, &r_sso); - let take_sso = ctx.block().and(I1, &d_sso, &r_str); - ctx.block().cond_br(&take_sso, &sso_label, &cold_label); - - ctx.current_block = sso_idx; - let box_sso = ctx.block().call( - DOUBLE, - "js_string_concat_box", - &[(DOUBLE, &lhs_box), (DOUBLE, &rhs_box)], - ); - let sso_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = cold_idx; - let l_h = unbox_str_handle(ctx.block(), &lhs_box); - let r_h = unbox_str_handle(ctx.block(), &rhs_box); - let h_cold = ctx - .block() - .call(I64, "js_string_append", &[(I64, &l_h), (I64, &r_h)]); - let box_cold = nanbox_string_inline(ctx.block(), &h_cold); - let cold_pred = ctx.block().label.clone(); - ctx.block().br(&merge_label); - - ctx.current_block = merge_idx; - let new_box = ctx.block().phi( - DOUBLE, - &[ - (&box_heap, &heap_pred), - (&box_rcold, &rcold_pred), - (&box_sso, &sso_pred), - (&box_cold, &cold_pred), - ], - ); - ctx.block().store(DOUBLE, &new_box, slot); - Ok(new_box) -} - -/// Lower `string + non_string` (or vice versa) concat with runtime -/// coercion of the non-string side. The non-string operand passes through -/// `js_jsvalue_to_string` which inspects its NaN tag and produces the -/// canonical JS string form (numbers via the formatter at -/// `crates/perry-runtime/src/value.rs:825`, booleans → `"true"`/`"false"`, -/// objects → `"[object Object]"`, etc.). -/// -/// The string-typed side still uses the fast inline `bitcast double → i64; -/// and POINTER_MASK_I64` unbox; only the non-string side pays the function -/// call. Both operand handles then feed `js_string_concat`. -pub(crate) fn lower_string_coerce_concat( - ctx: &mut FnCtx<'_>, - left: &Expr, - right: &Expr, - l_is_string: bool, - r_is_string: bool, -) -> Result { - // #6951: `l_box` is a heap string in an SSA register while `right` is - // lowered. If `right` allocates (`"tag" + f()`), a collection sweeps the - // left operand and the concat reads freed memory — a segfault, not a - // dropped character. `lower_operand_pair_rooted` emits nothing at all when - // `right` provably cannot collect, which is the common `"user_" + i` case. - let (l_box, r_box, guard) = lower_operand_pair_rooted(ctx, left, right)?; - - // Issue #58: fused string+value concat — when one side is a string - // and the other is not, use the fused runtime call that collapses - // js_jsvalue_to_string + js_string_concat into a single allocation - // for number operands (the common `"item_" + i` pattern). - if l_is_string && !r_is_string { - // Issue #214: SSO-safe unbox; repsel Phase 3a: inline `bitcast+and` - // for proven-heap operands (string literals — the `"user_" + i` - // shape) and tag-dispatch for canonical-Str locals. - let l_handle = str_operand_handle_tag_dispatched(ctx, left, &l_box); - let blk = ctx.block(); - let result_handle = blk.call( - I64, - "js_string_concat_value", - &[(I64, &l_handle), (DOUBLE, &r_box)], - ); - let boxed = nanbox_string_inline(blk, &result_handle); - temp_root_release(ctx, guard); - return Ok(boxed); - } - - if !l_is_string && r_is_string { - // Issue #214: SSO-safe unbox; repsel Phase 3a: see above. - let r_handle = str_operand_handle_tag_dispatched(ctx, right, &r_box); - let blk = ctx.block(); - let result_handle = blk.call( - I64, - "js_value_concat_string", - &[(DOUBLE, &l_box), (I64, &r_handle)], - ); - let boxed = nanbox_string_inline(blk, &result_handle); - temp_root_release(ctx, guard); - return Ok(boxed); - } - - // Both non-string (shouldn't normally reach here) — fall back to - // the generic path. - let l_handle = ctx - .block() - .call(I64, "js_jsvalue_to_string", &[(DOUBLE, &l_box)]); - // The coercion of the right operand allocates, and `l_handle` is a bare - // string address in an SSA register — root it across that call (#6951). - let l_root = temp_root_push_i64(ctx, &l_handle); - let r_handle = ctx - .block() - .call(I64, "js_jsvalue_to_string", &[(DOUBLE, &r_box)]); - let l_handle = temp_root_get_i64(ctx, &l_root); - let blk = ctx.block(); - - let result_handle = blk.call( - I64, - "js_string_concat", - &[(I64, &l_handle), (I64, &r_handle)], - ); - let boxed = nanbox_string_inline(blk, &result_handle); - temp_root_truncate(ctx, &l_root); - temp_root_release(ctx, guard); - Ok(boxed) -} - -/// Lower a static `s1 + s2` string concatenation. Both operands must -/// already be statically string-typed (caller's responsibility — see -/// `is_string_expr`). -/// -/// Pattern: -/// ```llvm -/// ; %l_box and %r_box are NaN-boxed strings (double values with STRING_TAG) -/// %l_bits = bitcast double %l_box to i64 -/// %l_handle = and i64 %l_bits, 281474976710655 ; POINTER_MASK_I64 -/// %r_bits = bitcast double %r_box to i64 -/// %r_handle = and i64 %r_bits, 281474976710655 -/// %result_handle = call i64 @js_string_concat(i64 %l_handle, i64 %r_handle) -/// %result_box = call double @js_nanbox_string(i64 %result_handle) -/// ``` -/// -/// The bitcast+and is the inline-fast unboxing pattern. We avoid calling -/// the slower `js_nanbox_get_pointer` (which does the same thing in Rust) -/// to keep concat hot-path overhead minimal. -pub(crate) fn lower_string_concat( - ctx: &mut FnCtx<'_>, - left: &Expr, - right: &Expr, -) -> Result { - // #6951: same hazard as `lower_string_coerce_concat` — the left operand is - // a heap string in an SSA register across the right operand's evaluation. - let (l_box, r_box, guard) = lower_operand_pair_rooted(ctx, left, right)?; - let blk = ctx.block(); - // SSO-aware fast path: pass operands as NaN-boxed f64s directly to - // `js_string_concat_sso`, which keeps SSO operands inline (no - // materialise-to-heap defeat) and returns the result NaN-boxed — - // SSO when the total fits 5 bytes, heap-pointer otherwise. Saves up - // to 3 heap allocations per concat on hot paths like ABC451D's - // recursive `before + after` (1.4M concats with 1-9 byte operands). - let result = blk.call( - DOUBLE, - "js_string_concat_box", - &[(DOUBLE, &l_box), (DOUBLE, &r_box)], - ); - temp_root_release(ctx, guard); - Ok(result) -} - -/// Cap the per-call part count for the n-way fold. Must match the -/// runtime's `MAX_PARTS` in `js_string_concat_chain`. 32 covers every -/// realistic CSV / log-line / template chain in user code. -const CONCAT_CHAIN_MAX_PARTS: usize = 32; - -/// Try to flatten a left-spine of `Binary { Add }` nodes where every Add -/// has at least one statically-string operand. Returns the parts in -/// left-to-right (source-order) order. Returns `None` if the chain is -/// shorter than the existing pairwise fast path's preference, has too -/// many parts, or contains an Add node where neither side is statically -/// string (which would risk numeric semantics under JS spec). -/// -/// Caller passes the OUTERMOST Add's children. If the outermost Add's -/// left child is itself a string-shaped Add, we recurse into it; right -/// children are always leaves in our flat representation. -pub(crate) fn flatten_string_add_chain<'a>( - ctx: &FnCtx<'_>, - left: &'a Expr, - right: &'a Expr, -) -> Option> { - use perry_hir::BinaryOp; - - let mut parts: Vec<&Expr> = Vec::with_capacity(8); - parts.push(right); - - // Walk down the left spine. At each step, the current `cur` was the - // left child of an Add we already accepted — so we know `cur + ...` - // is string-shaped at the level above. We need each Add we descend - // INTO to itself be string-shaped (≥1 statically-string operand), so - // the entire chain has unambiguous string semantics. - let mut cur: &Expr = left; - loop { - match cur { - Expr::Binary { - op: BinaryOp::Add, - left: l, - right: r, - } => { - let l_str = crate::type_analysis::is_definitely_string_expr(ctx, l); - let r_str = crate::type_analysis::is_definitely_string_expr(ctx, r); - if !l_str && !r_str { - // Stop the descent — this Add isn't unambiguously - // string-shaped. Treat the entire `cur` subtree as - // one opaque part. - parts.push(cur); - break; - } - parts.push(r); - cur = l; - if parts.len() >= CONCAT_CHAIN_MAX_PARTS { - return None; - } - } - _ => { - parts.push(cur); - break; - } - } - } - - parts.reverse(); - Some(parts) -} - -/// Lower a flat parts list to a single `js_string_concat_chain` call. -/// Each part is lowered to its NaN-boxed value, then stored into a -/// stack-allocated `[CONCAT_CHAIN_MAX_PARTS x double]` buffer; we pass -/// the base pointer + N to the runtime helper, which produces a single -/// allocation containing the entire concatenated result. -/// -/// The buffer is fixed-size (always sized to MAX_PARTS) and hoisted to -/// the function entry block via `alloca_entry_array`. A non-entry-block -/// alloca lowers to a runtime `sub %rsp, N` with no matching restore; -/// inside a loop body that's a stack leak (issue #167 — same shape that -/// blew up `buf.readInt32BE` in tight loops). Function-entry allocas -/// run once at prologue and the slot dominates every reachable use. -/// One per-function buffer is shared across all chain call sites — fine -/// because each chain call writes its parts and immediately calls into -/// the runtime helper before any other call site can clobber the slots. -pub(crate) fn lower_string_concat_chain(ctx: &mut FnCtx<'_>, parts: &[&Expr]) -> Result { - debug_assert!(parts.len() >= 2); - debug_assert!(parts.len() <= CONCAT_CHAIN_MAX_PARTS); - - // Lower each part first (in source order); side effects must fire - // left-to-right per JS spec. #6951: that ordering is exactly what makes - // every earlier part a heap value in an SSA register across every later - // part's evaluation — this is the template-literal / log-line shape, and - // one allocating interpolation was enough to sweep the parts already - // lowered. Parts that nothing allocating follows emit no rooting calls. - let (lowered, guard) = lower_exprs_rooted(ctx, parts)?; - - let n = lowered.len(); - // Hoist the buffer to the function entry block. Issue #167. - let buf_reg = ctx.func.alloca_entry_array(DOUBLE, CONCAT_CHAIN_MAX_PARTS); - let blk = ctx.block(); - for (i, val) in lowered.iter().enumerate() { - let slot = blk.gep(DOUBLE, &buf_reg, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, val, &slot); - } - // Pass the array's base pointer as i64 (codegen ABI uses i64 for - // raw pointer args matching the existing `js_string_concat` shape). - let base_i64 = blk.next_reg(); - blk.emit_raw(format!("{} = ptrtoint ptr {} to i64", base_i64, buf_reg)); - - let result_handle = blk.call( - I64, - "js_string_concat_chain", - &[(I64, &base_i64), (I32, &format!("{}", n))], - ); - let boxed = nanbox_string_inline(blk, &result_handle); - temp_root_release(ctx, guard); - Ok(boxed) -} diff --git a/crates/perry-codegen/src/lower_string_method/char_code_at.rs b/crates/perry-codegen/src/lower_string_method/char_code_at.rs index 26a9809adc..2e1f6417e6 100644 --- a/crates/perry-codegen/src/lower_string_method/char_code_at.rs +++ b/crates/perry-codegen/src/lower_string_method/char_code_at.rs @@ -10,7 +10,7 @@ use crate::expr::FnCtx; use crate::type_analysis::is_string_expr; use crate::types::{DOUBLE, I1, I32, I64}; -use super::str_operand_handle_tag_dispatched; +use crate::lower_string_concat::str_operand_handle_tag_dispatched; // `StringHeader` layout the fast path reads, from // `crates/perry-runtime/src/string/mod.rs`. That struct is `#[repr(C)]` with diff --git a/crates/perry-codegen/src/root_reload.rs b/crates/perry-codegen/src/root_reload.rs index da68bd9638..a2ad681e7e 100644 --- a/crates/perry-codegen/src/root_reload.rs +++ b/crates/perry-codegen/src/root_reload.rs @@ -50,7 +50,7 @@ //! //! # Why re-loading is sound, and when it is not //! -//! Re-reading a slot is NOT unconditionally safe, and `expr/temp_root.rs`'s +//! Re-reading a slot is NOT unconditionally safe, and `rooting/temp_root.rs`'s //! [`operand_is_reloadable`] documents exactly why: re-lowering a local reads //! its value *now*, and "now" is after the later arguments have run — any of //! which may have reassigned it. `new C(g, bump())` where `bump()` assigns `g` @@ -148,6 +148,7 @@ //! reload — and the re-derived `bitcast`/`and` above it are pure and fold. //! //! [`operand_is_reloadable`]: crate::expr::temp_root +//! [`operand_is_reloadable`]: crate::rooting use std::collections::{HashMap, HashSet, VecDeque}; @@ -1236,7 +1237,7 @@ mod tests { // ★ The soundness half. `f(x, (x = other, 1))` must pass the ORIGINAL // `x`; re-reading the slot below the assignment would hand the consumer // the new value, which is a miscompile and not a rooting fix. See - // `expr::temp_root::operand_is_reloadable`. + // `rooting::operand_is_reloadable`. let mut f = LlFunction::new("t", DOUBLE, vec![(DOUBLE, "%arg".into())]); let b = f.create_block("entry"); let slot = b.alloca(DOUBLE); diff --git a/crates/perry-codegen/src/rooting.rs b/crates/perry-codegen/src/rooting/mod.rs similarity index 76% rename from crates/perry-codegen/src/rooting.rs rename to crates/perry-codegen/src/rooting/mod.rs index c819279072..efc15960ab 100644 --- a/crates/perry-codegen/src/rooting.rs +++ b/crates/perry-codegen/src/rooting/mod.rs @@ -322,6 +322,30 @@ mod tests { // and the API is the only thing that makes the correct form the easy one. // --------------------------------------------------------------------------- +/// The raw, order-sensitive rooting API. +/// +/// **PRIVATE, and that is the campaign's terminal condition** (#7615). It used +/// to be `crate::expr::temp_root`, reachable from anywhere in the crate, and +/// every bug in the #7341 family was an ordering mistake against it: a push +/// below the collection point (#7192), a truncate at the wrong slot, a release +/// on one arm of an `if` (#7462), a re-read taken above the window (#7114). +/// +/// A private module inside `rooting` makes each of those unwritable outside +/// this file rather than merely uncounted — the ledger below can only report +/// what a module NAMES, and a module that cannot name it has nothing to +/// report. The accessors additionally carry an explicit +/// `pub(in crate::rooting)`, so re-opening the module in a moment of haste +/// does not silently widen them back. +/// +/// Two items keep `pub(crate)` and are re-exported below, because they are not +/// accessors and make no ordering decision: [`TempRootPool`] is the +/// compile-time slot bookkeeping `FnCtx` owns, and `expr_is_inert_primitive` +/// is the "can evaluating this run user code?" predicate the loop back-edge +/// poll shares (`crate::loop_purity`). +mod temp_root; + +pub(crate) use temp_root::{expr_is_inert_primitive, TempRootPool}; + use anyhow::Result; use perry_hir::Expr; @@ -378,7 +402,7 @@ impl RootedSlot { /// every slot acquired after it. Release in reverse acquisition order, as /// the un-migrated callers already had to. pub(crate) fn release(self, ctx: &mut FnCtx<'_>) { - crate::expr::temp_root::temp_root_truncate(ctx, &self.idx); + temp_root::temp_root_truncate(ctx, &self.idx); } } @@ -418,8 +442,8 @@ fn materialize<'a>(ctx: &mut FnCtx<'_>, args: &'a [Arg<'a>]) -> Vec<(LlvmType, S /// public path out of it fuses the read to the emission that consumes it. fn read_slot(ctx: &mut FnCtx<'_>, slot: &RootedSlot) -> String { match slot.repr { - Repr::Ptr => crate::expr::temp_root::temp_root_get_i64(ctx, &slot.idx), - Repr::Boxed => crate::expr::temp_root::temp_root_get_double(ctx, &slot.idx), + Repr::Ptr => temp_root::temp_root_get_i64(ctx, &slot.idx), + Repr::Boxed => temp_root::temp_root_get_double(ctx, &slot.idx), } } @@ -443,7 +467,7 @@ pub(crate) fn call_rooted( let reg = ctx .block() .call(ret_ty, callee, &borrow_args(&materialized)); - let idx = crate::expr::temp_root::temp_root_push_i64(ctx, ®); + let idx = temp_root::temp_root_push_i64(ctx, ®); RootedSlot { idx, repr: Repr::Ptr, @@ -535,8 +559,7 @@ pub(crate) fn with_operands_rooted_across<'f, T, R>( across: impl FnOnce(&mut FnCtx<'f>) -> Result, body: impl FnOnce(&mut FnCtx<'f>, &[String], T) -> Result, ) -> Result { - let across_collects = - crate::expr::temp_root::any_may_trigger_gc(ctx, across_exprs.iter().copied()); + let across_collects = temp_root::any_may_trigger_gc(ctx, across_exprs.iter().copied()); with_operands_rooted_window(ctx, exprs, across_collects, across, body) } @@ -584,7 +607,7 @@ fn with_operands_rooted_window<'f, T, R>( across: impl FnOnce(&mut FnCtx<'f>) -> Result, body: impl FnOnce(&mut FnCtx<'f>, &[String], T) -> Result, ) -> Result { - use crate::expr::temp_root::{any_may_trigger_gc, root_operands_begin}; + use temp_root::{any_may_trigger_gc, root_operands_begin}; let mut group = root_operands_begin(exprs.len()); let out = (|| { @@ -619,7 +642,7 @@ pub(crate) fn any_operand_may_collect<'a>( ctx: &FnCtx<'_>, exprs: impl IntoIterator, ) -> bool { - crate::expr::temp_root::any_may_trigger_gc(ctx, exprs) + temp_root::any_may_trigger_gc(ctx, exprs) } /// [`any_operand_may_collect`] for a single expression. @@ -630,7 +653,7 @@ pub(crate) fn any_operand_may_collect<'a>( /// arguments after it plus an allocating rebind. One `collects` for the whole /// list cannot say that. pub(crate) fn operand_may_collect(ctx: &FnCtx<'_>, expr: &Expr) -> bool { - crate::expr::temp_root::expr_may_trigger_gc(ctx, expr) + temp_root::expr_may_trigger_gc(ctx, expr) } // --------------------------------------------------------------------------- @@ -706,15 +729,29 @@ pub(crate) fn operand_may_collect(ctx: &FnCtx<'_>, expr: &Expr) -> bool { /// which the generic dynamic call needs, because its callee operand is a /// hand-emitted by-name property read rather than `lower_expr(callee)`. pub(crate) struct RootedGroup<'a> { - operands: crate::expr::temp_root::RootedOperands, + operands: temp_root::RootedOperands, exprs: Vec<&'a Expr>, accs: Vec, - emitted: Vec, + emitted: Vec, /// The LOWEST slot this group pushed, of either kind. One truncate at it /// drops the whole scope, because a truncate is a stack cut. first_slot: Option, } +/// What [`RootedGroup::adopt_emitted`] recorded for one emitted value. +/// +/// The `Reused` arm is the `protect == false` answer, and it exists for the +/// same reason [`RootedAcc`]'s `value` field does: a site whose window +/// provably cannot collect must keep the IR it had before it was rooted at +/// all, register numbering included. It is NOT a third protection strategy — +/// `operand_protection`'s `Reload` is still unavailable here (re-emitting the +/// producing call would call it twice) and `Reuse`-across-a-real-window is +/// still the bug. It only records that there was no window. +enum EmittedRoot { + Rooted(RootedSlot), + Reused(String), +} + /// A handle on one **emitted** value inside a [`RootedGroup`] — /// see [`RootedGroup::adopt_emitted`]. /// @@ -743,7 +780,7 @@ pub(crate) struct AccArray(usize); impl<'a> RootedGroup<'a> { fn new(capacity: usize) -> Self { RootedGroup { - operands: crate::expr::temp_root::root_operands_begin(capacity), + operands: temp_root::root_operands_begin(capacity), exprs: Vec::with_capacity(capacity), accs: Vec::new(), emitted: Vec::new(), @@ -822,6 +859,17 @@ impl<'a> RootedGroup<'a> { self.exprs.len() } + /// True when this group actually pushed a slot. + /// + /// The signal a caller uses to keep an eager unbox — and therefore its + /// exact register numbering — on the unprotected path, exactly as + /// `RootedOperands::is_rooted` served `math_simple.rs`'s `MapSet` before + /// the migration. It reports whether a slot exists, never which one, so it + /// cannot be turned into a release. + pub(crate) fn is_rooted(&self) -> bool { + self.first_slot.is_some() + } + /// Root a value that an **emitted step** produced, rather than one lowered /// from an expression. /// @@ -864,25 +912,58 @@ impl<'a> RootedGroup<'a> { /// [`with_rooted_accumulator`], which has taken a caller-produced `initial` /// since slice 3. Call this on the line below the emission that produced /// the value. + /// + /// # `protect` states the WINDOW, not the strategy (slice 8) + /// + /// Slice 7 shipped this without a flag and said so: "there is no flag, and + /// a caller cannot pick the wrong one". That claim was about the + /// *strategy* — `Reload` and `Reuse` are unavailable for an emitted value + /// on principle — and it still holds. `protect` answers the other + /// question, the one every combinator in this file already takes from its + /// caller in some form: **does anything between here and the last use + /// collect?** [`with_rooted_accumulator`] has taken it as `protect` since + /// slice 3, `RootedGroup::lower`/`adopt` take it as `collects`, and + /// `with_operands_rooted_across_call` hardcodes it to `true`. + /// + /// Slice 8 needed it for `expr/static_field_meta.rs`: a `ClassExprFresh` + /// with no statics, no captures, no symbol statics and no `static { … }` + /// block emits nothing at all between the class object's allocation and + /// the `nanbox_pointer_inline` that returns it, and that shape is + /// reachable (`lower_decl/body_stmt.rs`'s `fresh_binding` arm builds one + /// with three empty vectors). `protect == false` emits no push, no + /// re-reads and no truncate, so it keeps the pre-rooting IR byte for byte + /// — the same contract `rooted_handle_begin(ctx, h, false)` had. pub(crate) fn adopt_emitted( &mut self, ctx: &mut FnCtx<'_>, repr: Repr, value: &str, + protect: bool, ) -> EmittedValue { - let idx = match repr { - Repr::Ptr => crate::expr::temp_root::temp_root_push_i64(ctx, value), - Repr::Boxed => crate::expr::temp_root::temp_root_push_double(ctx, value), + let root = if protect { + let idx = match repr { + Repr::Ptr => temp_root::temp_root_push_i64(ctx, value), + Repr::Boxed => temp_root::temp_root_push_double(ctx, value), + }; + self.note_slot(Some(idx.clone())); + EmittedRoot::Rooted(RootedSlot { idx, repr }) + } else { + EmittedRoot::Reused(value.to_string()) }; - self.note_slot(Some(idx.clone())); - self.emitted.push(RootedSlot { idx, repr }); + self.emitted.push(root); EmittedValue(self.emitted.len() - 1) } /// Re-read an [`adopt_emitted`](RootedGroup::adopt_emitted) value **here**, /// in the representation it was pushed with. + /// + /// An unprotected value hands its original register back and emits + /// nothing, exactly as `RootedOperands::reread_one`'s `Reuse` arm does. pub(crate) fn reread_emitted(&self, ctx: &mut FnCtx<'_>, value: EmittedValue) -> String { - read_slot(ctx, &self.emitted[value.0]) + match &self.emitted[value.0] { + EmittedRoot::Rooted(slot) => read_slot(ctx, slot), + EmittedRoot::Reused(reg) => reg.clone(), + } } /// Allocate an argument-accumulator array of capacity `cap` and root it in @@ -895,7 +976,7 @@ impl<'a> RootedGroup<'a> { /// it lives in the group so that ONE release drops the operands and the /// arrays together. pub(crate) fn begin_array(&mut self, ctx: &mut FnCtx<'_>, cap: &str) -> AccArray { - let slot = crate::expr::temp_root::rooted_array_begin(ctx, cap); + let slot = temp_root::rooted_array_begin(ctx, cap); self.note_slot(Some(slot.clone())); self.accs.push(slot); AccArray(self.accs.len() - 1) @@ -905,20 +986,20 @@ impl<'a> RootedGroup<'a> { /// possibly-reallocated pointer back into it. pub(crate) fn push_array(&mut self, ctx: &mut FnCtx<'_>, acc: AccArray, value: &str) { let slot = self.accs[acc.0].clone(); - crate::expr::temp_root::temp_rooted_array_push(ctx, &slot, value); + temp_root::temp_rooted_array_push(ctx, &slot, value); } /// Re-read the finished array as a raw `i64` pointer. Does not release: the /// consuming call allocates while it reads the array. pub(crate) fn read_array(&self, ctx: &mut FnCtx<'_>, acc: AccArray) -> String { let slot = self.accs[acc.0].clone(); - crate::expr::temp_root::rooted_array_read(ctx, &slot) + temp_root::rooted_array_read(ctx, &slot) } /// Drop the whole scope. Call it *after* the consuming call: the consumer /// allocates while reading these values. pub(crate) fn release(self, ctx: &mut FnCtx<'_>) { - crate::expr::temp_root::temp_root_release(ctx, self.first_slot); + temp_root::temp_root_release(ctx, self.first_slot); } } @@ -995,7 +1076,7 @@ pub(crate) fn implicit_this_save(ctx: &mut FnCtx<'_>, new_this: &str) -> Implici let prev = ctx .block() .call(DOUBLE, "js_implicit_this_set", &[(DOUBLE, new_this)]); - let idx = crate::expr::temp_root::temp_root_push_double(ctx, &prev); + let idx = temp_root::temp_root_push_double(ctx, &prev); ImplicitThisSave { slot: RootedSlot { idx, @@ -1046,7 +1127,7 @@ pub(crate) struct NewTargetSave { /// Set `new.target` to `new_target` and root the value it displaced. pub(crate) fn new_target_save(ctx: &mut FnCtx<'_>, new_target: &str) -> NewTargetSave { let prev = ctx.block().call(DOUBLE, "js_new_target_get", &[]); - let idx = crate::expr::temp_root::temp_root_push_double(ctx, &prev); + let idx = temp_root::temp_root_push_double(ctx, &prev); ctx.block() .call(DOUBLE, "js_new_target_set", &[(DOUBLE, new_target)]); NewTargetSave { @@ -1151,12 +1232,8 @@ impl RootedAcc { pub(crate) fn advance(&mut self, ctx: &mut FnCtx<'_>, callee: &str, rest: &[Arg<'_>]) { let next = self.call(ctx, self.repr.llvm_ty(), callee, rest); match (&self.slot, self.repr) { - (Some(slot), Repr::Ptr) => { - crate::expr::temp_root::temp_root_set_i64(ctx, &slot.idx, &next) - } - (Some(slot), Repr::Boxed) => { - crate::expr::temp_root::temp_root_set_double(ctx, &slot.idx, &next) - } + (Some(slot), Repr::Ptr) => temp_root::temp_root_set_i64(ctx, &slot.idx, &next), + (Some(slot), Repr::Boxed) => temp_root::temp_root_set_double(ctx, &slot.idx, &next), (None, _) => self.value = next, } } @@ -1184,8 +1261,8 @@ pub(crate) fn with_rooted_accumulator<'f, R>( ) -> Result { let slot = protect.then(|| { let idx = match repr { - Repr::Ptr => crate::expr::temp_root::temp_root_push_i64(ctx, initial), - Repr::Boxed => crate::expr::temp_root::temp_root_push_double(ctx, initial), + Repr::Ptr => temp_root::temp_root_push_i64(ctx, initial), + Repr::Boxed => temp_root::temp_root_push_double(ctx, initial), }; RootedSlot { idx, repr } }); @@ -1390,122 +1467,294 @@ pub(crate) fn with_rooted_accumulator<'f, R>( /// serve and the note on what it weakens. Which is the shape of the answer this /// campaign keeps arriving at: an API gap recorded in slice N is a combinator /// in slice N+1, and writing the gap down is what makes the next slice cheap. +/// +/// # Slice 8 — the campaign's last slice, and its terminal condition +/// +/// The plan's terminal condition was `expr/temp_root.rs` "going +/// `pub(in crate::rooting)` — the raw accessor unreachable, not merely +/// uncounted". As literally spelled that is **not expressible in Rust**: +/// `pub(in path)` requires `path` to be an ANCESTOR module of the item +/// (E0742), and `crate::rooting` is not an ancestor of `crate::expr::temp_root`. +/// So the file MOVED — it is `crate::rooting::temp_root` now, declared with a +/// private `mod temp_root;` and with every accessor additionally carrying an +/// explicit `pub(in crate::rooting)`. Either alone would do it; both are here +/// because the module declaration is one keyword away from re-widening +/// twenty-five items at once. +/// +/// Two items keep `pub(crate)` and are re-exported at the top of this file, +/// and neither is an accessor: [`TempRootPool`] is the compile-time slot +/// bookkeeping `FnCtx` owns (no runtime behaviour, no ordering), and +/// `expr_is_inert_primitive` is the shared "can evaluating this run user +/// code?" predicate the loop back-edge poll consults +/// (`crate::loop_purity`). A predicate cannot be called in the wrong order. +/// +/// **Fourteen items were DELETED rather than narrowed**, because slice 8 left +/// them with no caller at all: `lower_exprs_rooted`, +/// `lower_operand_pair_rooted`, `any_later_ref_may_trigger_gc`, +/// `RootedOperands::is_rooted`, the whole `StoreOperandGuard` family +/// (`guard_store_operand`, `guard_store_operand_across`, +/// `reread_store_operand`, `release_store_operand`), the whole `RootedHandle` +/// family (`rooted_handle_begin`/`_get`/`_release`) and +/// `temp_root_scope_begin`/`_end`. CLAUDE.md's kill-policy is explicit that +/// "the losing mode should stop compiling", and each of these WAS a losing +/// mode: a caller-managed guard whose combinator replacement owns the release. +/// +/// ## Modules migrated, and how they were told apart from the decision-free ones +/// +/// The brief for this slice listed 14 files by `expr::temp_root` mention. +/// That count conflates two populations, and the ledger is only meaningful for +/// one of them. Sorting them is the first half of the work: +/// +/// **Eight modules made rooting decisions and are listed below.** Seven are +/// load-bearing on the committed source (each named the raw API before the +/// migration): +/// +/// * `expr/binary.rs` — five `lower_operand_pair_rooted` + +/// `temp_root_release` pairs, one per dynamic-dispatch arm, each with the +/// release on its own `return` path. They collapse into one +/// `lower_rooted_dynamic_binary` helper over [`with_operands_rooted`]: +/// five chances to misplace a release become none. +/// * `expr/math_simple.rs` — `Expr::MapSet` is a [`RootedGroup`] (two +/// operands with UNEQUAL windows, re-read at eight arm-specific points, +/// released once); `MapGet`/`MapHas` are the plain single-re-read shape. +/// `Expr::ArrayMap` is a live bug, below. +/// * `expr/static_field_meta.rs` — `ClassExprFresh` is a [`RootedGroup`] +/// over the class object with a nested [`with_rooted_accumulator`] for the +/// `__perry_ctor_caps` snapshot array and a nested +/// [`with_operands_rooted`] per symbol static. +/// * `expr/dyn_extern_i18n.rs` — the namespace-object build (#7280's +/// 269-member zod case) is exactly [`with_rooted_accumulator`]'s shape. +/// * `lower_string_method.rs` — the receiver root that spans ~60 return +/// paths, via [`open_rooted_group`]. +/// * `lower_string_concat.rs` — split out of `lower_string_method.rs` this +/// slice; load-bearing because the code it contains named +/// `lower_exprs_rooted`, `lower_operand_pair_rooted` and four raw +/// push/get/truncate/release spellings on `main`. +/// * `lower_call/new.rs` — `refresh_rooted_args` re-reads one operand group +/// at three caller-chosen points under a scope marker spanning ~20 return +/// paths. Slice 5 named it as the shape the API could not express and +/// slice 6 built [`RootedGroup`] for exactly it; this is the collection. +/// +/// The eighth, `lower_call/new_alloc.rs`, is **vacuous on the committed +/// source** — it never named the raw API, because it is the instance +/// allocation carved out of `new.rs` this slice and everything it emits sits +/// above the instance root. It is listed anyway, for the reason slice 3 listed +/// `array_push.rs`: an unlisted sibling of a listed module is the obvious +/// place to put a raw push and escape the check. Its listing means something +/// only because the sabotage arm was run on it. +/// +/// **Nine files mention the raw API and make no rooting decision at all.** +/// They are deliberately NOT listed, because a ledger line on a module that +/// never had a decision to make looks substantive and asserts nothing: +/// +/// * `expr/mod.rs` — declared `mod temp_root` and typed the `temp_roots` +/// field. Structural; both are gone with the move. +/// * `codegen/entry.rs`, `codegen/method.rs`, `codegen/function.rs`, +/// `codegen/closure.rs` — `TempRootPool::default()` at each `FnCtx` +/// construction. Constructing the pool is not using it. +/// * `stmt/loops.rs` — one call to `expr_is_inert_primitive`, a purity +/// predicate for the back-edge poll. +/// * `loop_purity.rs` — a doc link and nothing else (zero code sites; the +/// brief's count included the comment). +/// * `root_reload.rs`, `gc_call_effects.rs`, `runtime_decls/arrays.rs` — +/// the STRING LITERALS `"js_gc_temp_root_push"` and friends, which name +/// runtime symbols, not this module. So do five test files +/// (`expr/slice7_rooting_tests.rs`, `lower_call/console_rooting_tests.rs`, +/// `lower_call/timer_rooting_tests.rs`, +/// `codegen/testing_feature_gate_tests.rs`) plus +/// `linker_temp_lifecycle_tests.rs`, whose `temp_root_if_clang_available` +/// is about a temporary DIRECTORY. +/// +/// ## The three unverified leads slice 7 handed over +/// +/// * `static_field_meta.rs`'s `caps_arr` — **a real accumulator shape with a +/// provably empty window.** The array holds the only reference to +/// everything pushed so far while the next element is lowered, which is +/// #6951 exactly; but `captured_args` is built at one site +/// (`lower/lower_expr/arm_class.rs`) as +/// `ids.iter().map(|id| Expr::LocalGet(*id))`, and `expr_may_trigger_gc` +/// answers `false` for every `LocalGet`. So it is rooted through +/// [`with_rooted_accumulator`] with `protect` computed rather than +/// assumed: today that is `false` and the IR is byte for byte unchanged, +/// and the day a non-inert expression reaches the list it is rooted by +/// construction. +/// * `math_simple.rs`'s `ArrayMap` — **CONFIRMED live.** The receiver was +/// lowered, `callback` was lowered, and only then was the receiver +/// unboxed: `unbox_to_i64` below its own window masks a stale box rather +/// than repairing it (#7280 taxonomy (c)). `arr.map(x => …)` allocates a +/// closure at minimum. Fixed via [`with_operands_rooted`]. +/// * `dyn_extern_i18n.rs`'s `path_handle` — **DISMISSED, and the premise is +/// wrong about the CFG.** The lead says the raw handle is reused across a +/// compare loop "that runs module `__init` bodies". It does not: each +/// `__init()` is emitted into that iteration's MATCH block, which +/// branches straight to the join, so no `__init` dominates any later use +/// of `path_handle`. Along the fallthrough chain the only emissions +/// between the handle's production and its last use are +/// `js_get_string_pointer_unified` and `js_string_equals` — neither +/// re-enters user code nor enumerates an object, which is the standard +/// [`with_operands_rooted_across_call`]'s doc sets for an emitted step +/// (#7198). What that module DID have was the namespace-object +/// accumulator, which is migrated above. #[cfg(test)] const MIGRATED_MODULES: &[(&str, &str)] = &[ ( "crates/perry-codegen/src/expr/url_main.rs", - include_str!("expr/url_main.rs"), + include_str!("../expr/url_main.rs"), ), ( "crates/perry-codegen/src/lower_array_method.rs", - include_str!("lower_array_method.rs"), + include_str!("../lower_array_method.rs"), ), ( "crates/perry-codegen/src/expr/arrays_finds.rs", - include_str!("expr/arrays_finds.rs"), + include_str!("../expr/arrays_finds.rs"), ), ( "crates/perry-codegen/src/expr/array_methods.rs", - include_str!("expr/array_methods.rs"), + include_str!("../expr/array_methods.rs"), ), ( "crates/perry-codegen/src/expr/instance_misc1.rs", - include_str!("expr/instance_misc1.rs"), + include_str!("../expr/instance_misc1.rs"), ), ( "crates/perry-codegen/src/expr/logical_collections.rs", - include_str!("expr/logical_collections.rs"), + include_str!("../expr/logical_collections.rs"), ), ( "crates/perry-codegen/src/lower_call/property_get/map_set.rs", - include_str!("lower_call/property_get/map_set.rs"), + include_str!("../lower_call/property_get/map_set.rs"), ), ( "crates/perry-codegen/src/expr/objects_arrays_lit.rs", - include_str!("expr/objects_arrays_lit.rs"), + include_str!("../expr/objects_arrays_lit.rs"), ), ( "crates/perry-codegen/src/expr/array_literal.rs", - include_str!("expr/array_literal.rs"), + include_str!("../expr/array_literal.rs"), ), ( "crates/perry-codegen/src/expr/object_literal.rs", - include_str!("expr/object_literal.rs"), + include_str!("../expr/object_literal.rs"), ), ( "crates/perry-codegen/src/expr/array_push.rs", - include_str!("expr/array_push.rs"), + include_str!("../expr/array_push.rs"), ), ( "crates/perry-codegen/src/expr/index_get.rs", - include_str!("expr/index_get.rs"), + include_str!("../expr/index_get.rs"), ), ( "crates/perry-codegen/src/expr/index_get/guarded_array.rs", - include_str!("expr/index_get/guarded_array.rs"), + include_str!("../expr/index_get/guarded_array.rs"), ), ( "crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs", - include_str!("expr/index_get/inline_dyn_typed_array.rs"), + include_str!("../expr/index_get/inline_dyn_typed_array.rs"), ), ( "crates/perry-codegen/src/expr/index_set.rs", - include_str!("expr/index_set.rs"), + include_str!("../expr/index_set.rs"), ), ( "crates/perry-codegen/src/expr/index_set_typed_array.rs", - include_str!("expr/index_set_typed_array.rs"), + include_str!("../expr/index_set_typed_array.rs"), ), ( "crates/perry-codegen/src/expr/property_get.rs", - include_str!("expr/property_get.rs"), + include_str!("../expr/property_get.rs"), ), ( "crates/perry-codegen/src/expr/property_get/globalget.rs", - include_str!("expr/property_get/globalget.rs"), + include_str!("../expr/property_get/globalget.rs"), ), ( "crates/perry-codegen/src/expr/property_get/helpers.rs", - include_str!("expr/property_get/helpers.rs"), + include_str!("../expr/property_get/helpers.rs"), ), ( "crates/perry-codegen/src/expr/property_set.rs", - include_str!("expr/property_set.rs"), + include_str!("../expr/property_set.rs"), ), ( "crates/perry-codegen/src/lower_call/extern_timers.rs", - include_str!("lower_call/extern_timers.rs"), + include_str!("../lower_call/extern_timers.rs"), ), ( "crates/perry-codegen/src/lower_call/namespace_call.rs", - include_str!("lower_call/namespace_call.rs"), + include_str!("../lower_call/namespace_call.rs"), ), ( "crates/perry-codegen/src/lower_call/mod.rs", - include_str!("lower_call/mod.rs"), + include_str!("../lower_call/mod.rs"), ), ( "crates/perry-codegen/src/lower_call/func_ref.rs", - include_str!("lower_call/func_ref.rs"), + include_str!("../lower_call/func_ref.rs"), ), ( "crates/perry-codegen/src/lower_call/console_promise.rs", - include_str!("lower_call/console_promise.rs"), + include_str!("../lower_call/console_promise.rs"), ), ( "crates/perry-codegen/src/expr/child_proc.rs", - include_str!("expr/child_proc.rs"), + include_str!("../expr/child_proc.rs"), ), ( "crates/perry-codegen/src/expr/proxy_reflect.rs", - include_str!("expr/proxy_reflect.rs"), + include_str!("../expr/proxy_reflect.rs"), ), ( "crates/perry-codegen/src/expr/fs_await.rs", - include_str!("expr/fs_await.rs"), + include_str!("../expr/fs_await.rs"), + ), + ( + "crates/perry-codegen/src/expr/binary.rs", + include_str!("../expr/binary.rs"), + ), + ( + "crates/perry-codegen/src/expr/math_simple.rs", + include_str!("../expr/math_simple.rs"), + ), + ( + "crates/perry-codegen/src/expr/static_field_meta.rs", + include_str!("../expr/static_field_meta.rs"), + ), + ( + "crates/perry-codegen/src/expr/dyn_extern_i18n.rs", + include_str!("../expr/dyn_extern_i18n.rs"), + ), + ( + "crates/perry-codegen/src/lower_string_method.rs", + include_str!("../lower_string_method.rs"), + ), + ( + "crates/perry-codegen/src/lower_string_concat.rs", + include_str!("../lower_string_concat.rs"), + ), + ( + "crates/perry-codegen/src/lower_call/new.rs", + include_str!("../lower_call/new.rs"), + ), + ( + "crates/perry-codegen/src/lower_call/new_alloc.rs", + include_str!("../lower_call/new_alloc.rs"), ), ]; +/// `rooting/temp_root.rs`, inlined at compile time for the terminal-condition +/// test below. +#[cfg(test)] +const RAW_ROOTING_API_SRC: &str = include_str!("temp_root.rs"); + +/// The two items in `temp_root.rs` that are allowed to stay `pub(crate)`. +/// +/// Neither is an accessor. Adding to this list is how the campaign's terminal +/// condition would be given back, so it is spelled out rather than derived. +#[cfg(test)] +const RAW_API_PUBLIC_EXCEPTIONS: &[&str] = &["struct TempRootPool", "fn expr_is_inert_primitive"]; + /// Lines in `src` that reach past [`crate::rooting`] into the raw rooting API. #[cfg(test)] fn escape_hatch_uses(src: &str) -> Vec<(usize, String)> { @@ -1521,7 +1770,102 @@ fn escape_hatch_uses(src: &str) -> Vec<(usize, String)> { #[cfg(test)] mod migration_ledger { - use super::{escape_hatch_uses, MIGRATED_MODULES}; + use super::{ + escape_hatch_uses, MIGRATED_MODULES, RAW_API_PUBLIC_EXCEPTIONS, RAW_ROOTING_API_SRC, + }; + + /// Every `pub`-ish item declared in `temp_root.rs`, as + /// `("pub(crate)" | "pub(in crate::rooting)" | "pub", "fn name")`. + fn declared_items(src: &str) -> Vec<(String, String)> { + src.lines() + .filter_map(|line| { + let code = line.split("//").next().unwrap_or(line).trim_start(); + for vis in ["pub(in crate::rooting) ", "pub(crate) ", "pub "] { + if let Some(rest) = code.strip_prefix(vis) { + let mut it = rest.split_whitespace(); + let kind = it.next()?; + if !matches!(kind, "fn" | "struct" | "enum" | "mod" | "const" | "type") { + return None; + } + let name = it.next()?.split(['(', '<', '{', ':']).next()?.to_string(); + return Some((vis.trim().to_string(), format!("{kind} {name}"))); + } + } + None + }) + .collect() + } + + /// **The campaign's terminal condition** (#7615): the raw rooting API is + /// unreachable outside `crate::rooting`, not merely unnamed. + /// + /// The ledger above can only report what a module NAMES, which is why this + /// is a separate assertion rather than a stronger phrasing of that one. + /// Both halves are checked, because either alone can be undone by one + /// keyword: the module declaration must stay private, and every accessor + /// must carry `pub(in crate::rooting)` so re-opening the module does not + /// silently widen twenty-five items at once. + #[test] + fn the_raw_rooting_api_is_unreachable_outside_this_module() { + let items = declared_items(RAW_ROOTING_API_SRC); + assert!( + items.len() > 15, + "expected temp_root.rs to declare the raw API; found {} items — the \ + include_str! target moved and this check is measuring nothing", + items.len() + ); + let widened: Vec<&(String, String)> = items + .iter() + .filter(|(vis, item)| { + vis != "pub(in crate::rooting)" && !RAW_API_PUBLIC_EXCEPTIONS.contains(&&**item) + }) + .collect(); + assert!( + widened.is_empty(), + "the Layer 1 campaign's terminal condition is that every accessor in \ + rooting/temp_root.rs is pub(in crate::rooting). These are not, and \ + are not on the two-item exception list:\n{}", + widened + .iter() + .map(|(vis, item)| format!(" {vis} {item}")) + .collect::>() + .join("\n") + ); + + let decl = include_str!("mod.rs"); + assert!( + decl.contains("\nmod temp_root;\n"), + "rooting/temp_root.rs must be declared with a PRIVATE `mod temp_root;` — \ + a pub(crate) module would make every item in it reachable crate-wide \ + regardless of its own visibility" + ); + } + + /// Sabotage duty for the terminal-condition check: a widened accessor must + /// be reported, and the two allowed exceptions must not be. + #[test] + fn the_terminal_condition_check_reports_a_widened_accessor() { + let planted = "\ +pub(crate) fn temp_root_push_i64(ctx: &mut FnCtx<'_>, v: &str) -> String {} +pub(in crate::rooting) fn temp_root_truncate(ctx: &mut FnCtx<'_>, idx: &str) {} +pub(crate) struct TempRootPool {} +pub(crate) fn expr_is_inert_primitive(ctx: &FnCtx<'_>, e: &Expr) -> bool {} +"; + let items = declared_items(planted); + assert_eq!(items.len(), 4, "parsed {items:?}"); + let widened: Vec<&(String, String)> = items + .iter() + .filter(|(vis, item)| { + vis != "pub(in crate::rooting)" && !RAW_API_PUBLIC_EXCEPTIONS.contains(&&**item) + }) + .collect(); + assert_eq!( + widened.len(), + 1, + "exactly the widened accessor must be reported, got {widened:?}" + ); + assert_eq!(widened[0].1, "fn temp_root_push_i64"); + } /// An empty ledger passes vacuously, which is hazard 4 in CLAUDE.md applied /// to this test. Assert the subject exists before asserting it is clean. diff --git a/crates/perry-codegen/src/expr/temp_root.rs b/crates/perry-codegen/src/rooting/temp_root.rs similarity index 70% rename from crates/perry-codegen/src/expr/temp_root.rs rename to crates/perry-codegen/src/rooting/temp_root.rs index b8ff6c059b..73f186cb38 100644 --- a/crates/perry-codegen/src/expr/temp_root.rs +++ b/crates/perry-codegen/src/rooting/temp_root.rs @@ -50,7 +50,7 @@ use perry_hir::Expr; use crate::types::{DOUBLE, I32, I64}; -use super::FnCtx; +use crate::expr::FnCtx; /// Per-function pool of frame-rooted temp allocas (#7469). /// @@ -155,12 +155,12 @@ fn temp_pool_acquire(ctx: &mut FnCtx<'_>) -> Option { fn temp_slot_store(ctx: &mut FnCtx<'_>, handle: &str, value_i64: &str) { ctx.block().store(I64, value_i64, handle); let idx = ctx.temp_roots.frame_idx(handle); - super::shadow_slot::emit_shadow_slot_bind_ptr(ctx, idx, handle); + crate::expr::shadow_slot::emit_shadow_slot_bind_ptr(ctx, idx, handle); } /// Push `value_i64` (a bare heap pointer or NaN-boxed bits) and return the /// slot handle. -pub(crate) fn temp_root_push_i64(ctx: &mut FnCtx<'_>, value_i64: &str) -> String { +pub(in crate::rooting) fn temp_root_push_i64(ctx: &mut FnCtx<'_>, value_i64: &str) -> String { if let Some(handle) = temp_pool_acquire(ctx) { temp_slot_store(ctx, &handle, value_i64); return handle; @@ -170,7 +170,7 @@ pub(crate) fn temp_root_push_i64(ctx: &mut FnCtx<'_>, value_i64: &str) -> String } /// Push a NaN-boxed `double` temporary and return the slot-index register. -pub(crate) fn temp_root_push_double(ctx: &mut FnCtx<'_>, value: &str) -> String { +pub(in crate::rooting) fn temp_root_push_double(ctx: &mut FnCtx<'_>, value: &str) -> String { let bits = ctx.block().bitcast_double_to_i64(value); temp_root_push_i64(ctx, &bits) } @@ -180,7 +180,7 @@ pub(crate) fn temp_root_push_double(ctx: &mut FnCtx<'_>, value: &str) -> String /// In alloca mode the re-read is a plain load — the collector rewrote the /// alloca (shadow scan or statepoint relocation), so the load IS the /// post-collection value, same as a named local's re-read. -pub(crate) fn temp_root_get_i64(ctx: &mut FnCtx<'_>, idx: &str) -> String { +pub(in crate::rooting) fn temp_root_get_i64(ctx: &mut FnCtx<'_>, idx: &str) -> String { if ctx.temp_roots.alloca_mode == Some(true) { return ctx.block().load(I64, idx); } @@ -188,7 +188,7 @@ pub(crate) fn temp_root_get_i64(ctx: &mut FnCtx<'_>, idx: &str) -> String { } /// Re-read slot `idx` as a NaN-boxed `double`. -pub(crate) fn temp_root_get_double(ctx: &mut FnCtx<'_>, idx: &str) -> String { +pub(in crate::rooting) fn temp_root_get_double(ctx: &mut FnCtx<'_>, idx: &str) -> String { let bits = temp_root_get_i64(ctx, idx); ctx.block().bitcast_i64_to_double(&bits) } @@ -198,7 +198,7 @@ pub(crate) fn temp_root_get_double(ctx: &mut FnCtx<'_>, idx: &str) -> String { /// For producers that hand back a *different* address each round — the /// `concat` accumulator (#6971), where every `js_string_concat` yields a new /// string and the old one stops being the value that must stay alive. -pub(crate) fn temp_root_set_i64(ctx: &mut FnCtx<'_>, idx: &str, value_i64: &str) { +pub(in crate::rooting) fn temp_root_set_i64(ctx: &mut FnCtx<'_>, idx: &str, value_i64: &str) { if ctx.temp_roots.alloca_mode == Some(true) { temp_slot_store(ctx, idx, value_i64); return; @@ -212,13 +212,13 @@ pub(crate) fn temp_root_set_i64(ctx: &mut FnCtx<'_>, idx: &str, value_i64: &str) /// The `Object.assign` accumulator (#7200) is the same shape as the `concat` /// one: `js_object_assign_one` returns the target's *post-collection* address, /// so each link must republish rather than keep the address it passed in. -pub(crate) fn temp_root_set_double(ctx: &mut FnCtx<'_>, idx: &str, value: &str) { +pub(in crate::rooting) fn temp_root_set_double(ctx: &mut FnCtx<'_>, idx: &str, value: &str) { let bits = ctx.block().bitcast_double_to_i64(value); temp_root_set_i64(ctx, idx, &bits); } /// Drop slot `idx` and everything pushed above it. -pub(crate) fn temp_root_truncate(ctx: &mut FnCtx<'_>, idx: &str) { +pub(in crate::rooting) fn temp_root_truncate(ctx: &mut FnCtx<'_>, idx: &str) { if ctx.temp_roots.alloca_mode == Some(true) { // Mirror the FFI contract exactly: drop `idx` and everything acquired // above it. Each released slot is zeroed (dropping its retention) and @@ -236,7 +236,7 @@ pub(crate) fn temp_root_truncate(ctx: &mut FnCtx<'_>, idx: &str) { ctx.temp_roots.active = pos; for (alloca, slot_idx) in released { ctx.block().store(I64, "0", &alloca); - super::shadow_slot::emit_shadow_slot_clear(ctx, slot_idx); + crate::expr::shadow_slot::emit_shadow_slot_clear(ctx, slot_idx); } return; } @@ -252,7 +252,7 @@ pub(crate) fn temp_root_truncate(ctx: &mut FnCtx<'_>, idx: &str) { /// the triple is a load, the push itself, and a store — so the plain /// `js_array_push_f64` (which roots `value` internally on its grow path) is /// the cheaper form and the fused helper stays FFI-fallback-only. -pub(crate) fn temp_rooted_array_push(ctx: &mut FnCtx<'_>, idx: &str, value: &str) { +pub(in crate::rooting) fn temp_rooted_array_push(ctx: &mut FnCtx<'_>, idx: &str, value: &str) { if ctx.temp_roots.alloca_mode == Some(true) { let arr = ctx.block().load(I64, idx); let new_arr = ctx @@ -280,7 +280,7 @@ pub(crate) fn temp_rooted_array_push(ctx: &mut FnCtx<'_>, idx: &str, value: &str /// Pair with [`temp_rooted_array_push`] per argument, then /// [`rooted_array_read`] and [`temp_root_truncate`] — in that order, so the /// array stays rooted across the call that consumes it. -pub(crate) fn rooted_array_begin(ctx: &mut FnCtx<'_>, cap: &str) -> String { +pub(in crate::rooting) fn rooted_array_begin(ctx: &mut FnCtx<'_>, cap: &str) -> String { let arr = ctx.block().call(I64, "js_array_alloc", &[(I32, cap)]); temp_root_push_i64(ctx, &arr) } @@ -288,7 +288,7 @@ pub(crate) fn rooted_array_begin(ctx: &mut FnCtx<'_>, cap: &str) -> String { /// Read the accumulator back out of its temp-root slot. Does NOT truncate: /// callers truncate after the consuming call, so the array is still rooted /// while the consumer runs (formatting an argument list allocates). -pub(crate) fn rooted_array_read(ctx: &mut FnCtx<'_>, idx: &str) -> String { +pub(in crate::rooting) fn rooted_array_read(ctx: &mut FnCtx<'_>, idx: &str) -> String { temp_root_get_i64(ctx, idx) } @@ -297,7 +297,7 @@ pub(crate) fn rooted_array_read(ctx: &mut FnCtx<'_>, idx: &str) -> String { /// Deliberately one-sided: `false` must mean "provably allocates nothing", and /// everything unrecognized answers `true`. A wrong `false` is a /// use-after-free; a wrong `true` costs two runtime calls on a cold path. -pub(crate) fn expr_may_trigger_gc(ctx: &FnCtx<'_>, expr: &Expr) -> bool { +pub(in crate::rooting) fn expr_may_trigger_gc(ctx: &FnCtx<'_>, expr: &Expr) -> bool { match expr { // Immediates and plain slot reads. `LocalGet` reads an alloca, // `GlobalGet` a module global — neither allocates. (Reading an @@ -387,8 +387,8 @@ pub(crate) fn expr_is_inert_primitive(ctx: &FnCtx<'_>, expr: &Expr) -> bool { // provably hold no pointer cannot be strings, so the `+` is a // numeric add and allocates nothing. && (!matches!(op, perry_hir::BinaryOp::Add) - || (super::expr_is_known_non_pointer_shadow_value(ctx, left) - && super::expr_is_known_non_pointer_shadow_value(ctx, right))) + || (crate::expr::expr_is_known_non_pointer_shadow_value(ctx, left) + && crate::expr::expr_is_known_non_pointer_shadow_value(ctx, right))) } _ => false, } @@ -419,7 +419,7 @@ pub(crate) fn expr_is_inert_primitive(ctx: &FnCtx<'_>, expr: &Expr) -> bool { /// coercion of it reaches a poll decision. Honesty of scalar annotations is a /// standing invariant of the precise-root design, inherited here rather than /// introduced. -pub(crate) fn local_is_inert_primitive(ctx: &FnCtx<'_>, id: u32) -> bool { +pub(in crate::rooting) fn local_is_inert_primitive(ctx: &FnCtx<'_>, id: u32) -> bool { !ctx.shadow_slot_map.contains_key(&id) && !ctx.module_globals.contains_key(&id) && matches!( @@ -435,91 +435,6 @@ pub(crate) fn local_is_inert_primitive(ctx: &FnCtx<'_>, id: u32) -> bool { ) } -/// Does any expression after index `i` reach a collection point? -/// -/// This is the gate for protecting value `i`: a value that nothing allocating -/// follows cannot be collected before it is consumed, so the rooting calls -/// would be pure overhead. `i < n`, `x * 2` on proven-numeric locals, -/// `f(x, y)` on plain locals and `[1, 2, 3]` therefore emit exactly the IR -/// they emitted before #6951. -fn any_later_ref_may_trigger_gc(ctx: &FnCtx<'_>, exprs: &[&Expr], i: usize) -> bool { - exprs - .iter() - .skip(i + 1) - .any(|e| expr_may_trigger_gc(ctx, e)) -} - -/// Lower `exprs` left to right, keeping each already-evaluated value precisely -/// rooted across the evaluation of the ones that follow (#6951). -/// -/// Returns the lowered values — **re-read from their roots, or re-derived from -/// their immutable storage** ([`OperandProtection`]), so they are valid after -/// an evacuating cycle — and the guard index the caller must pass to -/// [`temp_root_release`] once the consuming call has run. `None` means nothing -/// needed a temp-root slot; it does NOT mean nothing was re-read, because the -/// [`OperandProtection::Reload`] half emits no runtime call at all. -pub(crate) fn lower_exprs_rooted( - ctx: &mut FnCtx<'_>, - exprs: &[&Expr], -) -> anyhow::Result<(Vec, Option)> { - let mut values = Vec::with_capacity(exprs.len()); - let mut slots: Vec> = Vec::with_capacity(exprs.len()); - let mut guard: Option = None; - let mut reload: Vec = Vec::with_capacity(exprs.len()); - for (i, expr) in exprs.iter().enumerate() { - let value = super::lower_expr(ctx, expr)?; - // `any_later_ref_may_trigger_gc` is the *window*: can anything between - // this operand and the consuming call collect? [`operand_protection`] - // turns that window into the one strategy this operand needs. - let collects = any_later_ref_may_trigger_gc(ctx, exprs, i); - match operand_protection(ctx, expr, collects) { - OperandProtection::Root => { - let idx = temp_root_push_double(ctx, &value); - // The FIRST slot pushed is the guard: truncating it drops every - // slot above it too, so one call releases the whole group. - if guard.is_none() { - guard = Some(idx.clone()); - } - slots.push(Some(idx)); - reload.push(false); - } - OperandProtection::Reload => { - slots.push(None); - reload.push(true); - } - OperandProtection::Reuse => { - slots.push(None); - reload.push(false); - } - } - values.push(value); - } - for (i, value) in values.iter_mut().enumerate() { - if let Some(idx) = slots[i].clone() { - *value = temp_root_get_double(ctx, &idx); - } else if reload[i] { - // #7114: no runtime call — just the load that was already emitted, - // emitted again below the collection point so it observes the - // address evacuation wrote back into the handle global. - *value = super::lower_expr(ctx, exprs[i])?; - } - } - Ok((values, guard)) -} - -/// Lower a `left`/`right` operand pair with the same contract as -/// [`lower_exprs_rooted`]. -pub(crate) fn lower_operand_pair_rooted( - ctx: &mut FnCtx<'_>, - left: &Expr, - right: &Expr, -) -> anyhow::Result<(String, String, Option)> { - let (mut values, guard) = lower_exprs_rooted(ctx, &[left, right])?; - let right_value = values.pop().expect("pair lowering yields two values"); - let left_value = values.pop().expect("pair lowering yields two values"); - Ok((left_value, right_value, guard)) -} - /// Already-lowered operand values kept alive across work whose shape the /// caller controls — a later operand whose *representation* is chosen per /// branch (`Expr::MapSet`, #6970) or an allocation that happens after the whole @@ -533,7 +448,7 @@ pub(crate) fn lower_operand_pair_rooted( /// When `protect` is false this emits nothing at all and [`RootedOperands::reread`] /// hands the original registers straight back, so unprotected sites keep their /// pre-#6951 IR byte for byte. -pub(crate) struct RootedOperands { +pub(in crate::rooting) struct RootedOperands { /// Slot index per operand, or `None` when the operand was not rooted. slots: Vec>, /// The registers as originally lowered — the answer when nothing is rooted @@ -578,7 +493,7 @@ pub(crate) struct RootedOperands { /// `wtf8_literal_operand_is_rooted_not_merely_reused` in /// `tests/temp_root_operand_temporaries.rs` pins the current answer so that edit /// goes red instead of shipping another silent wrong answer. -pub(crate) fn operand_is_reloadable(expr: &Expr) -> bool { +pub(in crate::rooting) fn operand_is_reloadable(expr: &Expr) -> bool { // ONLY provably immutable sources. A string literal always re-lowers to a // load of the same `__perry_init_strings_*` handle, so re-reading it can // never observe a different value. @@ -606,7 +521,7 @@ pub(crate) fn operand_is_reloadable(expr: &Expr) -> bool { /// `m.set(k, v)` roots `map` before `key` is lowered rather than after. /// /// See [`RootedOperands::push`] for the per-operand contract. -pub(crate) fn root_operands_begin(capacity: usize) -> RootedOperands { +pub(in crate::rooting) fn root_operands_begin(capacity: usize) -> RootedOperands { RootedOperands { slots: Vec::with_capacity(capacity), values: Vec::with_capacity(capacity), @@ -635,7 +550,7 @@ impl RootedOperands { /// /// When `collects` is false neither applies: nothing can be swept and /// nothing can move, so the register is reused and the IR is unchanged. - pub(crate) fn push( + pub(in crate::rooting) fn push( &mut self, ctx: &mut FnCtx<'_>, operand: &Expr, @@ -674,7 +589,7 @@ impl RootedOperands { /// - **unrooted and not re-loadable** → keep the register. This is only /// reached for values `expr_is_known_non_pointer_shadow_value` proved are /// not heap references, which relocation cannot invalidate. - pub(crate) fn reread( + pub(in crate::rooting) fn reread( &self, ctx: &mut FnCtx<'_>, operands: &[&Expr], @@ -700,7 +615,7 @@ impl RootedOperands { /// the roots exist to close. /// /// Same three cases as [`RootedOperands::reread`]; see its documentation. - pub(crate) fn reread_one( + pub(in crate::rooting) fn reread_one( &self, ctx: &mut FnCtx<'_>, operands: &[&Expr], @@ -711,21 +626,14 @@ impl RootedOperands { let idx = idx.clone(); temp_root_get_double(ctx, &idx) } - None if self.reloadable[i] => super::lower_expr(ctx, operands[i])?, + None if self.reloadable[i] => crate::expr::lower_expr(ctx, operands[i])?, None => self.values[i].clone(), }) } - /// True when this group actually pushed slots — the signal a caller uses to - /// keep an eager unbox (and therefore its exact register numbering) on the - /// unprotected path. - pub(crate) fn is_rooted(&self) -> bool { - self.guard.is_some() - } - /// Drop the group. Call it *after* the consuming call: the consumer /// allocates while reading these values. - pub(crate) fn release(self, ctx: &mut FnCtx<'_>) { + pub(in crate::rooting) fn release(self, ctx: &mut FnCtx<'_>) { temp_root_release(ctx, self.guard); } @@ -739,7 +647,7 @@ impl RootedOperands { /// drops both. So that caller needs the index rather than the act — and it /// must not release early, since the accumulator has to stay rooted across /// the consuming call too. - pub(crate) fn guard(&self) -> Option { + pub(in crate::rooting) fn guard(&self) -> Option { self.guard.clone() } } @@ -747,180 +655,14 @@ impl RootedOperands { /// Release a guard returned by [`lower_exprs_rooted`]. Call it *after* the /// consuming call, not before: the consumer allocates while reading these /// values. -pub(crate) fn temp_root_release(ctx: &mut FnCtx<'_>, guard: Option) { +pub(in crate::rooting) fn temp_root_release(ctx: &mut FnCtx<'_>, guard: Option) { if let Some(idx) = guard { temp_root_truncate(ctx, &idx); } } -/// An operand of a property/element STORE that is lowered *before* the value, -/// kept valid across the value's evaluation (#7154). -/// -/// Two operands are in that position, and both need it: -/// -/// - the **receiver**. `o.k = f()` and `o[k] = f()` evaluate the reference -/// first and the value second — spec order, and codegen follows it. That -/// leaves the receiver in an SSA register while `f()` runs, and `f()` -/// allocates. A back-edge poll inside it drives an evacuating minor which -/// relocates the receiver; the *slot* the register was loaded from is a root -/// and gets rewritten, but the register does not, so the store lands in -/// abandoned from-space memory and the field never appears on the object the -/// program keeps. -/// - the **computed key**. `o[k] = f()` lowers `k` before `f`, and a -/// non-literal string key is an ordinary heap string with the same exposure: -/// `unbox_str_handle` below the call then reads a pre-move `StringHeader*`, -/// so the field lands under a garbage key. Same for the `[sym]: init` pair of -/// a class expression's symbol statics, where the Symbol is lowered before -/// its initializer. -/// -/// This is the store-side instance of the [module invariant](self): property -/// (2) — a rewritten location — is worthless without property (3), reading that -/// location again below the collection point. It is #7114 with a store operand -/// instead of a call operand. -/// -/// A temp root (not a re-load) is the required strategy: re-lowering the -/// operand would observe an assignment made by `f()` itself, which is a -/// miscompile rather than a rooting fix — see [`operand_is_reloadable`]. -/// -/// Guards nest: push the receiver's first and the key's second, then release in -/// the opposite order, because [`temp_root_truncate`] is a stack *cut* and a -/// release of the outer one drops the inner. -pub(crate) struct StoreOperandGuard { - slot: Option, - /// The operand took [`OperandProtection::Reload`]: no runtime slot, but the - /// re-read below the collection point must re-emit the lowering rather than - /// reuse the register. See [`reread_store_operand`]. - reload: bool, -} - -/// Root `lowered` (the already-lowered `operand`) if evaluating `value` can -/// collect. Emits nothing otherwise, so stores with an inert RHS keep their old -/// IR. -pub(crate) fn guard_store_operand( - ctx: &mut FnCtx<'_>, - operand: &Expr, - lowered: &str, - value: &Expr, -) -> StoreOperandGuard { - let collects = expr_may_trigger_gc(ctx, value); - guard_store_operand_across(ctx, operand, lowered, collects) -} - -/// [`guard_store_operand`] with the window stated explicitly. -/// -/// The hazard is not visible from a single sibling expression: a receiver -/// lowered before both the key and the value is live across *both*, so its -/// `collects` is the disjunction. Deriving it from the value alone — which is -/// what every caller did before #7201 — leaves `o[f()] = 1` unguarded, because -/// the literal `1` cannot collect while `f()` obviously can. This mirrors -/// [`RootedOperands::push`], whose doc already states that "for `m.set(k, v)` -/// the receiver's window covers both `key`'s lowering and `value`'s". -pub(crate) fn guard_store_operand_across( - ctx: &mut FnCtx<'_>, - operand: &Expr, - lowered: &str, - collects: bool, -) -> StoreOperandGuard { - let protection = operand_protection(ctx, operand, collects); - let slot = match protection { - OperandProtection::Root => Some(temp_root_push_double(ctx, lowered)), - // `Reload` emits no runtime call, but it is NOT "keep the register": - // [`reread_store_operand`] re-lowers the operand below the collection - // point. `Reuse` means a proven non-pointer, which relocation cannot - // touch, so its register is genuinely reusable. - OperandProtection::Reload | OperandProtection::Reuse => None, - }; - StoreOperandGuard { - slot, - reload: protection == OperandProtection::Reload, - } -} - -/// Re-read the operand below the value's evaluation. Returns `lowered` -/// unchanged only when the operand is a proven non-pointer. -/// -/// # Why `Reload` must re-lower, not reuse (#7201) -/// -/// Until this was fixed, the `Reload` arm returned the caller's register -/// unchanged, on the reasoning that "for a literal [the register] is a load -/// from that same global". It is a load from that global *taken before the -/// collection point*. A string literal's `__perry_init_strings_*` handle is a -/// registered root that evacuation **rewrites** — that is the whole content of -/// #7114 — so the pre-collection register names from-space and the global does -/// not. Emitting the load again is the entire fix and costs no runtime call. -/// -/// This now matches [`RootedOperands::reread`], which has always re-lowered its -/// `Reload` operands. The two helper families answering the same question -/// differently is exactly the drift that produced #7114. -pub(crate) fn reread_store_operand( - ctx: &mut FnCtx<'_>, - guard: &StoreOperandGuard, - operand: &Expr, - lowered: &str, -) -> anyhow::Result { - match &guard.slot { - Some(idx) => { - let idx = idx.clone(); - Ok(temp_root_get_double(ctx, &idx)) - } - None if guard.reload => super::lower_expr(ctx, operand), - None => Ok(lowered.to_string()), - } -} - -/// Drop the guard. Call it *after* the store, not before: the store helper -/// allocates (key interning, field-array growth, shape transition). -pub(crate) fn release_store_operand(ctx: &mut FnCtx<'_>, guard: StoreOperandGuard) { - if let Some(idx) = guard.slot { - temp_root_truncate(ctx, &idx); - } -} - -/// A freshly allocated container handle (object, array, …) that generated code -/// keeps writing into while it lowers the initializer expressions. -/// -/// The handle is a raw `i64` in an SSA register, and every initializer that -/// allocates is a chance for the half-built container to be swept out from -/// under it — the object-literal form of the #6951 accumulator bug. Re-read -/// the handle through [`rooted_handle_get`] before every use. -pub(crate) struct RootedHandle { - slot: Option, - value: String, -} - -/// Root `handle` when `protect` says an upcoming initializer can collect. -/// `protect == false` emits nothing and [`rooted_handle_get`] hands the -/// original register straight back, so unprotected sites keep their old IR. -pub(crate) fn rooted_handle_begin( - ctx: &mut FnCtx<'_>, - handle_i64: &str, - protect: bool, -) -> RootedHandle { - let slot = protect.then(|| temp_root_push_i64(ctx, handle_i64)); - RootedHandle { - slot, - value: handle_i64.to_string(), - } -} - -pub(crate) fn rooted_handle_get(ctx: &mut FnCtx<'_>, handle: &RootedHandle) -> String { - match &handle.slot { - Some(idx) => { - let idx = idx.clone(); - temp_root_get_i64(ctx, &idx) - } - None => handle.value.clone(), - } -} - -pub(crate) fn rooted_handle_release(ctx: &mut FnCtx<'_>, handle: RootedHandle) { - if let Some(idx) = handle.slot { - temp_root_truncate(ctx, &idx); - } -} - /// Do any of an object literal's / call's initializer expressions collect? -pub(crate) fn any_may_trigger_gc<'a>( +pub(in crate::rooting) fn any_may_trigger_gc<'a>( ctx: &FnCtx<'_>, exprs: impl IntoIterator, ) -> bool { @@ -962,8 +704,8 @@ pub(crate) fn any_may_trigger_gc<'a>( /// (2). `m.set(fresh(), churn())` regressed straight back to an abort when this /// was written as a blanket `LocalGet` suppression; the Map receiver was /// exactly such a local. -pub(crate) fn operand_needs_root(ctx: &FnCtx<'_>, expr: &Expr) -> bool { - if super::expr_is_known_non_pointer_shadow_value(ctx, expr) { +pub(in crate::rooting) fn operand_needs_root(ctx: &FnCtx<'_>, expr: &Expr) -> bool { + if crate::expr::expr_is_known_non_pointer_shadow_value(ctx, expr) { return false; } // Only a string literal is suppressed: it is a registered root AND @@ -983,7 +725,7 @@ pub(crate) fn operand_needs_root(ctx: &FnCtx<'_>, expr: &Expr) -> bool { /// See the module header for the three properties a root buys. Each variant is /// the cheapest strategy that supplies all three for its class of operand: #[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub(crate) enum OperandProtection { +pub(in crate::rooting) enum OperandProtection { /// Push a temp-root slot and re-read it. The only strategy that gives /// liveness *and* a rewritten location *and* the call-time value, so it is /// what every operand with no other root gets — and also what a local or a @@ -1017,7 +759,7 @@ pub(crate) enum OperandProtection { /// Keeping the two predicates but calling them from two places is what let the /// pair drift, so the fix is the single call site, not a second copy of the /// re-load. -pub(crate) fn operand_protection( +pub(in crate::rooting) fn operand_protection( ctx: &FnCtx<'_>, expr: &Expr, collects: bool, @@ -1039,38 +781,3 @@ pub(crate) fn operand_protection( // reference, so there is nothing for the collector to move. OperandProtection::Reuse } - -/// Open an expression-scope temp-root barrier for a call/constructor whose -/// operands are `args`. -/// -/// Pushes a null marker slot and returns its index. Because -/// [`temp_root_truncate`] is a stack *cut*, [`temp_root_scope_end`] drops the -/// marker and every slot pushed above it — no matter which of the callee's -/// return paths ran. That is what makes rooting tractable in -/// `lower_call/new.rs`, where `lowered_args` is consumed at a dozen sites -/// spread over ~20 return paths (#6969); the alternative is a `temp_root_release` -/// at each, which is exactly the bookkeeping that gets missed. -/// -/// A null word decodes to nothing, so the marker itself roots no object. -/// Emits nothing when nothing inside the scope could ever need rooting. -/// -/// #7154: `also_needed` is the caller's extra reason to open the scope beyond -/// its operands. `lower_new_impl_inner` roots the freshly-allocated *instance* -/// across the constructor body, and `new C()` with no arguments is precisely -/// the shape that would otherwise push a slot with no marker above it to cut — -/// a temp-root entry leaked per construction. -pub(crate) fn temp_root_scope_begin( - ctx: &mut FnCtx<'_>, - args: &[Expr], - also_needed: bool, -) -> Option { - (also_needed || args.iter().any(|a| operand_needs_root(ctx, a))) - .then(|| temp_root_push_i64(ctx, "0")) -} - -/// Close a barrier opened by [`temp_root_scope_begin`]. -pub(crate) fn temp_root_scope_end(ctx: &mut FnCtx<'_>, scope: Option) { - if let Some(idx) = scope { - temp_root_truncate(ctx, &idx); - } -} diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 0ecbf67589..db71e19635 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -5290,8 +5290,7 @@ pub(crate) fn emit_gc_loop_safepoint( // `ctx` ends with the block so the poll emission below can take it // mutably. let needs_poll = { - let is_inert = - |e: &perry_hir::Expr| crate::expr::temp_root::expr_is_inert_primitive(ctx, e); + let is_inert = |e: &perry_hir::Expr| crate::rooting::expr_is_inert_primitive(ctx, e); crate::loop_purity::loop_may_allocate(body, controls, &is_inert) }; if !needs_poll { diff --git a/docs/engine-plan.md b/docs/engine-plan.md index ab312377c1..eff499e635 100644 --- a/docs/engine-plan.md +++ b/docs/engine-plan.md @@ -15,8 +15,11 @@ closed on the verdict; owner action: promote to required after its first green `json_pipeline` 500k copies the 268 MB cohort ONCE — wall −24.6% AND peak RSS −21%, the first change to improve both goal axes at once. #7592 total: **60.4 s → 3.86 s (~6× bun)**, `JSON.parse` (~742 ms) is the remaining tail. -The Layer-1 emitter migration is **started** (#7615: campaign map, per-module ledger, -1 of 88 modules done). The v0.5.1299 public-baseline sweep is +The Layer-1 emitter migration is **finished at its stated terminal condition** +(#7615, slice 8): the raw rooting API is now `crate::rooting::temp_root`, a +PRIVATE module whose accessors carry `pub(in crate::rooting)`, so a lowering +cannot reach past the combinators — unreachable, not merely uncounted. 36 +modules on the ledger; fourteen raw entry points deleted for want of a caller. The v0.5.1299 public-baseline sweep is kept as the baseline measurement event; rows fixed since are annotated in place rather than overwritten, because they were measured individually rather than in a fresh sweep. @@ -67,8 +70,8 @@ subclasses, static-method GET form, `instanceof` a subclass (#7575). promotion copies 268 MB twice — promote-on-first-copy design is on the issue with the fixed-point trap named; and `JSON.parse` 742 ms); class-field-store barriers (the half #7602 could not reach); #7480 repsel element-shape proofs; -Layer-1 emitter migration (#7615 — campaign map published, template slice landed, -1 of 88 modules). +Layer-3 ceiling list (#7615's sibling half — Layer 1 reached its terminal +condition in slice 8, so the remaining rooting work is runtime-side). **Gate debt still open:** #7554 (gc-ratchet CI has measured nothing since 2026-08-05 — REPAIR THIS BEFORE the next GC-pacing change, which needs it), @@ -85,7 +88,7 @@ has three homes, each needing its own mechanism.* | Layer | Home | Mechanism | Status | |---|---|---|---| | **0** | *enabler* | in-process LLVM | ✅ shipped (#7301), default cargo feature (#7353) | -| **1** | `perry-codegen` lowering code | `Raw`/`Rooted` discipline | design **validated & corrected** (#7459 — the RFC's own constructor was `E0499`); combinator form proven on the real emitter (#7461); the raw-pointer-across-lowering bug shape **eliminated crate-wide** (#7453, #7462–#7465). **Migration started**: campaign map + per-module ledger in **#7615**; `expr/url_main.rs` migrated end to end as the template slice (#7617), which found `URL.canParse`/`URL.parse` still carrying #7453's window. 1 of 88 modules; 262 hazard sites remain. **Measured limit, stated once: on the real emitter this does NOT make the bug fail to compile** — `FnCtx` has no interior mutability, so the borrow form is unbuildable on it; the combinator removes the bug from the path of least resistance and the ledger denies the escape hatch, and that is all | +| **1** | `perry-codegen` lowering code | `Raw`/`Rooted` discipline | design **validated & corrected** (#7459 — the RFC's own constructor was `E0499`); combinator form proven on the real emitter (#7461); the raw-pointer-across-lowering bug shape **eliminated crate-wide** (#7453, #7462–#7465). **Migration COMPLETE at its terminal condition (#7615, slices 1-8)**: `expr/temp_root.rs` is now the private `crate::rooting::temp_root`, every accessor `pub(in crate::rooting)`, so the raw API is unreachable from a lowering rather than merely unnamed — asserted by a source-level test with its own sabotage arm. 36 modules on the ledger; fourteen raw entry points (the `StoreOperandGuard` and `RootedHandle` families, `lower_exprs_rooted`, `lower_operand_pair_rooted`, `temp_root_scope_*`, …) DELETED for want of a caller. ★ Read the ledger's own caveat: a listed module cannot make an ORDERING mistake, which is not the same as "every window in it has a decision" — that audit half is per-module reading and is what remains. **Measured limit, stated once: on the real emitter this does NOT make the bug fail to compile** — `FnCtx` has no interior mutability, so the borrow form is unbuildable on it; the combinator removes the bug from the path of least resistance and the ledger denies the escape hatch, and that is all | | **2** | emitted code's liveness | statepoints | ✅ **the default**, target-aware (#7370): native roots where the runtime can walk frames, shadow stack elsewhere | | **3** | `perry-runtime` hand-written Rust | `RuntimeHandleScope`, non-optional | per-module ceilings (#7457): **595 of 705 modules locked at zero**, 107 listed with ceilings, 999 sites, and the list can only shrink — a cleaned module cannot regress (#7458). `across_*` combinators are the prescribed form (#7455). **End state not reached:** the raw accessor is still reachable inside listed modules | @@ -529,14 +532,31 @@ already working, on a workload that happens to reach it through `JSON.parse`. cannot pass on an empty subject again. Same family as #7024/#7025: the gate ran, its subject did not. -7. **Layer 1** — migrate remaining lowerings onto the rooted-combinator API - (`crates/perry-codegen/src/rooting.rs`). **#7615 is the ordered worklist**: - 88 modules, 694 raw-pointer sites, 262 hazard sites, grouped into ten - slices by hazard density. A slice finishes by adding its modules to - `MIGRATED_MODULES`, which denies `expr::temp_root` in them. The terminal - condition is `expr/temp_root.rs` going `pub(in crate::rooting)` — the raw - accessor unreachable, not merely uncounted. **Layer 3** — shrink the - 107-module ceiling list toward empty; same end state, same reason. +7. ~~**Layer 1** — migrate remaining lowerings onto the rooted-combinator + API~~ — **done at the stated terminal condition** (#7615, eight slices). + The condition was "`expr/temp_root.rs` going `pub(in crate::rooting)` — the + raw accessor unreachable, not merely uncounted". As literally spelled it is + not expressible in Rust (`pub(in path)` needs `path` to be an ANCESTOR of + the item — E0742), so the file MOVED: it is `crate::rooting::temp_root`, + declared `mod temp_root;` (private) with `pub(in crate::rooting)` on every + accessor. Both belts, because either alone is one keyword from being undone. + A raw call planted in a migrated module no longer compiles (E0603); that is + the sabotage arm, and it is the difference between this and a ledger line. + + Two items keep `pub(crate)` and are re-exported, neither an accessor: + `TempRootPool` (compile-time slot bookkeeping) and `expr_is_inert_primitive` + (the purity predicate the loop back-edge poll shares). Fourteen raw entry + points were DELETED rather than narrowed, per CLAUDE.md's kill-policy. + + ★ **What this does NOT claim.** The ledger's own caveat, drawn the hard way + in slice 4: a listed module cannot make an ORDERING mistake against the raw + API, because it no longer names it. A window with **no rooting decision at + all** is invisible to that check, and the only instrument for it is reading + the module. 36 modules are listed; the remaining audit is per-module reading + and #7640 is where the deferred sites live. + + **Layer 3** — shrink the 107-module ceiling list toward empty; same end + state, same reason. That is now the whole of the remaining rooting work. 8. **Statepoint-side static checker** — teach `gc_root_dominance_check.py` to read relocation bundles, closing the gap the #7452/#7460 repairs named. 9. ~~**RSS re-derivation under the statepoint default** (#7056)~~ — **done, and