diff --git a/changelog.d/11672-class-static-setter-value.md b/changelog.d/11672-class-static-setter-value.md new file mode 100644 index 0000000000..a521c35cd1 --- /dev/null +++ b/changelog.d/11672-class-static-setter-value.md @@ -0,0 +1 @@ +- **fix(runtime): a class static setter receives the assigned value, not the class (#11669).** Since #11651, a class's static accessors are accessor properties of the class function object. Their closure halves were built with the instance thunks, which call `raw(this, value)`. A compiled static setter is `fn(value)`, so every write through the property (`C.x = v`, a dynamic receiver, a subclass, `Reflect.set`, `Object.assign`, a reflected `.set.call`) passed the class itself as the value. Static accessor halves now use static thunks, which arm static `this`, push the declaring class as the private-static owner, and call `fn()` / `fn(value)`. This restores `test_gap_10480_define_property_generic_descriptor_accessors` and `test_gap_11499_class_object_static_write`, and adds `test_gap_11669_class_static_setter_value`. diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 0aa58f0f63..7bc2302555 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -199,7 +199,7 @@ pub(crate) use gc_roots::{ pub(crate) use registration::{ class_accessor_function_value, class_accessor_source_func_ptr, class_own_accessor_ptrs, class_own_setter_length, class_registered_static_accessor_ptrs, - invalidate_class_string_member_order, + class_static_accessor_function_value, invalidate_class_string_member_order, }; pub use registration::{ is_class_id_registered, js_register_class_getter, js_register_class_method, diff --git a/crates/perry-runtime/src/object/class_registry/registration.rs b/crates/perry-runtime/src/object/class_registry/registration.rs index b05650543f..a6499bee71 100644 --- a/crates/perry-runtime/src/object/class_registry/registration.rs +++ b/crates/perry-runtime/src/object/class_registry/registration.rs @@ -263,6 +263,77 @@ extern "C" fn class_accessor_setter_thunk( f(this, value) } +static CLASS_STATIC_ACCESSOR_GETTER_THUNK_INFO: crate::closure::JsFunctionInfo = + crate::closure::JsFunctionInfo::of( + class_static_accessor_getter_thunk + as crate::codegen_abi::JsBody0, + ); +static CLASS_STATIC_ACCESSOR_SETTER_THUNK_INFO: crate::closure::JsFunctionInfo = + crate::closure::JsFunctionInfo::of( + class_static_accessor_setter_thunk + as crate::codegen_abi::JsBody1, + ); + +/// Trampoline for a raw STATIC getter (`fn() -> f64`, #11669). A compiled +/// static accessor body takes no `this` parameter: it reads `this` from the +/// static-this override and its private statics from the owner stack, exactly +/// as `class_value::class_static_accessor_call_get` arms them. Capture 1 is the +/// declaring class id (the private-static owner). +extern "C" fn class_static_accessor_getter_thunk( + closure: *const crate::closure::ClosureHeader, + this: crate::closure::JsThis, +) -> f64 { + let raw = crate::closure::js_closure_get_capture_ptr(closure, 0) as usize; + if raw == 0 { + return f64::from_bits(crate::value::TAG_UNDEFINED); + } + let this = this.as_f64(); + let owner = static_accessor_thunk_owner(closure, this); + crate::object::static_this_arm_if_unarmed(this); + crate::object::static_private_owner_push(owner); + let f = unsafe { crate::closure::body_call::js_bare_body_fn!(raw as *const u8;) }; + let result = f(); + crate::object::static_private_owner_pop(); + crate::object::static_this_disarm(); + result +} + +/// Trampoline for a raw STATIC setter (`fn(value) -> f64`, #11669). The +/// instance trampoline calls `raw(this, value)`, which hands a static setter +/// the receiver (the class itself) as its value; see +/// [`class_static_accessor_getter_thunk`] for the `this`/owner protocol. +extern "C" fn class_static_accessor_setter_thunk( + closure: *const crate::closure::ClosureHeader, + this: crate::closure::JsThis, + value: f64, +) -> f64 { + let raw = crate::closure::js_closure_get_capture_ptr(closure, 0) as usize; + if raw == 0 { + return f64::from_bits(crate::value::TAG_UNDEFINED); + } + let this = this.as_f64(); + let owner = static_accessor_thunk_owner(closure, this); + crate::object::static_this_arm_if_unarmed(this); + crate::object::static_private_owner_push(owner); + let f = unsafe { crate::closure::body_call::js_bare_body_fn!(raw as *const u8; value) }; + let _ = f(value); + crate::object::static_private_owner_pop(); + crate::object::static_this_disarm(); + f64::from_bits(crate::value::TAG_UNDEFINED) +} + +/// The private-static owner of a static accessor trampoline: the class +/// function object of the declaring class id in capture 1 (a Number, never a +/// heap word), or `this` when absent. +fn static_accessor_thunk_owner(closure: *const crate::closure::ClosureHeader, this: f64) -> f64 { + let cid = crate::closure::js_closure_get_capture_f64(closure, 1); + if cid.is_finite() && cid >= 1.0 { + crate::object::class_value::class_value(cid as u32) + } else { + this + } +} + /// Return the raw getter/setter body wrapped by a descriptor-reflection /// accessor closure. The wrapper has its own calling-convention thunk, while /// Function.prototype.toString must consult the original MethodDefinition. @@ -275,6 +346,8 @@ pub(crate) unsafe fn class_accessor_source_func_ptr( let thunk = (*closure).code(); if thunk != class_accessor_getter_thunk as *const u8 && thunk != class_accessor_setter_thunk as *const u8 + && thunk != class_static_accessor_getter_thunk as *const u8 + && thunk != class_static_accessor_setter_thunk as *const u8 { return None; } @@ -297,20 +370,48 @@ pub(crate) fn class_accessor_function_value( is_setter: bool, prop_name: &str, setter_length: Option, +) -> f64 { + accessor_function_value(raw_ptr, is_setter, prop_name, setter_length, None) +} + +/// [`class_accessor_function_value`] for a STATIC accessor of `class_id`: the +/// compiled entry takes no `this` parameter (`fn() -> f64` / `fn(v)`), so the +/// closure uses the static trampolines (#11669). +pub(crate) fn class_static_accessor_function_value( + raw_ptr: usize, + is_setter: bool, + prop_name: &str, + setter_length: Option, + class_id: u32, +) -> f64 { + accessor_function_value(raw_ptr, is_setter, prop_name, setter_length, Some(class_id)) +} + +fn accessor_function_value( + raw_ptr: usize, + is_setter: bool, + prop_name: &str, + setter_length: Option, + static_class_id: Option, ) -> f64 { if raw_ptr == 0 { return f64::from_bits(crate::value::TAG_UNDEFINED); } - let thunk = if is_setter { - &CLASS_ACCESSOR_SETTER_THUNK_INFO - } else { - &CLASS_ACCESSOR_GETTER_THUNK_INFO + let thunk = match (static_class_id.is_some(), is_setter) { + (false, true) => &CLASS_ACCESSOR_SETTER_THUNK_INFO, + (false, false) => &CLASS_ACCESSOR_GETTER_THUNK_INFO, + (true, true) => &CLASS_STATIC_ACCESSOR_SETTER_THUNK_INFO, + (true, false) => &CLASS_STATIC_ACCESSOR_GETTER_THUNK_INFO, }; - let closure = crate::closure::js_closure_alloc(thunk, 1); + let captures = if static_class_id.is_some() { 2 } else { 1 }; + let closure = crate::closure::js_closure_alloc(thunk, captures); if closure.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } crate::closure::js_closure_set_capture_ptr(closure, 0, raw_ptr as i64); + if let Some(cid) = static_class_id { + crate::closure::js_closure_set_capture_f64(closure, 1, cid as f64); + } // Spec `.length`: params before the first default/rest. A getter takes no // params (0); a setter takes exactly one formal param — but `set m(x = 42)` // has `.length === 0` (defaults don't count). Codegen records the setter's diff --git a/crates/perry-runtime/src/object/class_value.rs b/crates/perry-runtime/src/object/class_value.rs index eb2cf4f30e..4779e87ca3 100644 --- a/crates/perry-runtime/src/object/class_value.rs +++ b/crates/perry-runtime/src/object/class_value.rs @@ -604,11 +604,12 @@ fn install_declared_static_accessor(class_id: u32, name: &str) { } else { None }; - super::class_registry::class_accessor_function_value( + super::class_registry::class_static_accessor_function_value( raw, is_setter, name, setter_length, + class_id, ) .to_bits() } diff --git a/test-files/test_gap_11669_class_static_setter_value.ts b/test-files/test_gap_11669_class_static_setter_value.ts new file mode 100644 index 0000000000..228d44e877 --- /dev/null +++ b/test-files/test_gap_11669_class_static_setter_value.ts @@ -0,0 +1,62 @@ +// #11669: a class STATIC accessor's reflected function value (the closure an +// accessor property of the class function object holds since #11651) must +// call the compiled static entry with the static convention: the setter gets +// the assigned VALUE, and `this` is the receiver. +class Base { + static log: string[] = []; + static #hidden = 0; + static _n = 0; + static get n(): number { + return this._n; + } + static set n(v: number) { + this._n = v * 2; + Base.log.push("set " + this.name + " " + String(v)); + } + static get hidden(): number { + return Base.#hidden; + } + static set hidden(v: number) { + Base.#hidden = v + 100; + } + static set withReturn(v: number) { + Base.log.push("withReturn " + String(v)); + return; + } +} +class Sub extends Base {} + +// Direct, dynamic-receiver and computed-key writes. +Base.n = 3; +console.log("direct", Base.n, Base._n); +let D: any = Sub; +D.n = 4; +console.log("dynamic-sub", Sub._n, Object.prototype.hasOwnProperty.call(Sub, "_n"), Base._n); +const k = "n"; +(Base as any)[k] = 5; +console.log("computed", Base.n); + +// Private static reached from a setter invoked through a subclass. +D.hidden = 7; +console.log("private", Base.hidden, D.hidden); + +// A setter whose body returns: the assignment expression is still the value. +const r = ((Base as any).withReturn = 9); +console.log("assign-expr", r); + +// The reflected halves called explicitly. +const desc = Object.getOwnPropertyDescriptor(Base, "n")!; +desc.set!.call(Base, 11); +console.log("reflected-set", Base._n, desc.get!.call(Base)); +desc.set!.call(Sub, 12); +console.log("reflected-set-sub", Sub._n, desc.get!.call(Sub)); +Reflect.set(Base, "n", 13); +console.log("reflect-set", Base._n); +Object.assign(Base, { n: 14 }); +console.log("object-assign", Base._n); + +// After a generic defineProperty (attributes only) the accessor still works. +Object.defineProperty(Base, "n", { enumerable: true }); +Base.n = 15; +console.log("after-define", Base.n, Object.keys(Base).includes("n")); +console.log(Base.log.join("|"));