diff --git a/CLAUDE.md b/CLAUDE.md index 9375e4e3b4..3c8c1b5eac 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.1355 +**Current Version:** 0.5.1356 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 61bb49b197..7be84629bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1355" +version = "0.5.1356" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1355" +version = "0.5.1356" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1355" +version = "0.5.1356" [[package]] name = "perry-ui-tvos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 8f91aab0c2..2ec8239abe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1355" +version = "0.5.1356" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7625-codegen-determinism.md b/changelog.d/7625-codegen-determinism.md new file mode 100644 index 0000000000..f6a3b8e370 --- /dev/null +++ b/changelog.d/7625-codegen-determinism.md @@ -0,0 +1,78 @@ +### Emitted LLVM IR is run-to-run deterministic again (#7622) + +Compiling one source twice with the same `perry` binary produced different IR. +Not a miscompile — but it defeats byte-level IR A/B, which is the primary +evidence the #7615 rooting-migration slices offer (three of nine apparent diffs +in #7620 were this, not the change), and it is a latent hazard on the object +cache. Both shapes were `std::collections` hash-map iteration order reaching the +emitter; Rust's default hasher is seeded per `RandomState`, so the order was a +fresh permutation every process. Six runs of each of the issue's two named +sources differed 5/5 from run 1 before the fix and 0/5 after. + +**Rayon was not the cause.** #7303 suspected module-codegen completion order; +`ctx.native_modules` is a `BTreeMap`, and parallel codegen +is untouched here. Nothing else varied either — no register renumbering, no +block reordering, no metadata churn, no `Instant`- or address-derived value. + +**Function-name registration** (`codegen/artifacts.rs`). The +`hir.closure_display_names` walk. Every entry mints a rodata constant through +`add_string_constant`, whose `@.str.N` counter numbers in first-use order, and +emits one `js_register_function_name` call into `__perry_init_strings_*`, so the +map's order set both. This is the same defect #7038 fixed 36 lines below it, in +the `closure_source_text` loop, and left standing here. Now sorted by `FuncId`. + +**The dispatch towers** (`lower_call/property_get/dynamic_dispatch.rs`). The +`ctx.class_ids` walk that builds `implementors` — each surviving entry is one +`icmp`-guarded case block, so map order *was* arm order, and consecutive call +sites named different `perry_method_*` callees run to run. The virtual-override +tower (`vdispatch.*`) reads the same map the same way. Both now walk +`(class_id, class_name)` order, which is total because the name is the map key. + +`max_explicit_arity`'s scan stopped at the **first** name carrying a class id, +and ids are not unique over `class_ids` keys — a class-expression self-binding +alias (`var X = class _X`) and an imported class registered under both its own +and its local-alias name each map two names to one id. So it was a hash-order +tie-break whose loser decides how many `TAG_UNDEFINED` padding args every +emitted call in the tower carries. It now takes the max over all names sharing +the id: what the variable already means, and the safe direction (under-padding +is the #235 garbage-argument bug; over-padding just lets a default-param +desugaring fire). + +**The method-symbol registry** (`codegen/method_registry.rs`). The +`class_table.values()` walk writes into the `(class, method) -> symbol` map both +`insert` (last writer wins) and `entry().or_insert_with` (first writer wins), and +`class_table` mixes local classes, their alias keys and imported stubs — so two +distinct `&Class` can contend for one entry, in the table every call site +consults to pick its callee. Now walked by sorted key. + +**The object cache was implicated.** `compute_object_cache_key` is a function of +`CompileOptions`, the post-transform HIR fingerprint, the perry version, a hash +of the perry binary and the codegen env vars — all deterministic — while the +emitted IR was not. Identical inputs therefore produced an identical key over +different `.o` bytes: a cache hit and a cold rebuild could hold different code. +For the shapes observed that difference is semantically neutral (the name +registry keys on distinct function pointers; tower arms key on distinct class +ids), but not by construction — the tower's `seen_pairs` dedups on +`(class_id, fname)`, which admits two arms sharing one class id with different +symbols, and there emission order *is* the behaviour. Same for the registry's +two tie-breaks. That hazard is now closed. + +**Tests** (`codegen/emission_order_tests.rs`, `--lib`, so they run on every PR +touching perry-codegen rather than only in the tag-gated integration tier). Four +cases over the two shapes with a reproducing fixture. Each builds its `Module` +fresh per compile — the offending maps live in the HIR, so double-compiling one +long-lived `Module` would re-iterate the same `RandomState` and pass +unconditionally — and each asserts its subject was live (registration count, +tower arm count) before judging order. Sabotage-verified in both directions: +reverting either sort alone turns exactly that shape's two tests red and leaves +the other shape's green. The virtual-override tower and the `method_registry` +walk are sorted with **no** test and labelled untested in the module prose: +`vdispatch` blocks appear in zero of 41 sampled `test_gap_*` programs (every +receiver-typed call measured is claimed earlier by `method_override.rs`'s +`method_direct` shape guard), and a green test over a fixture that emits no arms +asserts nothing. + +Validated locally: a 42-program sweep compiled 3x each is 0 nondeterministic / +41 compared; `cargo test -p perry-codegen --lib` 695 passed; the +gc-root-dominance corpus is green in both gated modes over 149 files with 40/40 +seeded violations caught. diff --git a/crates/perry-codegen/src/codegen/artifacts.rs b/crates/perry-codegen/src/codegen/artifacts.rs index d5fe6bf440..26df4b38f6 100644 --- a/crates/perry-codegen/src/codegen/artifacts.rs +++ b/crates/perry-codegen/src/codegen/artifacts.rs @@ -1833,16 +1833,29 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> { // "use of undefined value" (regression class of #318/#343). let materialized_closure_ids: std::collections::HashSet = closures.iter().map(|(id, _)| *id).collect(); - for (func_id, display) in &hir.closure_display_names { - if !materialized_closure_ids.contains(func_id) { - continue; - } - if display.is_empty() || named_inline_closure_ids.contains(func_id) { - continue; - } - if registered_fn_ids.contains(func_id) { - continue; - } + // Sorted, NOT raw `HashMap` iteration (#7622) — the same defect #7038 fixed + // one loop down for `closure_source_text`, left standing here. Every entry + // mints a rodata constant through `add_string_constant`, whose `@.str.N` + // counter numbers in first-use order, and emits one + // `js_register_function_name` call in `__perry_init_strings_*`. Iterating + // the map directly made both a per-process permutation, so the same source + // compiled by the same binary produced different `.ll` on every run — which + // silently invalidates any A/B that compares raw IR (the primary evidence + // the #7615 rooting slices offer). Emission order is the only thing that + // changes; sorting by `FuncId` makes it stable without altering what is + // emitted. + let mut materialized_closure_display: Vec<(&perry_hir::types::FuncId, &String)> = hir + .closure_display_names + .iter() + .filter(|(func_id, display)| { + materialized_closure_ids.contains(*func_id) + && !display.is_empty() + && !named_inline_closure_ids.contains(*func_id) + && !registered_fn_ids.contains(*func_id) + }) + .collect(); + materialized_closure_display.sort_by_key(|(func_id, _)| **func_id); + for (func_id, display) in materialized_closure_display { let sym = format!("perry_closure_{}__{}", module_prefix, func_id); user_fn_display_names.push((sym, display.clone())); } diff --git a/crates/perry-codegen/src/codegen/emission_order_tests.rs b/crates/perry-codegen/src/codegen/emission_order_tests.rs new file mode 100644 index 0000000000..0e61f142e3 --- /dev/null +++ b/crates/perry-codegen/src/codegen/emission_order_tests.rs @@ -0,0 +1,433 @@ +//! #7622 — the same HIR module must emit byte-identical LLVM IR every time. +//! +//! Two emission sites read their order straight out of a `std::collections` +//! hash map. Rust's default hasher is seeded per `RandomState`, so the order +//! was a fresh permutation on every compile: the same source, compiled twice by +//! the same `perry` binary, produced different `.ll`. +//! +//! * **Function-name registration** (`codegen/artifacts.rs`). Every inline +//! closure carrying a HIR display name mints a rodata constant through +//! `add_string_constant` — whose `@.str.N` counter numbers in first-use order +//! — and emits one `js_register_function_name` call into +//! `__perry_init_strings_*`. Iterating `hir.closure_display_names` permuted +//! both. (#7038 fixed the identical defect in the `closure_source_text` loop +//! directly below it and left this one standing.) +//! * **The dynamic method-dispatch tower** +//! (`lower_call/property_get/dynamic_dispatch.rs`). Every class implementing +//! the called property becomes one `icmp`-guarded case block; iterating +//! `ctx.class_ids` permuted the arms, so consecutive call sites named +//! different `perry_method_*` callees run to run. +//! +//! That is not cosmetic. It defeats the byte-level IR A/B that the #7615 +//! rooting-migration slices offer as their correctness evidence (three of nine +//! apparent diffs in #7620 were this, not the change), and the `.perry-cache` +//! object cache keys on a DETERMINISTIC fingerprint — so a cache hit and a +//! rebuild can legitimately hold different bytes for the same inputs. +//! +//! ## Why these tests are not vacuous +//! +//! Each case builds its `Module` FRESH for every compile. That matters: the +//! offending maps live in the HIR, so compiling one long-lived `Module` twice +//! would iterate the very same `RandomState` twice and pass no matter what. +//! `N = 16` entries makes a chance-ordered agreement a 1-in-16! event. +//! +//! Each case also asserts its subject was LIVE before it judges order — the +//! registration count, and the tower arm count. A fixture that stopped emitting +//! the construct under test would otherwise go green having proven nothing. +//! +//! Sabotage-verified: reverting either sort in isolation turns exactly that +//! shape's `…_are_emitted_in_…_order` and `…_is_run_to_run_deterministic` +//! tests red, and leaves the other shape's green. +//! +//! ## What is NOT covered here, and why +//! +//! The same commit sorts two further hash-order reads that this file does not +//! test, because no fixture was found that makes them emit anything: +//! +//! * the **virtual-override tower** (`vdispatch.*`, the sibling of the tower +//! above, in the same file, over the same map). `vdispatch` blocks appear in +//! ZERO of 41 sampled `test_gap_*` programs — every receiver-typed call +//! measured is claimed first by `method_override.rs`'s `method_direct` shape +//! guard. A green test over a fixture that emits no arms would assert +//! nothing, which is worse than no test, so there is none. +//! * `method_registry.rs`'s `class_table` walk, whose `insert` (last wins) and +//! `entry().or_insert_with` (first wins) tie-breaks only diverge when two +//! distinct `&Class` contend for one registry key. +//! +//! Both are mechanical siblings of the defects that ARE covered — and #7622 +//! exists precisely because #7038 fixed one such loop and left its neighbour — +//! so they are sorted, and labelled untested rather than left implied. + +use crate::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::types::Type; +use perry_hir::{Class, Expr, Function, Module, ModuleInitKind, Param, Stmt}; + +/// Enough entries that an accidentally-sorted hash order is not a plausible +/// explanation for a green run. +const N: u32 = 16; + +fn ir_opts() -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: false, + non_entry_module_prefixes: Vec::new(), + nextjs_path_init_modules: Vec::new(), + import_function_prefixes: std::collections::HashMap::new(), + import_function_ffi_aliases: std::collections::HashMap::new(), + import_function_origin_names: std::collections::HashMap::new(), + import_function_v8_specifiers: std::collections::HashMap::new(), + import_function_node_submodule: std::collections::HashMap::new(), + namespace_node_submodules: std::collections::HashMap::new(), + namespace_v8_specifiers: std::collections::HashMap::new(), + namespace_member_prefixes: std::collections::HashMap::new(), + namespace_member_origin_names: std::collections::HashMap::new(), + emit_ir_only: true, + verify_native_regions: false, + disable_buffer_fast_path: false, + namespace_imports: Vec::new(), + imported_classes: Vec::new(), + imported_enums: Vec::new(), + imported_async_funcs: std::collections::HashSet::new(), + type_aliases: std::collections::HashMap::new(), + imported_func_param_counts: std::collections::HashMap::new(), + imported_func_has_rest: std::collections::HashSet::new(), + imported_func_synthetic_arguments: std::collections::HashSet::new(), + imported_func_return_types: std::collections::HashMap::new(), + imported_vars: std::collections::HashSet::new(), + output_type: "executable".to_string(), + needs_stdlib: false, + needs_ui: false, + needs_geisterhand: false, + geisterhand_port: 7676, + enabled_features: Vec::new(), + native_module_init_names: Vec::new(), + js_module_specifiers: Vec::new(), + bundled_extensions: Vec::new(), + native_library_functions: Vec::new(), + i18n_table: None, + fast_math: false, + fp_contract_mode: crate::FpContractMode::Off, + app_metadata: AppMetadata::default(), + namespace_entries: Vec::new(), + dynamic_import_path_to_prefix: std::collections::HashMap::new(), + deferred_module_prefixes: std::collections::HashSet::new(), + module_init_deps: Vec::new(), + is_dynamic_import_target: false, + debug_locations: false, + module_source: None, + debug_source_line_offset: 0, + } +} + +fn empty_module(name: &str) -> Module { + Module { + name: name.to_string(), + imports: Vec::new(), + exports: Vec::new(), + classes: Vec::new(), + interfaces: Vec::new(), + type_aliases: Vec::new(), + enums: Vec::new(), + globals: Vec::new(), + functions: Vec::new(), + script_global_functions: Vec::new(), + references_global_this: false, + annexb_global_undefined_names: Vec::new(), + init: Vec::new(), + exported_native_instances: Vec::new(), + exported_func_return_native_instances: Vec::new(), + exported_objects: Vec::new(), + exported_functions: Vec::new(), + widgets: Vec::new(), + uses_fetch: false, + uses_webassembly: false, + extern_funcs: Vec::new(), + init_was_unrolled: false, + has_top_level_await: false, + init_kind: ModuleInitKind::Eager, + async_step_closures: std::collections::HashSet::new(), + closure_display_names: std::collections::HashMap::new(), + class_display_names: std::collections::HashMap::new(), + closure_source_text: std::collections::HashMap::new(), + async_generator_funcs: std::collections::HashSet::new(), + gen_param_prologue_len: std::collections::HashMap::new(), + } +} + +fn ir(module: &Module) -> String { + String::from_utf8(compile_module(module, ir_opts()).expect("codegen should succeed")) + .expect("LLVM IR should be UTF-8") +} + +// --------------------------------------------------------------------------- +// Shape 1: `js_register_function_name` / `@.str.N` +// --------------------------------------------------------------------------- + +/// `let _fK = () => {}` for `K` in `0..N`, each carrying a HIR display name. +/// +/// The `_` prefix is load-bearing: the top-level `let`-bound arm of the +/// display-name collection skips underscore names outright, so every entry +/// falls through to the arm that reads `hir.closure_display_names` — the one +/// under test. +fn closure_display_module() -> Module { + let mut m = empty_module("emission_order_names.ts"); + for k in 0..N { + let func_id = 100 + k; + m.init.push(Stmt::Let { + id: 900 + k, + name: format!("_f{:02}", k), + ty: Type::Any, + mutable: false, + init: Some(Expr::Closure { + func_id, + params: Vec::new(), + return_type: Type::Void, + body: Vec::new(), + captures: Vec::new(), + mutable_captures: Vec::new(), + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: true, + }), + }); + m.closure_display_names + .insert(func_id, format!("name{:02}", k)); + } + m +} + +/// The `func_id` of every `js_register_function_name` call, in emission order. +fn registered_closure_ids(ir: &str) -> Vec { + ir.lines() + .filter(|l| l.contains("call void @js_register_function_name(")) + .filter_map(|l| { + let at = l.find("@perry_closure_")?; + let rest = &l[at..]; + let end = rest.find(',')?; + rest[..end].rsplit("__").next()?.parse::().ok() + }) + .collect() +} + +#[test] +fn closure_display_names_are_emitted_in_func_id_order() { + let ids = registered_closure_ids(&ir(&closure_display_module())); + // Liveness: the construct under test was actually emitted. + assert_eq!( + ids.len(), + N as usize, + "expected one js_register_function_name per closure display name; \ + the fixture stopped exercising the emission path" + ); + let mut sorted = ids.clone(); + sorted.sort_unstable(); + assert_eq!( + ids, sorted, + "js_register_function_name calls must be emitted in FuncId order, not \ + `hir.closure_display_names` hash order (#7622)" + ); +} + +#[test] +fn closure_display_name_emission_is_run_to_run_deterministic() { + // A FRESH module per compile — same content, a different `RandomState` for + // its `closure_display_names` map. Reusing one module would re-iterate the + // same map and pass unconditionally. + let first = ir(&closure_display_module()); + let second = ir(&closure_display_module()); + assert!( + first.contains("call void @js_register_function_name("), + "liveness: fixture emitted no function-name registrations" + ); + assert_eq!( + first, second, + "two compiles of the same module must emit byte-identical IR (#7622)" + ); +} + +// --------------------------------------------------------------------------- +// Shape 2: the class-id dispatch tower +// --------------------------------------------------------------------------- + +fn method_fn(id: u32, name: &str) -> Function { + Function { + id, + name: name.to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Number, + body: vec![Stmt::Return(Some(Expr::Number(1.0)))], + is_async: false, + is_generator: false, + is_strict: true, + was_plain_async: false, + was_unrolled: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + } +} + +fn plain_class(id: u32, name: &str, method: Function) -> Class { + Class { + id, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: Vec::new(), + constructor: None, + methods: vec![method], + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + computed_members: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + } +} + +/// `N` classes that all declare `m()`, plus `function callm(o: any) { o.m(); }`. +/// +/// `o` is an `any`-typed parameter, so `receiver_class_name` cannot name a +/// class and the call lowers through the dynamic-dispatch tower — one +/// `icmp`-guarded case per implementing class. +/// +/// Class ids run OPPOSITE to class names on purpose: `C00` gets id `N`, `C15` +/// gets id 1. The emission order under test is by class id, and with ids +/// assigned in name order the two are indistinguishable — a sort keyed on the +/// class NAME would satisfy the assertion just as well, so the test would not +/// pin the property it claims. Reversed, only a class-id sort produces the +/// expected sequence. +fn dispatch_tower_module() -> Module { + let mut m = empty_module("emission_order_tower.ts"); + for k in 0..N { + m.classes.push(plain_class( + N - k, + &format!("C{:02}", k), + method_fn(200 + k, "m"), + )); + } + m.functions.push(Function { + id: 700, + name: "callm".to_string(), + type_params: Vec::new(), + params: vec![Param { + id: 701, + name: "o".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + return_type: Type::Void, + body: vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(701)), + property: "m".to_string(), + byte_offset: 0, + }), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + })], + is_async: false, + is_generator: false, + is_strict: true, + was_plain_async: false, + was_unrolled: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + }); + m +} + +/// The class name of every `call … @perry_method_*__m(...)` inside the body of +/// `callm` — i.e. the dispatch tower's arm order. +/// +/// Scoped to that one function on purpose. Codegen also emits a per-class +/// closure-call wrapper (`artifacts.rs`, one `call @perry_method_…__m` each), +/// and those walk the `hir.classes` Vec, so they are already ordered and would +/// mask the tower's order if folded into the same list. +fn tower_arm_classes(ir: &str) -> Vec { + let mut in_callm = false; + let mut out = Vec::new(); + for l in ir.lines() { + if l.starts_with("define ") { + in_callm = l.contains("callm"); + continue; + } + if !in_callm || !l.contains("call") || !l.contains("@perry_method_") { + continue; + } + let Some(at) = l.find("@perry_method_") else { + continue; + }; + let rest = &l[at..]; + let Some(end) = rest.find('(') else { continue }; + // `@perry_method_____m` + let Some(class) = rest[..end] + .strip_suffix("__m") + .and_then(|s| s.rsplit("__").next()) + else { + continue; + }; + if class.starts_with('C') { + out.push(class.to_string()); + } + } + out +} + +#[test] +fn dispatch_tower_arms_are_emitted_in_class_id_order() { + let arms = tower_arm_classes(&ir(&dispatch_tower_module())); + // Liveness: the tower was actually emitted, with one arm per class. Without + // this the test would pass on a fixture that stopped producing a tower at + // all (e.g. if the receiver ever became statically typed). + assert_eq!( + arms.len(), + N as usize, + "expected one dispatch-tower arm per implementing class, got {:?}", + arms + ); + // Spelled out rather than derived by sorting `arms` itself: `C{:02}` runs + // opposite to the class ids, so this sequence is satisfied ONLY by a + // class-id ordering. A name-keyed sort — or `arms.sort()` compared against + // itself — would accept the reverse and prove nothing. + let expected: Vec = (0..N).rev().map(|k| format!("C{:02}", k)).collect(); + assert_eq!( + arms, expected, + "dispatch-tower arms must be emitted in class-id order, not \ + `ctx.class_ids` hash order (#7622)" + ); +} + +#[test] +fn dispatch_tower_emission_is_run_to_run_deterministic() { + let first = ir(&dispatch_tower_module()); + let second = ir(&dispatch_tower_module()); + assert!( + first.contains("@perry_method_"), + "liveness: fixture emitted no class methods" + ); + assert_eq!( + first, second, + "two compiles of the same module must emit byte-identical IR (#7622)" + ); +} diff --git a/crates/perry-codegen/src/codegen/method_registry.rs b/crates/perry-codegen/src/codegen/method_registry.rs index 996ceafdc6..202a20a83c 100644 --- a/crates/perry-codegen/src/codegen/method_registry.rs +++ b/crates/perry-codegen/src/codegen/method_registry.rs @@ -39,7 +39,18 @@ pub(crate) fn build_method_names( // which mangled function name to call for `obj.method(args)`. Method // names are also scoped by module prefix. let mut method_names: HashMap<(String, String), String> = HashMap::new(); - for c in class_table.values() { + // Walk `class_table` by SORTED key, not in `HashMap` order (#7622). The + // body writes into `method_names` two order-dependent ways — plain + // `insert` (last writer wins) for the class's own keys, and + // `entry().or_insert_with` (first writer wins) for its `aliases` — and + // `class_table` is not key-unique over `&Class`: it mixes local classes, + // their self-binding alias keys, and imported class stubs, so two distinct + // `&Class` can contend for the same registry entry. `method_names` is what + // every call site consults to pick its `perry_method_*` callee, so a + // hash-order tie-break there is visible in the emitted IR. + let mut class_table_sorted: Vec<(&String, &&perry_hir::Class)> = class_table.iter().collect(); + class_table_sorted.sort_unstable_by_key(|(name, _)| *name); + for (_, c) in class_table_sorted { // Use the source module prefix for imported classes so the method // symbol name matches where the method was actually compiled. let class_prefix = imported_class_prefix diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 5c1e7e5fe2..d321d2152e 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -44,6 +44,8 @@ mod artifacts; mod boxed_locals; mod closure; mod closure_collect; +#[cfg(test)] +mod emission_order_tests; mod entry; mod func_registry; mod function; diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index bdf1b9ca56..92354eba0d 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -238,7 +238,21 @@ pub(crate) fn try_lower_instance_method_call( let mut impl_owner: Vec> = Vec::new(); let mut seen_pairs: std::collections::HashSet<(u32, String)> = std::collections::HashSet::new(); - for (start_cls, &start_cid) in ctx.class_ids.iter() { + // Walk `class_ids` in a FIXED order, not `HashMap` order (#7622). Each + // surviving entry becomes one `icmp`-guarded case block in the tower + // below, so the map's per-process iteration order was the tower's arm + // order: the same source compiled twice by the same binary emitted the + // same arms naming DIFFERENT `perry_method_*` callees per position. + // `seen_pairs` also dedups on `(class_id, fname)`, so with two names + // sharing a class id (a class-expression self-binding alias, or an + // imported class registered under both its own and its local-alias + // name) which arm is emitted FIRST is what the runtime's first-match + // tower actually executes — order-dependence that is behavioural, not + // just cosmetic. Sorting by `(class_id, name)` is total and stable. + let mut dispatch_roots: Vec<(&String, u32)> = + ctx.class_ids.iter().map(|(k, &v)| (k, v)).collect(); + dispatch_roots.sort_unstable_by(|a, b| a.1.cmp(&b.1).then_with(|| a.0.cmp(b.0))); + for (start_cls, start_cid) in dispatch_roots { let mut cur: Option = Some(start_cls.clone()); while let Some(c) = cur { let key = (c.clone(), property.to_string()); @@ -691,7 +705,15 @@ pub(crate) fn try_lower_instance_method_call( // function than the static fallback, C needs an // explicit case in the dispatch table. let mut overrides: Vec<(u32, String)> = Vec::new(); - for (sub_name, &sub_id) in ctx.class_ids.iter() { + // Fixed order, not `HashMap` order — the virtual-override tower has + // the same #7622 defect as the interface tower above, and for the + // same reason: `overrides` is walked by index to emit the + // `vdispatch.caseN` blocks, the `icmp eq i32` chain and the phi + // incoming list, so the map's per-process order WAS the arm order. + let mut override_roots: Vec<(&String, u32)> = + ctx.class_ids.iter().map(|(k, &v)| (k, v)).collect(); + override_roots.sort_unstable_by(|a, b| a.1.cmp(&b.1).then_with(|| a.0.cmp(b.0))); + for (sub_name, sub_id) in override_roots { if *sub_name == class_name { continue; } @@ -776,18 +798,29 @@ pub(crate) fn try_lower_instance_method_call( } walk = ctx.classes.get(&cur).and_then(|c| c.extends_name.clone()); } + // #7622: no `break` on the first name that carries `sub_id`. Class + // ids are not unique over `class_ids` KEYS — a class-expression + // self-binding alias (`var X = class _X`) and an imported class + // registered under both its own and its local-alias name both map + // two names to one id — so first-match-wins made this a hash-order + // tie-break, and the loser's arity is what decides how many + // TAG_UNDEFINED padding args EVERY emitted call in the tower + // carries. Taking the max over all names sharing the id is both + // order-independent and the direction this variable already means: + // under-padding is the #235 garbage-argument bug, over-padding just + // lets a default-param desugaring fire. for (sub_id, _) in &overrides { for (sub_name, &id) in ctx.class_ids.iter() { - if id == *sub_id { - if let Some(&n) = ctx - .method_param_counts - .get(&(sub_name.clone(), property.to_string())) - { - if n > max_explicit_arity { - max_explicit_arity = n; - } + if id != *sub_id { + continue; + } + if let Some(&n) = ctx + .method_param_counts + .get(&(sub_name.clone(), property.to_string())) + { + if n > max_explicit_arity { + max_explicit_arity = n; } - break; } } }