Skip to content

Commit c740f88

Browse files
proggeramlugRalph Küpper
andauthored
fix(runtime): a generic specialization answers with its generic's constructor and prototype entries (#7826)
* fix(runtime): a generic specialization answers with its generic's constructor and prototype entries `new Gen<number>()` is monomorphized into a separate class (`Gen$num`, `monomorph::mangle::generate_specialized_name`) with its own class id, and the instance is stamped with THAT id. TypeScript erases type arguments, so at runtime there is exactly one `Gen` — the specializations are an implementation detail that was leaking through the id-keyed surfaces. #7575 fixed `instanceof`, #7632 fixed `constructor.name`, #7762 fixed the two prototype-object registries. Two holes remained, both keyed on the raw id: 1. THE CONSTRUCTOR VALUE. `class_object_props`'s instance arm synthesized the class ref straight from `(*obj).class_id`, so `a.constructor !== Gen` and `a.constructor !== b.constructor`. #7632 made this WORSE before better: both report the name `Gen`, so two values printed identically and compared unequal. 2. THE PROPERTY LOOKUP CHAIN — the one #7762's prototype-object aliasing did not reach, and the more damaging of the pair. `lookup_prototype_method` walked the PARENT chain from the specialization's id, so a patch on `Gen.prototype` was invisible on a specialized instance: `Gen.prototype.tag = "G"` then `a.tag` gave `undefined` while `Object.getPrototypeOf(a) === Gen.prototype` reported `true`. The two edges disagreed about the same object. Both take the origin edge the other three surfaces already take. In the chain walk the generic is tried BEFORE the parent, because it is an alias rather than an ancestor — a specialization's parent chain is its generic's parent chain, so hopping to the parent first would walk past `Gen` and never come back. That also keeps the walk line-count-neutral, which `construct.rs` requires: it sits exactly at the 2000-line cap. METHOD DISPATCH IS DELIBERATELY NOT ALIASED. It runs off the per-class-id vtable, so each specialization keeps its own monomorphized bodies — the same boundary #7762 drew, and the reason this is not a `CLASS_REGISTRY` parent edge (that chain also resolves `super()` construction and would re-run the wrong constructor). `test-files/test_gap_generic_specialization_constructor_identity_7757.ts` is byte-identical to node. It pins the edge in the negative direction too: two different generics stay distinct, a specialized SUBCLASS reports the subclass rather than the base, and declared methods still dispatch per specialization. Fixes #7757 * docs(changelog): add fragment for the #7757 specialization identity fix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 3eea8c7 commit c740f88

4 files changed

Lines changed: 120 additions & 2 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
**A generic class specialization now answers with its generic's constructor, and sees its generic's prototype patches.**
2+
`new Gen<number>()` is monomorphized into a separate class (`Gen$num`) with its
3+
own class id; TypeScript erases type arguments, so at runtime there is exactly
4+
one `Gen`. After `instanceof` (#7575), `constructor.name` (#7632) and the two
5+
prototype-object registries (#7762), two id-keyed holes remained. The instance
6+
`.constructor` arm synthesized the class ref straight from the instance's class
7+
id, so `a.constructor !== Gen` and `a.constructor !== b.constructor` — and since
8+
#7632 gave both the display name `Gen`, they printed identically while comparing
9+
unequal. Separately, and untouched by #7762's aliasing of the prototype
10+
*objects*, the prototype-method chain walk started at the specialization's id
11+
and followed only parent edges, so `Gen.prototype.tag = "G"` was invisible from a
12+
specialized instance even though `Object.getPrototypeOf(a) === Gen.prototype`
13+
reported true — the two edges disagreed about the same object. Both now take the
14+
same generic-origin edge, with the generic tried *before* the parent because it
15+
is an alias rather than an ancestor. Method dispatch is deliberately not aliased:
16+
it runs off the per-class-id vtable, so each specialization keeps its own
17+
monomorphized bodies. Covered by
18+
`test-files/test_gap_generic_specialization_constructor_identity_7757.ts`, which
19+
also pins the negative direction — distinct generics stay distinct, a specialized
20+
subclass reports the subclass rather than its base, and `instanceof` still
21+
discriminates. (#7757)

crates/perry-runtime/src/object/class_registry/construct.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1934,7 +1934,7 @@ pub(crate) fn lookup_prototype_method(class_id: u32, name: &str) -> Option<f64>
19341934
return Some(f64::from_bits(bits));
19351935
}
19361936
}
1937-
match get_parent_class_id(cid) {
1937+
match crate::object::class_generic_origin(cid).or_else(|| get_parent_class_id(cid)) {
19381938
Some(p) if p != 0 && p != cid => {
19391939
cid = p;
19401940
depth += 1;

crates/perry-runtime/src/object/field_get_set/class_object_props.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,22 @@ pub(super) unsafe fn instance_constructor_value(
5454
if let Some(v) = own_data_field_by_name(obj, key) {
5555
return Some(v);
5656
}
57-
let class_id = (*obj).class_id;
57+
// #7757: a monomorphized specialization must present the GENERIC's
58+
// reflective surface. `new Gen<number>()` is stamped with `Gen$num`'s id
59+
// (`monomorph::mangle::generate_specialized_name`), so every arm below
60+
// answered with the specialization -- making `a.constructor !== Gen` and,
61+
// worse after #7632 gave both the same display name, two constructors that
62+
// PRINT identically and compare unequal. TypeScript erases type arguments:
63+
// at runtime there is exactly one `Gen`.
64+
//
65+
// This is the third and last identity surface to take the same origin edge
66+
// `instanceof` uses (#7575), the display name uses (#7632) and the two
67+
// prototype registries use (#7762). METHOD DISPATCH is deliberately not
68+
// aliased -- it runs off the per-class-id vtable, so each specialization
69+
// keeps its own monomorphized bodies.
70+
let class_id = crate::object::class_generic_origin((*obj).class_id)
71+
.filter(|generic| is_class_id_registered(*generic))
72+
.unwrap_or((*obj).class_id);
5873
// #6530: a capture-carrying class has no ClassRef value — the
5974
// class VALUE is the per-evaluation class OBJECT registered at
6075
// `js_object_mark_class` time. Return that same object so
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Perry monomorphizes a generic class: `new Gen<number>()` is stamped with a
2+
// SEPARATE class id (`monomorph::mangle::generate_specialized_name` →
3+
// `Gen$num`). TypeScript erases type arguments, so at runtime there is exactly
4+
// one `Gen`, one `Gen.prototype` and one `Gen.prototype.constructor` — the
5+
// specializations are an implementation detail that must not reach any
6+
// id-keyed identity or lookup surface.
7+
//
8+
// Three faces of the same leak were fixed one at a time: `instanceof` (#7575),
9+
// `constructor.name` (#7632), and the two prototype-object registries (#7762).
10+
// This file pins the two that remained — the CONSTRUCTOR VALUE, and the
11+
// PROPERTY LOOKUP chain, which is the one that made a prototype patch
12+
// invisible on a specialized instance.
13+
//
14+
// #7632 made the constructor case worse before it got better: two constructors
15+
// that print identically and compare unequal is a harder failure to debug than
16+
// an obviously-wrong name.
17+
18+
class Gen<T> {
19+
v: T | undefined;
20+
tell(): string {
21+
return "gen";
22+
}
23+
}
24+
25+
const a = new Gen<number>();
26+
const b = new Gen<string>();
27+
const c = new Gen(); // never specialized
28+
29+
// --- constructor identity ---------------------------------------------------
30+
console.log("a===Gen:", a.constructor === Gen);
31+
console.log("b===Gen:", b.constructor === Gen);
32+
console.log("c===Gen:", c.constructor === Gen);
33+
console.log("a===b:", a.constructor === b.constructor);
34+
console.log("name:", (a.constructor as any).name);
35+
36+
// The prototype edge and the constructor edge must AGREE. They disagreed for a
37+
// release: `getPrototypeOf(a) === Gen.prototype` was already true while
38+
// `a.constructor === Gen` was false.
39+
console.log("proto===Gen.prototype:", Object.getPrototypeOf(a) === Gen.prototype);
40+
console.log("protoA===protoB:", Object.getPrototypeOf(a) === Object.getPrototypeOf(b));
41+
console.log("a.ctor===proto.ctor:", a.constructor === (Object.getPrototypeOf(a) as any).constructor);
42+
43+
// --- prototype LOOKUP, not just prototype identity --------------------------
44+
// A patch on `Gen.prototype` must be visible from a specialized instance. It
45+
// was not: the instance's chain walk keyed on the specialization's id and never
46+
// reached the generic's entries, so this read was `undefined` while
47+
// `getPrototypeOf(a) === Gen.prototype` reported true.
48+
(Gen.prototype as any).patched = "P";
49+
(Gen.prototype as any).patchedMethod = function (this: any) {
50+
return "pm";
51+
};
52+
console.log("a.patched:", (a as any).patched, (b as any).patched, (c as any).patched);
53+
console.log("a.patchedMethod():", (a as any).patchedMethod(), (b as any).patchedMethod());
54+
55+
// Declared methods still work — they dispatch off the per-class-id vtable,
56+
// which is deliberately NOT aliased so each specialization keeps its own
57+
// monomorphized bodies.
58+
console.log("tell:", a.tell(), b.tell(), c.tell());
59+
60+
// --- the edge must not collapse everything into everything ------------------
61+
class Other<T> {
62+
w: T | undefined;
63+
}
64+
const o = new Other<number>();
65+
console.log("o===Other:", o.constructor === Other);
66+
console.log("o!==Gen:", o.constructor !== (Gen as any));
67+
console.log("a instanceof Gen:", a instanceof Gen, "| o instanceof Gen:", o instanceof Gen);
68+
69+
// A specialization of a SUBCLASS reports the subclass, not the base: the origin
70+
// edge is an alias for one class, not a walk up the parent chain.
71+
class Base {
72+
tag = "base";
73+
}
74+
class Sub<T> extends Base {
75+
u: T | undefined;
76+
}
77+
const s = new Sub<number>();
78+
console.log("s===Sub:", s.constructor === Sub, "| s!==Base:", s.constructor !== (Base as any));
79+
console.log("s instanceof Sub:", s instanceof Sub, "| s instanceof Base:", s instanceof Base);
80+
81+
// Constructing through the reported constructor round-trips.
82+
console.log("new a.ctor instanceof Gen:", new (a.constructor as any)() instanceof Gen);

0 commit comments

Comments
 (0)