diff --git a/CLAUDE.md b/CLAUDE.md index 54c8d0de08..2cfcaa523f 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.1292 +**Current Version:** 0.5.1293 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 8af8536395..2ad3a86324 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1292" +version = "0.5.1293" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1292" +version = "0.5.1293" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1292" +version = "0.5.1293" [[package]] name = "perry-ui-tvos" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1292" +version = "0.5.1293" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 3270356078..f4f6de0a43 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1292" +version = "0.5.1293" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7515-class-ctor-dead-field-init.md b/changelog.d/7515-class-ctor-dead-field-init.md new file mode 100644 index 0000000000..591c6cc795 --- /dev/null +++ b/changelog.d/7515-class-ctor-dead-field-init.md @@ -0,0 +1,68 @@ +### Codegen: the dead-default-field-init elision now reaches user-written constructors (#7512) + +#7469 elides the default-`undefined` write for a class field the constructor's +own prologue provably overwrites. Its changelog said it covered "plain user +ctors like `constructor(a, b) { this.a = a; this.b = b }`". It did not, and +could not: `ctor_prologue_param_assigned_fields` matched the prologue on +`Expr::PropertySet`, and **no user syntax lowers to that node**. +`perry-hir/src/lower/lower_expr/assignment.rs` turns every source-level +`obj.prop = value` — `this.v = v` included — into the spec `PutValue` node +`Expr::PutValueSet`. `Expr::PropertySet` is emitted only by *synthesized* HIR, +which is exactly what the anon-shape object-literal constructor +(`lower/context.rs::mint_anon_shape_class`) is built from. The elision was +therefore measured on the one construction form it reached, and was +structurally unreachable for the declared class it was documented as covering. + +The visible consequence is the anomaly filed as #7512: `new Node(v, w)` with two +declared `number` fields was **slower** than the equivalent `{v, w}` literal, so +the most statically-known construction form in the language was the least +optimized one. Emitted-IR census of the two constructors, same workload, same +compiler (`--trace llvm`): + +| per construction | `{v, w}` literal | `new Node(v, w)` before | after | +|---|--:|--:|--:| +| field-store IC diamonds | 2 | **4** | 2 | +| `js_typed_feedback_class_field_set_guard` | 2 | 2 | 0 | +| `js_class_field_set_fallback` | 2 | 2 | 0 | +| `js_array_numeric_value_to_raw_f64` | 0 | 4 | 2 | +| constructor body, IR lines | 111 | **314** | 157 | + +The two extra diamonds each stored a compile-time-constant `undefined` that the +next two statements overwrote. Both took the cold by-name arm on *every* +construction rather than occasionally: a freshly allocated instance carries no +typed-shape descriptor (`js_gc_init_typed_shape_layout` runs after the +constructor returns), so a `requires_raw_f64` set-guard on a declared `number` +field cannot pass, and `js_class_field_set_fallback` — a feedback-fallback +record plus a linear-key-search `js_object_set_field_by_name` — ran twice per +object. A class whose fields are declared `any` paid the same two dead diamonds. + +The fix is one recognizer, `prologue_assigned_field`, accepting both spellings +of `this. = `. The proof obligation is unchanged and is +about the *operands*, not the store opcode: `This` and `LocalGet` of a plain +parameter cannot throw, allocate, or observe `this`, so the prologue write is +reached before any other effect of the constructor. `PutValueSet` additionally +requires a constant string key and a `This` receiver. Every existing refusal +still applies — derived classes, field initializers, computed keys, parameter +defaults, setter-shadowed fields, and any statement that breaks the leading run. + +The elided write is not an observable `[[Set]]`, so it cannot change how many +times an accessor runs. A class field declaration is a `CreateDataProperty` — a +DEFINE — so it never consults an inherited accessor, and it installs an own data +property that the prologue assignment then writes directly. +`test-files/test_class_field_init_proto_setter.ts` pins that at the execution +level for the case the compile-time `class.setters` check structurally cannot +see: a setter installed on `C.prototype` *after* compilation runs **zero** times, +byte-identical to Node, both before and after this change. + +Behaviour is unchanged elsewhere too: an 11-case semantics probe (unassigned +fields still read `undefined`, `Object.keys`/JSON shape, declared-accessor +shadowing, derived classes, interrupted prologues, post-construction +reassignment, 200-instance shared-shape consistency) produces byte-identical +output before and after, and an A/B of every `test-files/*.ts` containing a +`constructor(` finds no drift between the two compilers. + +**Not fixed here, and worth its own ticket:** the residual gap is an ordering +one. `lower_call/new.rs` emits `js_gc_init_typed_shape_layout` *after* the +constructor call, so no raw-f64-declared class-field store inside any +constructor can ever pass its guard — the surviving real stores still take +`js_put_value_set`. That belongs with #7510's construction-path item. diff --git a/crates/perry-codegen/src/lower_call/field_init.rs b/crates/perry-codegen/src/lower_call/field_init.rs index 92bbeef0dc..32ffad47dd 100644 --- a/crates/perry-codegen/src/lower_call/field_init.rs +++ b/crates/perry-codegen/src/lower_call/field_init.rs @@ -13,15 +13,100 @@ use crate::expr::{lower_expr, FnCtx}; use crate::nanbox::{double_literal, POINTER_MASK_I64}; use crate::types::{DOUBLE, I32, I64}; +/// The field name a constructor-prologue statement assigns from a plain +/// parameter, or `None` if the statement is not of that shape. +/// +/// **Two HIR shapes mean the same thing here, and matching only one of them is +/// what #7512 was** (`new Node(v, w)` measured 63% slower than the equivalent +/// `{v, w}` literal, the reverse of the expected ordering): +/// +/// - `Expr::PropertySet` is what the compiler SYNTHESIZES. Every anon-shape +/// object-literal constructor (`lower/context.rs::mint_anon_shape_class`) +/// and the destructuring lowering emit it directly. +/// - `Expr::PutValueSet` is what USER SOURCE lowers to. `lower_expr`'s +/// assignment arm (`perry-hir/src/lower/lower_expr/assignment.rs`) turns +/// *every* source-level `obj.prop = value` — `this.v = v` in a hand-written +/// constructor included — into the spec `PutValue` node. Nothing a user can +/// type produces `Expr::PropertySet`. +/// +/// So #7469's elision, which only ever matched `PropertySet`, fired on the +/// synthesized literal ctor and was structurally unreachable for the declared +/// class it was documented as covering. The class paid two extra full +/// class-field-set IC diamonds per construction — a guard call plus a +/// by-name `js_class_field_set_fallback` each, since a fresh instance has no +/// typed-shape descriptor yet and the raw-f64 guard therefore cannot pass — +/// writing a compile-time-constant `undefined` that the next two statements +/// overwrite. +/// +/// The proof obligation is identical for both shapes and is entirely about the +/// *operand* expressions, not the store opcode: `This` and `LocalGet()` cannot throw, allocate, or observe `this`, so the assignment is +/// reached before any other effect of the constructor. +/// +/// **What the elided write is NOT** (the obvious objection, and it is +/// measurably wrong — `test-files/test_class_field_init_proto_setter.ts`): it +/// is not an observable `[[Set]]`, so eliding it cannot change how many times +/// an accessor runs. A class field declaration is a `CreateDataProperty` — a +/// DEFINE — per `ClassFieldDefinitionRecord` evaluation, so it never consults +/// an inherited accessor, and it installs an OWN data property that the +/// prologue's assignment then writes directly rather than dispatching past. +/// A setter installed on the prototype *after* compilation (which the +/// `class.setters` check below cannot see, by construction) runs **zero** +/// times either way, matching Node exactly. The reading in which the field +/// init is a `[[Set]]` and the setter therefore fires twice is the legacy +/// `useDefineForClassFields: false` behaviour, which neither this compiler nor +/// Node implements. The `class.setters` refusal below exists for the separate +/// case of a setter the class DECLARES, where Perry's own class-field-set +/// lowering does dispatch to the synthesized `__set_` method. +/// +/// `PutValueSet` additionally requires a constant string key (a computed key +/// is an arbitrary expression that can run user code) and `receiver` to be +/// `This` as well, since codegen evaluates both. +fn prologue_assigned_field<'a>( + stmt: &'a Stmt, + param_ids: &std::collections::HashSet, +) -> Option<&'a str> { + let is_plain_param = |e: &Expr| matches!(e, Expr::LocalGet(id) if param_ids.contains(id)); + match stmt { + // Synthesized (anon-shape ctor, destructuring lowering). + Stmt::Expr(Expr::PropertySet { + object, + property, + value, + }) if matches!(object.as_ref(), Expr::This) && is_plain_param(value.as_ref()) => { + Some(property.as_str()) + } + // User-written `this.f = p;`. + Stmt::Expr(Expr::PutValueSet { + target, + key, + value, + receiver, + strict: _, + }) if matches!(target.as_ref(), Expr::This) + && matches!(receiver.as_ref(), Expr::This) + && is_plain_param(value.as_ref()) => + { + match key.as_ref() { + Expr::String(property) => Some(property.as_str()), + _ => None, + } + } + _ => None, + } +} + /// Field names whose default-`undefined` initializer write is provably dead /// because the class's own constructor unconditionally overwrites them before -/// anything can observe `this` (#7469). +/// anything can observe `this` (#7469; extended to user-written constructors +/// by #7512). /// /// A field declared without an initializer must normally be written as /// `undefined` in the init phase (#486: `new C().x === undefined` is spec, not /// zero-bytes-from-the-allocator). But the most common constructor shape — -/// including every synthesized anon-shape literal ctor -/// (`lower/context.rs::mint_anon_shape_class`) — opens with a run of plain +/// every synthesized anon-shape literal ctor +/// (`lower/context.rs::mint_anon_shape_class`), and the hand-written +/// `constructor(v, w) { this.v = v; this.w = w }` — opens with a run of plain /// `this.f = ` statements. For those fields the `undefined` write is a /// dead store: it is overwritten before any code that could read `this.f` /// runs. On `churn.ts` that dead store was 2 of the 4 guarded field-store @@ -49,18 +134,18 @@ use crate::types::{DOUBLE, I32, I64}; /// - **Every constructor parameter is plain**: no default (a default expression /// evaluates before the prologue and, in the general lowering, could observe /// `this`), no rest, no decorators, no `arguments` materialization. -/// - **No setter shares a name with a prologue-assigned field** — the -/// PropertySet would dispatch to the setter instead of writing the slot, and -/// the elided `undefined` write was the only slot write. +/// - **No setter shares a name with a prologue-assigned field** — the store +/// would dispatch to the setter instead of writing the slot, and the elided +/// `undefined` write was the only slot write. /// - The field itself is public and non-computed (`is_private` false, /// `key_expr` none). /// -/// The prologue is the maximal leading run of -/// `Stmt::Expr(PropertySet { object: This, property, value: LocalGet() })` -/// statements. A `LocalGet` of a plain parameter cannot throw, allocate, or -/// observe `this`, so every field it assigns is written before ANY other -/// effect of the constructor — which is exactly the guarantee that makes the -/// earlier `undefined` write dead. +/// The prologue is the maximal leading run of statements that +/// [`prologue_assigned_field`] recognizes as `this. = `. A +/// `LocalGet` of a plain parameter cannot throw, allocate, or observe `this`, +/// so every field it assigns is written before ANY other effect of the +/// constructor — which is exactly the guarantee that makes the earlier +/// `undefined` write dead. fn ctor_prologue_param_assigned_fields( class: &perry_hir::Class, ) -> std::collections::HashSet { @@ -91,17 +176,11 @@ fn ctor_prologue_param_assigned_fields( let param_ids: std::collections::HashSet<_> = ctor.params.iter().map(|p| p.id).collect(); let mut assigned = std::collections::HashSet::new(); for stmt in &ctor.body { - match stmt { - Stmt::Expr(Expr::PropertySet { - object, - property, - value, - }) if matches!(object.as_ref(), Expr::This) - && matches!(value.as_ref(), Expr::LocalGet(id) if param_ids.contains(id)) => - { - assigned.insert(property.clone()); + match prologue_assigned_field(stmt, ¶m_ids) { + Some(property) => { + assigned.insert(property.to_string()); } - _ => break, + None => break, } } if assigned.is_empty() { @@ -449,3 +528,6 @@ pub(crate) fn apply_field_initializers_recursive( } Ok(()) } + +#[cfg(test)] +mod tests; diff --git a/crates/perry-codegen/src/lower_call/field_init/tests.rs b/crates/perry-codegen/src/lower_call/field_init/tests.rs new file mode 100644 index 0000000000..1d8b5d601b --- /dev/null +++ b/crates/perry-codegen/src/lower_call/field_init/tests.rs @@ -0,0 +1,314 @@ +//! Unit tests for the #7469 / #7512 dead-default-field-init predicate. +//! +//! The whole point of these is the pair +//! `synthesized_and_user_ctor_prologues_agree` pins: the two HIR spellings of +//! `this.f = ` must be treated identically. #7469 shipped matching only +//! the SYNTHESIZED one, which is why the elision was measured working on an +//! object literal and was structurally unreachable for the hand-written class +//! its own changelog claimed to cover (#7512). + +use super::*; +use perry_hir::types::Type; +use perry_hir::{Class, ClassField, Function, Param}; + +fn param(id: u32, name: &str) -> Param { + Param { + id, + name: name.to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + } +} + +fn field(name: &str) -> ClassField { + ClassField { + name: name.to_string(), + key_expr: None, + ty: Type::Number, + init: None, + is_private: false, + is_readonly: false, + decorators: Vec::new(), + } +} + +fn func(params: Vec, body: Vec) -> Function { + Function { + id: 0, + name: "constructor".to_string(), + type_params: Vec::new(), + params, + return_type: Type::Void, + body, + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + } +} + +fn class(fields: Vec, constructor: Option) -> Class { + Class { + id: 0, + name: "Node".to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields, + constructor, + methods: Vec::new(), + getters: Vec::new(), + setters: 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, + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + } +} + +/// The shape a USER writes. `this.v = v` lowers to `Expr::PutValueSet` +/// (`perry-hir/src/lower/lower_expr/assignment.rs`), never +/// `Expr::PropertySet` — no source syntax produces the latter. +fn user_this_assign(property: &str, param_id: u32) -> Stmt { + Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::This), + key: Box::new(Expr::String(property.to_string())), + value: Box::new(Expr::LocalGet(param_id)), + receiver: Box::new(Expr::This), + strict: true, + }) +} + +/// The shape the COMPILER synthesizes for an anon-shape object-literal +/// constructor (`lower/context.rs::mint_anon_shape_class`). +fn synthesized_this_assign(property: &str, param_id: u32) -> Stmt { + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::This), + property: property.to_string(), + value: Box::new(Expr::LocalGet(param_id)), + }) +} + +fn sorted(set: std::collections::HashSet) -> Vec { + let mut v: Vec = set.into_iter().collect(); + v.sort(); + v +} + +/// #7512: a hand-written `constructor(v, w) { this.v = v; this.w = w }` must +/// qualify. Before the fix the predicate matched only `Expr::PropertySet`, so +/// a declared class emitted two extra class-field-set IC diamonds per +/// construction that the equivalent `{v, w}` literal did not — making the more +/// statically-known construction form the slower one. +#[test] +fn user_written_ctor_prologue_qualifies() { + let c = class( + vec![field("v"), field("w")], + Some(func( + vec![param(1, "v"), param(2, "w")], + vec![user_this_assign("v", 1), user_this_assign("w", 2)], + )), + ); + assert_eq!( + sorted(ctor_prologue_param_assigned_fields(&c)), + vec!["v".to_string(), "w".to_string()] + ); +} + +/// The regression pin for the ordering #7512 is about: the object-literal form +/// and the class form of the same program must elide the same field set. If +/// this ever diverges again, one construction form is silently paying +/// bookkeeping the other does not. +#[test] +fn synthesized_and_user_ctor_prologues_agree() { + let synthesized = class( + vec![field("v"), field("w")], + Some(func( + vec![param(1, "v"), param(2, "w")], + vec![ + synthesized_this_assign("v", 1), + synthesized_this_assign("w", 2), + ], + )), + ); + let user = class( + vec![field("v"), field("w")], + Some(func( + vec![param(1, "v"), param(2, "w")], + vec![user_this_assign("v", 1), user_this_assign("w", 2)], + )), + ); + let synthesized_fields = sorted(ctor_prologue_param_assigned_fields(&synthesized)); + assert_eq!(synthesized_fields, vec!["v".to_string(), "w".to_string()]); + assert_eq!( + synthesized_fields, + sorted(ctor_prologue_param_assigned_fields(&user)) + ); +} + +/// The prologue is the MAXIMAL LEADING run: a statement that is not a plain +/// `this.f = ` ends it, and every field assigned after it keeps its +/// default `undefined` write. +#[test] +fn prologue_stops_at_the_first_non_matching_statement() { + let c = class( + vec![field("v"), field("w")], + Some(func( + vec![param(1, "v"), param(2, "w")], + vec![ + user_this_assign("v", 1), + // A call can allocate, throw, and observe `this`, so `w`'s + // default write is not provably dead. + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::LocalGet(9)), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + }), + user_this_assign("w", 2), + ], + )), + ); + assert_eq!( + sorted(ctor_prologue_param_assigned_fields(&c)), + vec!["v".to_string()] + ); +} + +/// A non-parameter RHS is not covered by the "cannot throw, allocate, or +/// observe `this`" argument, so it does not open a prologue. +#[test] +fn non_param_rhs_does_not_qualify() { + let c = class( + vec![field("v")], + Some(func( + vec![param(1, "v")], + vec![Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::This), + key: Box::new(Expr::String("v".to_string())), + // A body local, not a constructor parameter. + value: Box::new(Expr::LocalGet(77)), + receiver: Box::new(Expr::This), + strict: true, + })], + )), + ); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} + +/// A computed key (`this[k] = p`) evaluates an arbitrary expression to produce +/// the key, so it neither names a field statically nor is effect-free. +#[test] +fn computed_key_put_value_set_does_not_qualify() { + let c = class( + vec![field("v")], + Some(func( + vec![param(1, "v")], + vec![Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::This), + key: Box::new(Expr::LocalGet(5)), + value: Box::new(Expr::LocalGet(1)), + receiver: Box::new(Expr::This), + strict: true, + })], + )), + ); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} + +/// `PutValueSet` carries the object in both `target` and `receiver` and +/// codegen evaluates both; a receiver that is not `this` is a different +/// operation and must not be read as a prologue field write. +#[test] +fn non_this_receiver_does_not_qualify() { + let c = class( + vec![field("v")], + Some(func( + vec![param(1, "v")], + vec![Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::This), + key: Box::new(Expr::String("v".to_string())), + value: Box::new(Expr::LocalGet(1)), + receiver: Box::new(Expr::LocalGet(4)), + strict: true, + })], + )), + ); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} + +/// A same-named setter swallows the prologue store, leaving the elided +/// `undefined` as the only write that ever reached the slot. +#[test] +fn setter_shadowed_field_refuses_the_elision() { + let mut c = class( + vec![field("v"), field("w")], + Some(func( + vec![param(1, "v"), param(2, "w")], + vec![user_this_assign("v", 1), user_this_assign("w", 2)], + )), + ); + c.setters + .push(("v".to_string(), func(vec![param(3, "x")], Vec::new()))); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} + +/// A parameter default evaluates before the prologue and can, in the general +/// lowering, observe `this`. +#[test] +fn param_default_refuses_the_elision() { + let mut params = vec![param(1, "v"), param(2, "w")]; + params[1].default = Some(Expr::Number(0.0)); + let c = class( + vec![field("v"), field("w")], + Some(func( + params, + vec![user_this_assign("v", 1), user_this_assign("w", 2)], + )), + ); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} + +/// A field carrying an initializer expression means the init phase runs user +/// code that may legally read an earlier field. +#[test] +fn field_initializer_refuses_the_elision() { + let mut fields = vec![field("v"), field("w")]; + fields[1].init = Some(Expr::Number(1.0)); + let c = class( + fields, + Some(func( + vec![param(1, "v"), param(2, "w")], + vec![user_this_assign("v", 1), user_this_assign("w", 2)], + )), + ); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} + +/// A derived class has its own `super()` machinery between the field-init +/// phase and the constructor body. +#[test] +fn derived_class_refuses_the_elision() { + let mut c = class( + vec![field("v")], + Some(func(vec![param(1, "v")], vec![user_this_assign("v", 1)])), + ); + c.extends_name = Some("Base".to_string()); + assert!(ctor_prologue_param_assigned_fields(&c).is_empty()); +} diff --git a/test-files/test_class_field_init_proto_setter.ts b/test-files/test_class_field_init_proto_setter.ts new file mode 100644 index 0000000000..405f35ab6d --- /dev/null +++ b/test-files/test_class_field_init_proto_setter.ts @@ -0,0 +1,89 @@ +// #7512 / #7515: the dead-default-field-init elision must not change how many +// times an accessor runs. +// +// A setter installed on `C.prototype` AFTER compilation is invisible to the +// compile-time `class.setters` check that guards the elision, so this pins the +// observable behaviour directly rather than through that check. +// +// The spec answer is ZERO setter calls: a class field declaration (`v;`) is a +// CreateDataProperty — a DEFINE, not a [[Set]] — so it never consults an +// inherited accessor, and it installs an OWN data property that the +// constructor's `this.v = v` then writes directly. Eliding the default write +// therefore removes nothing observable. (The "field init is a [[Set]]" reading +// is the legacy `useDefineForClassFields: false` behaviour, which is not what +// this compiler or Node implements.) + +let calls = 0; +let seen = ""; + +class C { + v: number; + w: number; + constructor(v: number, w: number) { + this.v = v; + this.w = w; + } +} + +Object.defineProperty(C.prototype, "v", { + configurable: true, + set(x: any) { + calls = calls + 1; + seen = seen + "[" + String(x) + "]"; + }, + get(): any { + return 999; + }, +}); + +const c1 = new C(1, 2); +console.log("setter_calls", calls); +console.log("setter_args", seen); +console.log("read_v", c1.v); +console.log("read_w", c1.w); + +// Per-construction and stable across instances. +const c2 = new C(3, 4); +console.log("setter_calls_after_2", calls); +console.log("read_v2", c2.v, "read_w2", c2.w); + +// A setter on a SEPARATE prototype the class does not declare, reached through +// the same post-compilation route, on a field the prologue assigns second. +let wCalls = 0; + +class D { + a: number; + b: number; + constructor(a: number, b: number) { + this.a = a; + this.b = b; + } +} + +Object.defineProperty(D.prototype, "b", { + configurable: true, + set(x: any) { + wCalls = wCalls + 1; + }, + get(): any { + return -1; + }, +}); + +const d = new D(8, 9); +console.log("d_setter_calls", wCalls); +console.log("d", d.a, d.b); + +// A field the prologue does NOT assign still reads `undefined`. +class E { + a: number; + b: number; + c: number; + constructor(a: number, b: number) { + this.a = a; + this.b = b; + } +} + +const e = new E(5, 6); +console.log("e", e.a, e.b, String(e.c), e.c === undefined, JSON.stringify(e));