Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions changelog.d/9226-class-prototype-own-keys.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
**Class prototypes now expose one coherent, spec-ordered own-key surface**
(#9226). `Object.getOwnPropertyNames(C.prototype)` omitted every accessor,
listed Perry's internal `@@iterator` dispatch alias as if it were a source
string key, and `Object.getOwnPropertySymbols` returned nothing — so
`Reflect.ownKeys` disagreed with `hasOwnProperty` and
`getOwnPropertyDescriptor`, both of which found the missing keys. A two-step
"list the keys, then inspect each one" walk — what decorators, DI containers,
serializers and test-framework method discovery all do — got self-contradictory
answers.

Three separate causes, not one. Accessors were missing because the names
builder read only `vtable.methods`, never `getters`/`setters`. The
`"@@iterator"` string was a *lowering* artifact: every well-known-symbol class
member was diverted away from the computed-key path and registered under a
synthetic string name, so the real Symbol key was never installed anywhere
`getOwnPropertySymbols` could see it. And the order was whatever the dispatch
hash maps happened to yield, because those maps are keyed for lookup speed and
carry no source position.

Only three well-known-symbol forms now need special lowering (a generator
`[Symbol.iterator]`, `static [Symbol.hasInstance]`, and a
`get [Symbol.toStringTag]`); the rest register a real Symbol key, with the
generator form registering both its dispatch wrapper and its Symbol key. The
synthetic dispatch aliases stay in the vtable for fast calls but are filtered
out of enumeration — a class with a source method literally named
`"@@iterator"` still lists it, because that one carries a definition-order
record and an alias does not. Definition order itself comes from the member
function's HIR id, allocated while walking the ClassBody, which is what lets
reflection reconstruct one source order across the separate method, getter,
setter and Symbol registries. Static fields keep first-install order, and a
class key that is deleted and recreated moves to the end, as `[[OwnPropertyKeys]]`
requires.

`Reflect.ownKeys` is now the union in spec order — integer-index strings
ascending, remaining strings in property-creation order, then Symbols in
property-creation order — with `getOwnPropertyNames` and
`getOwnPropertySymbols` as its two halves, and `hasOwnProperty` agreeing with
both for Symbol-keyed class members. A 41-assertion gap fixture that diverges
from `node --experimental-strip-types` on 36 of its 41 lines before the change
is byte-identical after, and a 104-assertion class-prototype differential goes
from 11 divergences to zero.
84 changes: 72 additions & 12 deletions crates/perry-codegen/src/codegen/string_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,14 +657,15 @@ pub(super) fn emit_string_pool(
// symbols for those live in the defining module's object file.
// Each module's init registers its own classes; the linker
// ensures all init functions run before main.
// (class_id, name, llvm_symbol, total_param_count, has_synth_args, has_rest, spec_length)
let mut method_triples: Vec<(u32, String, String, u32, bool, bool, u32)> = Vec::new();
// (class_id, name, llvm_symbol, total_param_count, has_synth_args,
// has_rest, spec_length, definition_order)
let mut method_triples: Vec<(u32, String, String, u32, bool, bool, u32, u32)> = Vec::new();
// #1788: (cid, static-method name, perry_static_* symbol, param_count,
// has_rest). Registered into the runtime CLASS_STATIC_METHODS table so a
// subclass whose parent is a class-expression value inherits the parent's
// static methods (`class Sub extends make(...) {}; Sub.greet()`); has_rest
// tells the dispatcher to bundle trailing args for a `...rest` param.
let mut static_method_triples: Vec<(u32, String, String, u32, bool, u32)> = Vec::new();
let mut static_method_triples: Vec<(u32, String, String, u32, bool, u32, u32)> = Vec::new();
// #1787: (cid, standalone-constructor symbol, total_param_count).
// Registered into CLASS_CONSTRUCTORS so `new <classObjectValue>()` (a
// class-expression value constructed dynamically) can replay the class's
Expand Down Expand Up @@ -759,6 +760,7 @@ pub(super) fn emit_string_pool(
has_synth_args,
has_rest,
spec_length,
method.id,
));
}
// #1788: static methods are emitted as `perry_static_*` (no `this`
Expand All @@ -785,6 +787,7 @@ pub(super) fn emit_string_pool(
sm.params.len() as u32,
has_rest,
spec_length,
sm.id,
));
}
// #1787: the standalone constructor `<prefix>__<class>_constructor`
Expand Down Expand Up @@ -857,8 +860,16 @@ pub(super) fn emit_string_pool(
ctor_triples.push((cid, ctor_symbol, ctor_params, ctor_sig_caps));
}
method_triples.sort_unstable();
for (cid, method_name, llvm_name, param_count, has_synth_args, has_rest, spec_length) in
method_triples
for (
cid,
method_name,
llvm_name,
param_count,
has_synth_args,
has_rest,
spec_length,
definition_order,
) in method_triples
{
chunker.roll_if_full();
let blk = chunker.current_block();
Expand Down Expand Up @@ -892,6 +903,16 @@ pub(super) fn emit_string_pool(
(I64, has_rest_str),
],
);
blk.call_void(
"js_register_class_string_member_order",
&[
(I64, &cid.to_string()),
(I64, &bytes_i64),
(I64, &len_str),
(I64, "0"),
(I64, &definition_order.to_string()),
],
);
// Record the default-aware spec `.length` so `C.prototype.m.length`
// reflects params-before-first-default, not the raw param count.
blk.call_void(
Expand All @@ -908,7 +929,9 @@ pub(super) fn emit_string_pool(
// static methods (subclass extends a class-expression value) resolve at
// runtime via the class_id parent-chain walk.
static_method_triples.sort_unstable();
for (cid, method_name, llvm_name, param_count, has_rest, spec_length) in static_method_triples {
for (cid, method_name, llvm_name, param_count, has_rest, spec_length, definition_order) in
static_method_triples
{
chunker.roll_if_full();
let blk = chunker.current_block();
let entry = match strings.iter().find(|e| e.value == method_name) {
Expand All @@ -932,6 +955,16 @@ pub(super) fn emit_string_pool(
(I64, has_rest_str),
],
);
blk.call_void(
"js_register_class_string_member_order",
&[
(I64, &cid.to_string()),
(I64, &bytes_i64),
(I64, &len_str),
(I64, "1"),
(I64, &definition_order.to_string()),
],
);
// Record the default-aware spec `.length` for the static method so
// `C.staticGen.length` reflects params-before-first-default rather than
// the raw param count (which over-counts generator/async methods).
Expand Down Expand Up @@ -1121,7 +1154,7 @@ pub(super) fn emit_string_pool(
// `undefined`.
// (class_id, prop_name, llvm_symbol, is_static) — static accessors register
// onto the class constructor (CLASS_STATIC_ACCESSORS), not the instance vtable.
let mut getter_pairs: Vec<(u32, String, String, bool)> = Vec::new();
let mut getter_pairs: Vec<(u32, String, String, bool, u32)> = Vec::new();
for (class_name, class) in classes.iter() {
// Refs #486: skip alias keys (see method-emission loop above).
if *class_name != class.name {
Expand Down Expand Up @@ -1166,11 +1199,11 @@ pub(super) fn emit_string_pool(
sanitize_member(&inner),
)
};
getter_pairs.push((cid, prop.clone(), llvm_name, is_static));
getter_pairs.push((cid, prop.clone(), llvm_name, is_static, getter_fn.id));
}
}
getter_pairs.sort_unstable();
for (cid, prop_name, llvm_name, is_static) in getter_pairs {
for (cid, prop_name, llvm_name, is_static, definition_order) in getter_pairs {
chunker.roll_if_full();
let blk = chunker.current_block();
let entry = match strings.iter().find(|e| e.value == prop_name) {
Expand All @@ -1196,6 +1229,16 @@ pub(super) fn emit_string_pool(
(I64, &func_i64),
],
);
blk.call_void(
"js_register_class_string_member_order",
&[
(I64, &cid.to_string()),
(I64, &bytes_i64),
(I64, &len_str),
(I64, if is_static { "1" } else { "0" }),
(I64, &definition_order.to_string()),
],
);
}

// Refs #486 (hono): parallel registration for class setters. Without
Expand All @@ -1215,7 +1258,7 @@ pub(super) fn emit_string_pool(
// class/setter-length-dflt): without a per-func-ptr length registration
// the runtime fell back to the setter's ABI arity (1), over-counting the
// defaulted param.
let mut setter_pairs: Vec<(u32, String, String, bool, u32)> = Vec::new();
let mut setter_pairs: Vec<(u32, String, String, bool, u32, u32)> = Vec::new();
for (class_name, class) in classes.iter() {
if *class_name != class.name {
continue;
Expand Down Expand Up @@ -1252,11 +1295,18 @@ pub(super) fn emit_string_pool(
)
};
let spec_length = spec_function_length(&setter_fn.params) as u32;
setter_pairs.push((cid, prop.clone(), llvm_name, is_static, spec_length));
setter_pairs.push((
cid,
prop.clone(),
llvm_name,
is_static,
spec_length,
setter_fn.id,
));
}
}
setter_pairs.sort_unstable();
for (cid, prop_name, llvm_name, is_static, spec_length) in setter_pairs {
for (cid, prop_name, llvm_name, is_static, spec_length, definition_order) in setter_pairs {
chunker.roll_if_full();
let blk = chunker.current_block();
let entry = match strings.iter().find(|e| e.value == prop_name) {
Expand Down Expand Up @@ -1291,6 +1341,16 @@ pub(super) fn emit_string_pool(
(I64, &func_i64),
],
);
blk.call_void(
"js_register_class_string_member_order",
&[
(I64, &cid.to_string()),
(I64, &bytes_i64),
(I64, &len_str),
(I64, if is_static { "1" } else { "0" }),
(I64, &definition_order.to_string()),
],
);
}

// Issue #493: register each rest-bearing closure body's func_ptr ->
Expand Down
6 changes: 6 additions & 0 deletions crates/perry-codegen/src/expr/static_field_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
is_static,
param_count,
has_rest,
definition_order,
} => {
let key_v = lower_expr(ctx, key_expr)?;
if let Some(&class_id) = ctx.class_ids.get(class_name) {
Expand All @@ -349,6 +350,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
let param_count_str = param_count.to_string();
let is_static_str = (*is_static as i64).to_string();
let has_rest_str = (*has_rest as i64).to_string();
let definition_order_str = definition_order.to_string();
ctx.block().call_void(
"js_register_class_computed_method",
&[
Expand All @@ -358,6 +360,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
(I64, &param_count_str),
(I64, &is_static_str),
(I64, &has_rest_str),
(I64, &definition_order_str),
],
);
}
Expand All @@ -371,6 +374,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
getter_name,
setter_name,
is_static,
definition_order,
} => {
let key_v = lower_expr(ctx, key_expr)?;
if let Some(&class_id) = ctx.class_ids.get(class_name) {
Expand Down Expand Up @@ -407,6 +411,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
.unwrap_or_else(|| "0".to_string());
let cid_str = class_id.to_string();
let is_static_str = (*is_static as i64).to_string();
let definition_order_str = definition_order.to_string();
ctx.block().call_void(
"js_register_class_computed_accessor",
&[
Expand All @@ -415,6 +420,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
(I64, &getter_i64),
(I64, &setter_i64),
(I64, &is_static_str),
(I64, &definition_order_str),
],
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,11 @@ pub(crate) fn declare_core(module: &mut LlModule) {
module.declare_function("js_register_class_getter", VOID, &[I64, I64, I64, I64]);
// Refs #486: per-class setter dispatch — see object.rs::js_register_class_setter.
module.declare_function("js_register_class_setter", VOID, &[I64, I64, I64, I64]);
module.declare_function(
"js_register_class_string_member_order",
VOID,
&[I64, I64, I64, I64, I64],
);
// Default-aware spec `.length` per class method (CLASS_METHOD_BIND_LENGTHS).
module.declare_function(
"js_register_class_method_bind_length",
Expand Down
4 changes: 2 additions & 2 deletions crates/perry-codegen/src/runtime_decls/strings_part2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,12 @@ pub(crate) fn declare_phase_b_strings_part2(module: &mut LlModule) {
module.declare_function(
"js_register_class_computed_method",
VOID,
&[I64, DOUBLE, I64, I64, I64, I64],
&[I64, DOUBLE, I64, I64, I64, I64, I64],
);
module.declare_function(
"js_register_class_computed_accessor",
VOID,
&[I64, DOUBLE, I64, I64, I64],
&[I64, DOUBLE, I64, I64, I64, I64],
);
// v0.5.747: register a string-named static field on a class so reads
// via the runtime dynamic-dispatch path (when the class ref is in an
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-hir/src/analysis/value_types_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1339,13 +1339,15 @@ fn infers_class_prototype_and_super_meta_value_shapes() {
is_static: false,
param_count: 0,
has_rest: false,
definition_order: 1,
},
Expr::RegisterClassComputedAccessor {
class_name: "Widget".to_string(),
key_expr: Box::new(Expr::String("value".to_string())),
getter_name: Some("getValue".to_string()),
setter_name: None,
is_static: false,
definition_order: 2,
},
] {
assert_eq!(infer_expr_type(&expr, &env), Type::Void);
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-hir/src/ir/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ pub enum Expr {
is_static: bool,
param_count: u32,
has_rest: bool,
definition_order: u32,
},

/// Register one side of a computed class accessor.
Expand All @@ -546,6 +547,7 @@ pub enum Expr {
getter_name: Option<String>,
setter_name: Option<String>,
is_static: bool,
definition_order: u32,
},

/// Issue #1772: per-evaluation identity for a class EXPRESSION
Expand Down
3 changes: 3 additions & 0 deletions crates/perry-hir/src/lower_decl/class_computed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,23 @@ pub(crate) fn class_computed_member_registration_expr(
.last()
.map(|p| p.is_rest)
.unwrap_or(false),
definition_order: member.function.id,
},
ClassComputedMemberKind::Getter => Expr::RegisterClassComputedAccessor {
class_name: class_name.to_string(),
key_expr: Box::new(member.key_expr.clone()),
getter_name: Some(member.function.name.clone()),
setter_name: None,
is_static: member.is_static,
definition_order: member.function.id,
},
ClassComputedMemberKind::Setter => Expr::RegisterClassComputedAccessor {
class_name: class_name.to_string(),
key_expr: Box::new(member.key_expr.clone()),
getter_name: None,
setter_name: Some(member.function.name.clone()),
is_static: member.is_static,
definition_order: member.function.id,
},
}
}
Expand Down
Loading
Loading