From 84cf1d54da074ed199f47c680dcd15f913dcb4c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 05:00:56 +0000 Subject: [PATCH 1/2] perf(hir): infer constructor this-fields for class expressions (#10499) A plain-JS class expression (`var NodeObject = class {...}`, `module.exports = class X {...}`) was lowered with zero fields because only lower_class_decl scanned the constructor for top-level `this. = ...` stores, so every constructor store became a property add through full [[Set]]. Share that scan (with its declared/inherited/accessor/method exclusions) between lower_class_decl and lower_class_from_ast. A class expression infers only when its parent layout is statically known (no heritage, or a static parent with a registered field set); over a runtime parent (mixin parameter, lexically-local binding, `extends f()`) an own slot would shadow the parent's value. When the heritage is a plain identifier that took the dynamic path, look up the registered sets under it to exclude the parent's fields, which also fixes a declaration subclass of a class-expression base reading the base's fields as undefined. --- crates/perry-hir/src/lower/tests.rs | 1 + .../src/lower/tests/class_expr_ctor_fields.rs | 101 +++++++ crates/perry-hir/src/lower_decl/class_decl.rs | 189 +------------ .../src/lower_decl/class_decl/ctor_fields.rs | 251 ++++++++++++++++++ .../src/lower_decl/class_decl/from_ast.rs | 52 ++-- .../test_gap_10499_class_expr_ctor_fields.ts | 200 ++++++++++++++ 6 files changed, 600 insertions(+), 194 deletions(-) create mode 100644 crates/perry-hir/src/lower/tests/class_expr_ctor_fields.rs create mode 100644 crates/perry-hir/src/lower_decl/class_decl/ctor_fields.rs create mode 100644 test-files/test_gap_10499_class_expr_ctor_fields.ts diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 8e02fb4ef6..580c85e2d8 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1264,6 +1264,7 @@ mod function_ctor_runtime_routing; mod mixin_parent_chain; mod native_module_sync; +mod class_expr_ctor_fields; mod class_expr_subclass_captures; mod nullish_over_optional_chain; mod subclass_ctor_inherited_method; diff --git a/crates/perry-hir/src/lower/tests/class_expr_ctor_fields.rs b/crates/perry-hir/src/lower/tests/class_expr_ctor_fields.rs new file mode 100644 index 0000000000..b01471a18f --- /dev/null +++ b/crates/perry-hir/src/lower/tests/class_expr_ctor_fields.rs @@ -0,0 +1,101 @@ +//! #10499: a class EXPRESSION's top-level constructor `this. = …` +//! assignments must be inferred as own fields exactly like the identical +//! class DECLARATION's. Pre-fix only `lower_class_decl` ran the scan, so +//! `var NodeExpr = class {…}` lowered with `fields: 0` and every constructor +//! store was a generic `[[Set]]` property add (159× slower than Node). + +fn lower(source: &str) -> crate::ir::Module { + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + super::lower_module(&module, "t", "t.ts").expect("source lowers") +} + +fn field_names(hir: &crate::ir::Module, class: &str) -> Vec { + hir.classes + .iter() + // A NAMED class expression registers as `__class_expr_`. + .find(|c| c.name == class || c.name.starts_with(&format!("{class}__class_expr_"))) + .unwrap_or_else(|| { + panic!( + "fixture declares class {class}; classes: {:?}", + hir.classes.iter().map(|c| &c.name).collect::>() + ) + }) + .fields + .iter() + .map(|f| f.name.clone()) + .collect() +} + +/// The issue's reproducer: the expression and the declaration get the same +/// six fields, in the same (execution) order. +#[test] +fn class_expression_infers_ctor_fields_like_declaration() { + let hir = lower( + r#" + class NodeDecl { constructor(kind, pos, end) { this.pos = pos; this.end = end; this.kind = kind; this.id = 0; this.flags = 0; this.parent = undefined; } } + var NodeExpr = class { constructor(kind, pos, end) { this.pos = pos; this.end = end; this.kind = kind; this.id = 0; this.flags = 0; this.parent = undefined; } }; + "#, + ); + let expected = ["pos", "end", "kind", "id", "flags", "parent"]; + assert_eq!(field_names(&hir, "NodeDecl"), expected); + assert_eq!(field_names(&hir, "NodeExpr"), expected); +} + +/// The declaration's exclusions apply to expressions too: own methods +/// (self-binding) and accessors are not data fields; declared fields are not +/// duplicated; minified comma sequences are scanned. +#[test] +fn class_expression_keeps_declaration_exclusions() { + let hir = lower( + r#" + var C = class { + declared = 1; + constructor(p) { + this.declared = 2; + this.run = this.run.bind(this); + this.points = p; + (this.a = 1), (this.b = 2); + } + run() {} + set points(v) {} + get points() { return 0; } + }; + "#, + ); + assert_eq!(field_names(&hir, "C"), ["declared", "a", "b"]); +} + +/// A declaration subclass of a class-expression base must see the base's +/// inferred fields as inherited and not re-add them as own fields (two slots +/// for one name). +#[test] +fn declaration_subclass_excludes_class_expression_parent_fields() { + let hir = lower( + r#" + var Base = class { constructor() { this.kind = "base"; this.shared = 1; } }; + class DeclSub extends Base { constructor() { super(); this.shared = 3; this.tag = 4; } } + "#, + ); + assert_eq!(field_names(&hir, "Base"), ["kind", "shared"]); + assert_eq!(field_names(&hir, "DeclSub"), ["tag"]); +} + +/// A class expression whose parent is only known at runtime (a mixin +/// parameter, `extends pick()`) must NOT infer fields: an own slot for a +/// name the runtime parent's constructor also writes would shadow the +/// parent's value. +#[test] +fn class_expression_over_runtime_parent_infers_no_fields() { + let hir = lower( + r#" + class Left { constructor() { this.side = "left"; } } + function pick() { return Left; } + var Picked = class extends pick() { constructor() { super(); this.side = this.side + "!"; this.own = 1; } }; + function Tagged(BaseClass) { + return class TaggedImpl extends BaseClass { constructor() { super(); this.kind = 1; } }; + } + "#, + ); + assert!(field_names(&hir, "Picked").is_empty()); + assert!(field_names(&hir, "TaggedImpl").is_empty()); +} diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 094f466e97..3fb0671004 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -48,12 +48,14 @@ fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool { } mod class_heritage; +mod ctor_fields; mod decl_self_binding; pub(crate) use decl_self_binding::fresh_class_decl_self_binding; mod from_ast; mod member_helpers; mod member_registration; use class_heritage::*; +use ctor_fields::infer_ctor_this_fields; pub(crate) use from_ast::lower_class_from_ast; pub(crate) use member_helpers::capture_class_source; use member_helpers::{ @@ -1014,184 +1016,17 @@ pub fn lower_class_decl( } } - // Detect fields from constructor body `this.xxx = ...` assignments. - // JavaScript classes (e.g., transpiled from TypeScript) often don't have ClassProp - // declarations; instead they assign to `this` in the constructor body. - // - // IMPORTANT: Also exclude fields inherited from parent classes. If the parent already - // declares `kind` and the subclass writes `this.kind = ...`, the subclass must NOT - // add `kind` as a new own field. Otherwise, codegen's resolve_class_fields later - // merges parent and own indices and the subclass's shadow `kind` gets a different - // offset from the parent's, leaving TWO `kind` slots that disagree at runtime. + // Detect fields from constructor body `this.xxx = ...` assignments (shared with + // class expressions, #10499 — see `ctor_fields.rs`). { - // Collect inherited field names by walking the parent chain via the extends_name. - // Previous lower_class_decl calls have registered each class's full (own+inherited) - // field set, so a single lookup on the direct parent yields the complete chain. - let mut inherited_field_names: std::collections::HashSet = - std::collections::HashSet::new(); - if let Some(ref parent_name) = extends_name { - if let Some(parent_fields) = ctx.lookup_class_field_names(parent_name) { - for f in parent_fields { - inherited_field_names.insert(f.clone()); - } - } - } - - // Issue #665 (sixth pass): collect own + inherited accessor (getter+setter) - // property names. Real-world packages like rate-limiter-flexible - // declare a `set points(v)` accessor AND write `this.points = opts.points` - // from the constructor body. Pre-fix the bare-this scan below - // mis-categorised `points` as an own data field, allocating an - // inline slot that surfaced via `Object.keys` and shadowed the - // accessor when a subclass instance's `.points` was read across - // modules (the runtime's setter dispatch walks the class vtable - // chain correctly, but the spurious own-data slot wins lookup). - let mut accessor_names = runtime_instance_accessor_names(&class_decl.class.body); - // Pull in accessor names from the parent chain. The parent's - // registration stored the own+inherited union, so a single lookup - // on the direct parent suffices. - if let Some(ref parent_name) = extends_name { - if let Some(parent_accessors) = ctx.lookup_class_accessor_names(parent_name) { - accessor_names.extend_from(parent_accessors); - } - } - - // Own instance method names. A constructor `this.method = this.method.bind(this)` - // (zod's `ZodType` ctor self-binds ~20 methods; React class components do the - // same) is a METHOD OVERRIDE, not a new data field — the assignment creates a - // runtime own property handled by the method-override dispatch path. Allocating - // an inline field slot for it makes the codegen field branch shadow the method on - // every read, so `this.method` reads the uninitialised slot (`undefined`) BEFORE - // the assignment runs — exactly what made `this.parse.bind(this)` throw "Bind must - // be called on a function" in zod. Mirrors the accessor exclusion (#665). - let mut method_names: std::collections::HashSet = std::collections::HashSet::new(); - for member in &class_decl.class.body { - match member { - ast::ClassMember::Method(m) if matches!(m.kind, ast::MethodKind::Method) => { - let key = match &m.key { - ast::PropName::Ident(i) => i.sym.to_string(), - ast::PropName::Str(s) => s.value.as_str().unwrap_or("").to_string(), - _ => continue, - }; - method_names.insert(key); - } - ast::ClassMember::PrivateMethod(m) if matches!(m.kind, ast::MethodKind::Method) => { - method_names.insert(format!("#{}", m.key.name)); - } - _ => {} - } - } - // Issue #10487: pull in the parent chain's own+inherited method - // names too, mirroring the accessor union just above. A subclass - // constructor's `this.close = …` overriding a PARENT method (not - // redeclared on this class) must be recognized as a method - // override, not a new own data field, or the field wins the - // dynamic-dispatch lookup and instance reads see `undefined` - // until the assignment statement runs. - if let Some(ref parent_name) = extends_name { - if let Some(parent_methods) = ctx.lookup_class_method_names(parent_name) { - for m in parent_methods { - method_names.insert(m.clone()); - } - } - } - - let declared_field_names: std::collections::HashSet = - fields.iter().map(|f| f.name.clone()).collect(); - // Pull each top-level `this. = …` field name out of one ctor - // statement-expression. Minified bundles (Next.js `BaseNextRequest`'s - // `constructor(a,b,c){this.method=a,this.url=b,this.body=c}`) collapse - // every ctor assignment into ONE comma-`Seq` expression-statement, so a - // scan that only matched `Expr::Assign` detected ZERO fields — the - // parent's `method`/`url`/`body` never entered `packed_keys`, leaving - // the subclass instance allocated with too-few inline slots so the - // captured-class shape prepends `__perry_cap_*` over the (missing) real - // slots and `e.url` reads undefined ("Invalid URL" 500 on dynamic page - // routes). Descend through `Seq` (and the `Paren`/`Assign`-result-chain - // wrappers minifiers emit) so each comma-separated `this.x = …` is - // recognised the same as a standalone assignment statement. - fn collect_this_field_assigns(expr: &ast::Expr, out: &mut Vec) { - match expr { - ast::Expr::Assign(assign) => { - // A chained assignment's RHS can itself be `this.x = …` - // (`this.a = this.b = v`): the inner `this.b = v` evaluates - // (and creates `b`'s slot) BEFORE the outer assignment to - // `this.a`, so collect the RHS first to keep Object.keys in - // the same insertion order Node produces (`b` then `a`). - collect_this_field_assigns(&assign.right, out); - if let ast::AssignTarget::Simple(ast::SimpleAssignTarget::Member(mem)) = - &assign.left - { - if let ast::Expr::This(_) = &*mem.obj { - if let ast::MemberProp::Ident(prop_ident) = &mem.prop { - out.push(prop_ident.sym.to_string()); - } - } - } - } - ast::Expr::Seq(seq) => { - for e in &seq.exprs { - collect_this_field_assigns(e, out); - } - } - ast::Expr::Paren(p) => collect_this_field_assigns(&p.expr, out), - _ => {} - } - } - for member in &class_decl.class.body { - if let ast::ClassMember::Constructor(ctor) = member { - if let Some(ref body) = ctor.body { - for stmt in &body.stmts { - if let ast::Stmt::Expr(expr_stmt) = stmt { - let mut names: Vec = Vec::new(); - collect_this_field_assigns(&expr_stmt.expr, &mut names); - for fname in names { - if !declared_field_names.contains(&fname) - && !inherited_field_names.contains(&fname) - && !accessor_names.contains_any(&fname) - && !method_names.contains(&fname) - { - fields.push(ClassField { - name: fname, - key_expr: None, - ty: Type::Any, - init: None, - is_private: false, - is_readonly: false, - decorators: Vec::new(), - }); - } - } - } - } - } - } - } - // Dedup fields: keep first occurrence of each name - let mut seen = std::collections::HashSet::new(); - fields.retain(|f| seen.insert(f.name.clone())); - - // Register this class's complete field set (own + inherited) so subclasses that - // extend it can see the full inheritance chain during their own lowering. - let mut complete_field_names: Vec = inherited_field_names.into_iter().collect(); - for f in &fields { - if !complete_field_names.contains(&f.name) { - complete_field_names.push(f.name.clone()); - } - } - ctx.register_class_field_names(name.clone(), complete_field_names); - - // Issue #665: register own+inherited accessor names so subclasses - // lowered after this one can also skip them when scanning ctor - // bodies. `accessor_names` already contains the getter/setter names - // from the parent-chain lookup above. - ctx.register_class_accessor_names(name.clone(), accessor_names); - - // Issue #10487: register this class's complete (own + inherited) - // method-name set, mirroring the accessor registration just above, - // so a further subclass lowered after this one sees the full - // chain in one lookup. - ctx.register_class_method_names(name.clone(), method_names.into_iter().collect()); + infer_ctor_this_fields( + ctx, + &class_decl.class, + &name, + extends_name.as_deref(), + true, + &mut fields, + ); // Issue #302: also register field TYPES so the for-of arm can // detect `for (... of this.someMap)` patterns. Only own fields are diff --git a/crates/perry-hir/src/lower_decl/class_decl/ctor_fields.rs b/crates/perry-hir/src/lower_decl/class_decl/ctor_fields.rs new file mode 100644 index 0000000000..29d5e4882c --- /dev/null +++ b/crates/perry-hir/src/lower_decl/class_decl/ctor_fields.rs @@ -0,0 +1,251 @@ +//! Constructor `this. = …` field inference, shared by class +//! declarations (`lower_class_decl`) and class expressions +//! (`lower_class_from_ast`). +//! +//! Issue #10499: the scan used to live inline in `lower_class_decl` only, so +//! a plain-JS class EXPRESSION (`var NodeObject = class {…}`, +//! `module.exports = class X {…}`, esbuild/rollup `var X = class {…}`) was +//! lowered with zero fields and every constructor store became a property +//! ADD through the full `[[Set]]` path (~4,800 instructions each) instead of +//! a slot store — `new` on typescript.js's `NodeObject` ran 159× slower than +//! Node against 7× for the identical declaration. + +use super::*; + +/// Append an own instance field for every top-level constructor +/// `this. = …` assignment not already covered by a declared field, an +/// inherited field, an (own or inherited) accessor, or an (own or inherited) +/// method, then register the class's complete field / accessor / method +/// name sets under `name` so classes lowered later that extend it see the +/// whole chain in one lookup. +/// +/// `parent_name` is the class-registry name of the direct parent, if known. +/// When it is not (the heritage took the dynamic `extends_expr` path, e.g. a +/// lexically-local `const Base = class {…}` binding), a plain-identifier +/// heritage is still looked up under its alias-resolved name to EXCLUDE +/// whatever that class registered: an extra exclusion only leaves a name on +/// the generic property path, while a missed one makes an own slot shadow +/// the value the parent's constructor stored (#10499 — a declaration +/// subclass of a class-expression base read `this.side` as `undefined`). +/// +/// With `infer_fields == false` nothing is appended and no field-name set is +/// registered (a class whose parent layout is unknown must not publish an +/// incomplete one); the accessor and method sets are still registered. +pub(super) fn infer_ctor_this_fields( + ctx: &mut LoweringContext, + class: &ast::Class, + name: &str, + parent_name: Option<&str>, + infer_fields: bool, + fields: &mut Vec, +) { + // JavaScript classes (e.g., transpiled from TypeScript) often don't have ClassProp + // declarations; instead they assign to `this` in the constructor body. + // + // IMPORTANT: Also exclude fields inherited from parent classes. If the parent already + // declares `kind` and the subclass writes `this.kind = ...`, the subclass must NOT + // add `kind` as a new own field. Otherwise, codegen's resolve_class_fields later + // merges parent and own indices and the subclass's shadow `kind` gets a different + // offset from the parent's, leaving TWO `kind` slots that disagree at runtime. + // + // Collect inherited field names by walking the parent chain via the parent name. + // Previous lowerings have registered each class's full (own+inherited) field set, + // so a single lookup on the direct parent yields the complete chain. + let heritage_ident_name: Option = match (parent_name, class.super_class.as_deref()) { + (None, Some(ast::Expr::Ident(ident))) => { + let raw = ident.sym.to_string(); + Some(ctx.resolve_class_alias(&raw).unwrap_or(raw)) + } + _ => None, + }; + let parent_name = parent_name.or(heritage_ident_name.as_deref()); + let mut inherited_field_names: std::collections::HashSet = + std::collections::HashSet::new(); + if let Some(parent_name) = parent_name { + if let Some(parent_fields) = ctx.lookup_class_field_names(parent_name) { + for f in parent_fields { + inherited_field_names.insert(f.clone()); + } + } + } + + // Issue #665 (sixth pass): collect own + inherited accessor (getter+setter) + // property names. Real-world packages like rate-limiter-flexible + // declare a `set points(v)` accessor AND write `this.points = opts.points` + // from the constructor body. Pre-fix the bare-this scan below + // mis-categorised `points` as an own data field, allocating an + // inline slot that surfaced via `Object.keys` and shadowed the + // accessor when a subclass instance's `.points` was read across + // modules (the runtime's setter dispatch walks the class vtable + // chain correctly, but the spurious own-data slot wins lookup). + let mut accessor_names = runtime_instance_accessor_names(&class.body); + // Pull in accessor names from the parent chain. The parent's + // registration stored the own+inherited union, so a single lookup + // on the direct parent suffices. + if let Some(parent_name) = parent_name { + if let Some(parent_accessors) = ctx.lookup_class_accessor_names(parent_name) { + accessor_names.extend_from(parent_accessors); + } + } + + // Own instance method names. A constructor `this.method = this.method.bind(this)` + // (zod's `ZodType` ctor self-binds ~20 methods; React class components do the + // same) is a METHOD OVERRIDE, not a new data field — the assignment creates a + // runtime own property handled by the method-override dispatch path. Allocating + // an inline field slot for it makes the codegen field branch shadow the method on + // every read, so `this.method` reads the uninitialised slot (`undefined`) BEFORE + // the assignment runs — exactly what made `this.parse.bind(this)` throw "Bind must + // be called on a function" in zod. Mirrors the accessor exclusion (#665). + let mut method_names: std::collections::HashSet = std::collections::HashSet::new(); + for member in &class.body { + match member { + ast::ClassMember::Method(m) if matches!(m.kind, ast::MethodKind::Method) => { + let key = match &m.key { + ast::PropName::Ident(i) => i.sym.to_string(), + ast::PropName::Str(s) => s.value.as_str().unwrap_or("").to_string(), + _ => continue, + }; + method_names.insert(key); + } + ast::ClassMember::PrivateMethod(m) if matches!(m.kind, ast::MethodKind::Method) => { + method_names.insert(format!("#{}", m.key.name)); + } + _ => {} + } + } + // Issue #10487: pull in the parent chain's own+inherited method + // names too, mirroring the accessor union just above. A subclass + // constructor's `this.close = …` overriding a PARENT method (not + // redeclared on this class) must be recognized as a method + // override, not a new own data field, or the field wins the + // dynamic-dispatch lookup and instance reads see `undefined` + // until the assignment statement runs. + if let Some(parent_name) = parent_name { + if let Some(parent_methods) = ctx.lookup_class_method_names(parent_name) { + for m in parent_methods { + method_names.insert(m.clone()); + } + } + } + + if infer_fields { + infer_and_register_fields( + class, + name, + ctx, + inherited_field_names, + &accessor_names, + &method_names, + fields, + ); + } + + // Issue #665: register own+inherited accessor names so subclasses + // lowered after this one can also skip them when scanning ctor + // bodies. `accessor_names` already contains the getter/setter names + // from the parent-chain lookup above. For a class EXPRESSION this + // also lets the assignment recogniser in `expr_assign.rs` treat + // `C.prototype. = v` as a setter INVOCATION instead of a + // prototype-method monkey-patch (test262 accessor-name-inst setters). + ctx.register_class_accessor_names(name.to_string(), accessor_names); + + // Issue #10487: register this class's complete (own + inherited) + // method-name set, mirroring the accessor registration just above, + // so a further subclass lowered after this one sees the full + // chain in one lookup. + ctx.register_class_method_names(name.to_string(), method_names.into_iter().collect()); +} + +fn infer_and_register_fields( + class: &ast::Class, + name: &str, + ctx: &mut LoweringContext, + inherited_field_names: std::collections::HashSet, + accessor_names: &crate::class_accessors::ClassAccessorNames, + method_names: &std::collections::HashSet, + fields: &mut Vec, +) { + let declared_field_names: std::collections::HashSet = + fields.iter().map(|f| f.name.clone()).collect(); + for member in &class.body { + if let ast::ClassMember::Constructor(ctor) = member { + if let Some(ref body) = ctor.body { + for stmt in &body.stmts { + if let ast::Stmt::Expr(expr_stmt) = stmt { + let mut names: Vec = Vec::new(); + collect_this_field_assigns(&expr_stmt.expr, &mut names); + for fname in names { + if !declared_field_names.contains(&fname) + && !inherited_field_names.contains(&fname) + && !accessor_names.contains_any(&fname) + && !method_names.contains(&fname) + { + fields.push(ClassField { + name: fname, + key_expr: None, + ty: Type::Any, + init: None, + is_private: false, + is_readonly: false, + decorators: Vec::new(), + }); + } + } + } + } + } + } + } + // Dedup fields: keep first occurrence of each name + let mut seen = std::collections::HashSet::new(); + fields.retain(|f| seen.insert(f.name.clone())); + + // Register this class's complete field set (own + inherited) so subclasses that + // extend it can see the full inheritance chain during their own lowering. + let mut complete_field_names: Vec = inherited_field_names.into_iter().collect(); + for f in fields.iter() { + if !complete_field_names.contains(&f.name) { + complete_field_names.push(f.name.clone()); + } + } + ctx.register_class_field_names(name.to_string(), complete_field_names); +} + +// Pull each top-level `this. = …` field name out of one ctor +// statement-expression. Minified bundles (Next.js `BaseNextRequest`'s +// `constructor(a,b,c){this.method=a,this.url=b,this.body=c}`) collapse +// every ctor assignment into ONE comma-`Seq` expression-statement, so a +// scan that only matched `Expr::Assign` detected ZERO fields — the +// parent's `method`/`url`/`body` never entered `packed_keys`, leaving +// the subclass instance allocated with too-few inline slots so the +// captured-class shape prepends `__perry_cap_*` over the (missing) real +// slots and `e.url` reads undefined ("Invalid URL" 500 on dynamic page +// routes). Descend through `Seq` (and the `Paren`/`Assign`-result-chain +// wrappers minifiers emit) so each comma-separated `this.x = …` is +// recognised the same as a standalone assignment statement. +fn collect_this_field_assigns(expr: &ast::Expr, out: &mut Vec) { + match expr { + ast::Expr::Assign(assign) => { + // A chained assignment's RHS can itself be `this.x = …` + // (`this.a = this.b = v`): the inner `this.b = v` evaluates + // (and creates `b`'s slot) BEFORE the outer assignment to + // `this.a`, so collect the RHS first to keep Object.keys in + // the same insertion order Node produces (`b` then `a`). + collect_this_field_assigns(&assign.right, out); + if let ast::AssignTarget::Simple(ast::SimpleAssignTarget::Member(mem)) = &assign.left { + if let ast::Expr::This(_) = &*mem.obj { + if let ast::MemberProp::Ident(prop_ident) = &mem.prop { + out.push(prop_ident.sym.to_string()); + } + } + } + } + ast::Expr::Seq(seq) => { + for e in &seq.exprs { + collect_this_field_assigns(e, out); + } + } + ast::Expr::Paren(p) => collect_this_field_assigns(&p.expr, out), + _ => {} + } +} diff --git a/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs b/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs index c78736327c..c40cf1fe10 100644 --- a/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs +++ b/crates/perry-hir/src/lower_decl/class_decl/from_ast.rs @@ -669,23 +669,41 @@ pub(crate) fn lower_class_from_ast( } } - // Mirror `lower_class_decl`: register the union of this class's accessor - // names (own get/set, including private and the parent chain) so the - // assignment recogniser in `expr_assign.rs` treats `C.prototype. - // = v` as a setter INVOCATION instead of a prototype-method monkey-patch. - // `lower_class_decl` registers these for class declarations; without the - // parallel call here, a class EXPRESSION's instance setters (e.g. - // `var C = class { set ''(p){…} }; C.prototype[''] = v`) were silently - // dropped to `RegisterPrototypeMethod`. Test262 accessor-name-inst setters. - { - let mut accessor_names = runtime_instance_accessor_names(&class.body); - if let Some(ref parent_name) = extends_name { - if let Some(parent_accessors) = ctx.lookup_class_accessor_names(parent_name) { - accessor_names.extend_from(parent_accessors); - } - } - ctx.register_class_accessor_names(name.to_string(), accessor_names); - } + // Issue #10499: infer own fields from the constructor's top-level + // `this. = …` assignments exactly as `lower_class_decl` does, so a + // plain-JS class EXPRESSION (`var NodeObject = class {…}`) gets the same + // slot layout as the identical declaration instead of zero fields (every + // constructor store a property ADD through full `[[Set]]`). Also + // registers the class's field / accessor / method name sets. The + // accessor set matters beyond subclass lowering: the assignment + // recogniser in `expr_assign.rs` uses it to treat `C.prototype. + // = v` as a setter INVOCATION instead of a prototype-method monkey-patch + // (test262 accessor-name-inst setters, `var C = class { set ''(p){…} }`). + // + // Unlike a declaration, only infer when the parent's field layout is + // statically known: no heritage at all, or a statically resolved parent + // (`extends_name` with no dynamic `extends_expr`) whose complete field + // set is registered. A class expression is far more often built over a + // RUNTIME parent — a mixin's `class extends Base` parameter, a + // lexically-local binding (see the #5437 comment above), `extends + // pick()` — and there an own slot inferred for a name the parent's + // constructor also writes (`this.side = …` in both) would shadow the + // parent's value: the subclass reads its own still-undefined slot. + // Such a class also does not publish a field set, so a subclass lowered + // later sees its parent layout as unknown too. + let static_parent_fields_known = match (&class.super_class, &extends_name, &extends_expr) { + (None, _, _) => true, + (Some(_), Some(parent), None) => ctx.lookup_class_field_names(parent).is_some(), + _ => false, + }; + infer_ctor_this_fields( + ctx, + class, + name, + extends_name.as_deref(), + static_parent_fields_known, + &mut fields, + ); // Issue #740: synthesize __perry_cap_* capture machinery for class // expressions that reference enclosing-fn locals (e.g. `const Inner = diff --git a/test-files/test_gap_10499_class_expr_ctor_fields.ts b/test-files/test_gap_10499_class_expr_ctor_fields.ts new file mode 100644 index 0000000000..b7bded1fbb --- /dev/null +++ b/test-files/test_gap_10499_class_expr_ctor_fields.ts @@ -0,0 +1,200 @@ +// #10499: a class EXPRESSION's constructor `this. = …` stores must be +// inferred as own fields exactly like the identical class DECLARATION's +// (perf: slot stores instead of a `[[Set]]` property add per store). This +// file pins the observable semantics that inference must not change. + +// Plain base, the typescript.js `NodeObject` shape. +var NodeExpr: any = class { + constructor(kind: any, pos: any, end: any) { + this.pos = pos; + this.end = end; + this.kind = kind; + this.id = 0; + this.flags = 0; + this.parent = undefined; + } +}; +const n = new NodeExpr(7, 1, 2); +console.log(Object.keys(n).join(",")); +console.log(JSON.stringify(n)); +n.pos = 10; +n.extra = "x"; +console.log(n.pos, n.end, n.kind, n.extra, Object.keys(n).join(",")); + +// Insertion order follows execution order, including a chained assignment +// (`this.a = this.b = v` creates `b` first) and a minified comma sequence. +var Chain: any = class { + constructor(v: any) { + this.a = this.b = v; + (this.c = 1), (this.d = 2); + } +}; +console.log(Object.keys(new Chain(3)).join(",")); + +// Self-binding an own method in the constructor is a method override, not a +// data field that shadows the method before the assignment runs. +var Bound: any = class { + constructor() { + this.seen = typeof this.run; + this.run = this.run.bind(this); + } + run() { + return this.seen; + } +}; +const bound = new Bound(); +console.log(bound.run(), Object.keys(bound).join(",")); + +// An accessor written from the constructor goes through the setter. +var WithAccessor: any = class { + constructor(p: any) { + this._p = 0; + this.points = p; + } + set points(v: any) { + this._p = v * 2; + } + get points() { + return this._p; + } +}; +const acc = new WithAccessor(4); +console.log(acc.points, Object.keys(acc).join(",")); + +// Class-expression subclass of a class-expression base: re-assigning a base +// field in the subclass must share the base's slot, not shadow it. +var Base: any = class { + constructor() { + this.kind = "base"; + this.shared = 1; + } + describe() { + return this.kind + ":" + this.shared; + } +}; +var Sub: any = class extends Base { + constructor() { + super(); + this.kind = "sub"; + this.own = 2; + } +}; +const sub = new Sub(); +console.log(sub.describe(), sub.own, Object.keys(sub).join(",")); +console.log(sub instanceof Base, sub instanceof Sub); + +// Class-DECLARATION subclass of a class-expression base. +class DeclSub extends NodeExpr { + constructor() { + super(1, 2, 3); + this.pos = 99; + this.tag = "decl"; + } +} +const ds: any = new DeclSub(); +console.log(ds.pos, ds.end, ds.tag, Object.keys(ds).join(",")); + +// Class-expression subclass of a class declaration. +class DeclBase { + constructor() { + (this as any).x = 1; + } +} +var ExprSub: any = class extends DeclBase { + constructor() { + super(); + this.x = 5; + this.y = 6; + } +}; +const es = new ExprSub(); +console.log(es.x, es.y, Object.keys(es).join(",")); + +// Parent chosen at runtime: the subclass must not assume the parent's layout. +var Left: any = class { + constructor() { + this.side = "left"; + this.l = 1; + } +}; +var Right: any = class { + constructor() { + this.side = "right"; + this.r = 2; + } +}; +function pick(left: boolean): any { + return left ? Left : Right; +} +// Two distinct sites, each with its own runtime parent (#11042 covers one +// site re-evaluated over different parents). +const DynL: any = class extends pick(true) { + constructor() { + super(); + this.dyn = "d"; + this.side = this.side + "!"; + } +}; +const DynR: any = class extends pick(false) { + constructor() { + super(); + this.dyn = "d"; + this.side = this.side + "!"; + } +}; +for (const d of [new DynL(), new DynR()]) { + console.log(d.side, d.l, d.r, d.dyn, Object.keys(d).join(",")); +} + +// Mixin: the parent is a parameter, unknown until the call. +function Tagged(BaseClass: any): any { + return class extends BaseClass { + constructor(...args: any[]) { + super(...args); + this.tagged = true; + this.kind = "tagged-" + this.kind; + } + }; +} +const TaggedNode = Tagged(NodeExpr); +const tn = new TaggedNode(4, 5, 6); +console.log(tn.kind, tn.pos, tn.tagged, Object.keys(tn).join(",")); + +// Class expression built inside a factory, capturing a local. +function makeClass(tag: string): any { + return class { + constructor(v: any) { + this.value = v; + this.tag = tag; + } + show() { + return this.tag + "=" + this.value; + } + }; +} +const A = makeClass("a"); +const B = makeClass("b"); +console.log(new A(1).show(), new B(2).show(), Object.keys(new A(3)).join(",")); + +// `module.exports = class …`-style anonymous expression in an object slot. +const exportsLike: any = {}; +exportsLike.Res = class { + constructor(points: any) { + this.remainingPoints = points; + this.consumedPoints = 0; + } +}; +const res = new exportsLike.Res(5); +console.log(res.remainingPoints, res.consumedPoints, Object.keys(res).join(",")); + +// Hot loop over many instances stays correct. +let s = 0; +const ring: any[] = new Array(64).fill(null); +for (let i = 0; i < 20000; i++) { + const node = new NodeExpr(i & 63, -1, -1); + node.pos = i; + node.end = i + 5; + ring[i & 63] = node; + s = (s + node.kind + node.end) % 1000003; +} +console.log(s, ring[5].pos, ring[5].kind); From cef74868f754df9f9f5a763241694a8def0b6e86 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 05:01:31 +0000 Subject: [PATCH 2/2] docs: changelog fragment for #11369 --- changelog.d/11369-class-expr-ctor-field-inference.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog.d/11369-class-expr-ctor-field-inference.md diff --git a/changelog.d/11369-class-expr-ctor-field-inference.md b/changelog.d/11369-class-expr-ctor-field-inference.md new file mode 100644 index 0000000000..32d028fa84 --- /dev/null +++ b/changelog.d/11369-class-expr-ctor-field-inference.md @@ -0,0 +1,4 @@ +- **perf(hir): class expressions now infer constructor `this.x = …` fields like declarations (#10499).** Only `lower_class_decl` scanned the constructor for top-level `this. = …` stores. A plain-JS class expression (`var NodeObject = class {…}`, `module.exports = class X {…}`, esbuild/rollup `var X = class {…}`) was lowered with `fields: 0`, so each constructor store became a property add through full `[[Set]]` (≈ 4,800 instructions). `new` on typescript.js's `NodeObject` shape ran 159× slower than Node, against 7× for the identical declaration. The scan, with its declared/inherited/accessor/method exclusions, now lives in `crates/perry-hir/src/lower_decl/class_decl/ctor_fields.rs`, and both `lower_class_decl` and `lower_class_from_ast` call it. On the issue's microbenchmark (perry-dev, N = 1M), `expr` went from ~1,090 ms to ~127 ms, the same as `decl`. + - A class expression infers fields only when its parent layout is statically known: no heritage, or a static parent with a registered field set. Over a runtime parent (a mixin parameter, a lexically-local binding, `extends f()`), an inferred own slot would hide the value the parent's constructor stored. + - Some heritage is a plain identifier that takes the dynamic path. For those, the parent's registered field/accessor/method sets are now looked up under the alias-resolved identifier and excluded. This also fixes a declaration subclass of a class-expression base reading the base's fields as `undefined` (`class Sub extends Base { constructor() { super(); this.side += "!" } }` printed `undefined!`). + - Tests: `test-files/test_gap_10499_class_expr_ctor_fields.ts` and `crates/perry-hir/src/lower/tests/class_expr_ctor_fields.rs`. An A/B over all 387 class-using gap tests gave byte-identical output on `main` and the fix.