Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ pub unsafe extern "C" fn js_register_class_computed_method(
setters: HashMap::new(),
});
vtable.methods.insert(
name,
name.clone(),
VTableMethodEntry {
func_ptr: func_ptr as usize,
param_count: param_count as u32,
Expand All @@ -662,6 +662,10 @@ pub unsafe extern "C" fn js_register_class_computed_method(
has_rest: has_rest != 0,
},
);
// Backfill when reflection already materialized `C.prototype`.
drop(registry);
let proto = class_decl_prototype_object(class_id);
super::state::install_class_decl_prototype_method_field(proto, class_id, &name);
}
VTABLE_GEN.fetch_add(1, Ordering::Release);
}
Expand Down
56 changes: 43 additions & 13 deletions crates/perry-runtime/src/object/class_registry/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,28 +580,58 @@ pub(crate) fn class_decl_prototype_method_names(class_id: u32) -> Vec<String> {
let mut names = Vec::new();
if let Ok(registry) = CLASS_VTABLE_REGISTRY.read() {
if let Some(vtable) = registry.as_ref().and_then(|reg| reg.get(&class_id)) {
names.extend(
vtable
.methods
.keys()
.filter(|name| *name != "constructor")
.cloned(),
);
// The real class constructor is stored in `Class::constructor`,
// not in the instance-method vtable. An entry named
// `"constructor"` here is therefore an ordinary method, most
// notably `class C { ["constructor"]() {} }`. It must replace the
// implicit `C.prototype.constructor` data property when the
// reflective prototype object is materialized.
names.extend(vtable.methods.keys().cloned());
}
}
names.sort();
names.dedup();
names
}

pub(super) fn install_class_decl_prototype_method_field(
proto: *mut ObjectHeader,
class_id: u32,
name: &str,
) {
if proto.is_null() {
return;
}
let scope = crate::gc::RuntimeHandleScope::new();
let proto_handle = scope.root_raw_mut_ptr(proto);
// Do not bind by reading the prototype object here. Its implicit
// `constructor` data property would shadow a computed method with that
// name and make the installation write the class constructor straight
// back. The canonical vtable value is the property value we need.
let method_handle =
scope.root_nanbox_f64(class_prototype_method_value_for_name(class_id, name));
let key_handle = scope.root_string_ptr(crate::string::js_string_from_bytes(
name.as_ptr(),
name.len() as u32,
));
let method = method_handle.get_nanbox_f64();
proto_handle.with_mut_ptr::<ObjectHeader, _>(|proto| {
key_handle.with_const_ptr::<crate::StringHeader, _>(|key| {
js_object_set_field_by_name(proto, key, method)
})
});
proto_handle.with_mut_ptr::<ObjectHeader, _>(|proto| {
set_builtin_property_attrs(
proto as usize,
name.to_string(),
PropertyAttrs::new(true, false, true),
)
});
}

fn install_class_decl_prototype_method_fields(proto: *mut ObjectHeader, class_id: u32) {
let proto_value = crate::value::js_nanbox_pointer(proto as i64);
for name in class_decl_prototype_method_names(class_id) {
let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32);
let leaked: &'static [u8] = name.as_bytes().to_vec().leak();
let method = js_class_method_bind(proto_value, leaked.as_ptr(), leaked.len());
js_object_set_field_by_name(proto, key, method);
set_builtin_property_attrs(proto as usize, name, PropertyAttrs::new(true, false, true));
install_class_decl_prototype_method_field(proto, class_id, &name);
}
}

Expand Down
23 changes: 22 additions & 1 deletion crates/perry-runtime/src/symbol/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,34 @@ pub unsafe extern "C" fn js_object_get_own_property_symbols(obj_f64: f64) -> i64
if obj_key == 0 {
return crate::array::js_array_alloc(0) as i64;
}
// A declared class prototype is a materialized ObjectHeader, while its
// computed Symbol methods/accessors live in the class registry. Seed the
// ordinary-object enumeration with those own keys so
// `Object.getOwnPropertySymbols(C.prototype)` sees `[sym]() {}` exactly as
// direct `C.prototype[sym]` dispatch does. A later assignment to the same
// symbol is deduplicated below; class elements precede such assignments in
// property-creation order.
let mut entries: Vec<(usize, u64)> = crate::object::class_id_for_decl_prototype_object(obj_key)
.map(|class_id| {
crate::object::class_own_symbol_member_keys(class_id, false)
.into_iter()
.map(|sym_key| (sym_key, 0))
.collect()
})
.unwrap_or_default();

let guard = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES);
let mut entries = guard
let stored_entries = guard
.as_ref()
.and_then(|m| m.get(&obj_key))
.cloned()
.unwrap_or_default();
drop(guard);
for entry in stored_entries {
if !entries.iter().any(|(sym_key, _)| *sym_key == entry.0) {
entries.push(entry);
}
}
// `entries` is the full own-symbol-key list in property-CREATION order:
// data entries hold their value, accessor properties hold an
// order-preserving placeholder written by `set_symbol_accessor_property`
Expand Down
15 changes: 10 additions & 5 deletions crates/perry-runtime/src/value/dyn_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,16 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 {
);
}
}
let idx_i32 = if index.is_nan() || index.is_infinite() {
return f64::from_bits(TAG_UNDEFINED);
} else {
index as i32
};
// NaN and +/-Infinity are not array indices, but they are still ordinary
// property keys (`"NaN"`, `"Infinity"`, `"-Infinity"`) on Objects and
// Arrays. Delegate this cold case to the polymorphic key path, which runs
// ToPropertyKey and already distinguishes ordinary from integer-indexed
// exotic receivers. The old early return made a computed definition such
// as `{ [Infinity]: value }` unreadable through `obj[Infinity]`.
if index.is_nan() || index.is_infinite() {
return crate::object::js_object_get_index_polymorphic(raw_ptr as i64, index);
}
let idx_i32 = index as i32;
if idx_i32 >= 0 {
if let Some(value) = unsafe {
crate::object::arguments_object_get_index(
Expand Down
46 changes: 46 additions & 0 deletions test-files/test_issue_5894_computed_property_names.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Issue #5894: computed property keys must stay visible through the same
// reflective surfaces as non-computed properties.

const sym1 = Symbol("one");
const sym2 = Symbol("two");

class C {
["constructor"](): number {
return 1;
}

[sym1](): string {
return "first";
}

[((value: symbol): symbol => value)(sym2)](): string {
return "second";
}
}

const instance = new C();
const prototypeSymbols = Object.getOwnPropertySymbols(C.prototype);
console.log(C === C.prototype.constructor);
console.log(
Object.getOwnPropertyDescriptor(C.prototype, "constructor")?.value === C,
);
console.log(instance.constructor());
console.log(instance[sym1]());
console.log(instance[sym2]());
console.log(prototypeSymbols.length);
console.log(prototypeSymbols[0] === sym1);
console.log(prototypeSymbols[1] === sym2);

const numericKeys = {
[1.2]: "finite",
[-0]: "zero",
[Infinity]: "positive infinity",
[-Infinity]: "negative infinity",
[NaN]: "not a number",
};

console.log(numericKeys[1.2]);
console.log(numericKeys[-0]);
console.log(numericKeys[Infinity]);
console.log(numericKeys[-Infinity]);
console.log(numericKeys[NaN]);
Loading