From a8b3b4e856c6618909674a1a261739b2ca5e2a64 Mon Sep 17 00:00:00 2001 From: Ralph Date: Fri, 19 Jun 2026 01:02:49 -0700 Subject: [PATCH] fix(class): delete prototype method removes its keys_array field (#5441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #5395 (#5395) made `delete C.prototype.` on a class-declaration prototype mark the vtable key deleted and early-return, so hasOwnProperty / getOwnPropertyDescriptor would agree the member is gone. That is correct for instance ACCESSORS (`get x()`), which live only in the class vtable — there is no keys_array entry to remove. Instance METHODS are different: `install_class_decl_prototype_method_fields` plants each method as a REAL keys_array data field on the prototype object (writable:true, enumerable:false, configurable:true). For a method the early return marked the vtable key deleted but left the real field in place, so hasOwnProperty kept finding it via `own_key_present`. test262's `verifyProperty` configurable probe (`delete obj[name]` then assert the key absent) therefore failed with "m descriptor should be configurable" — ~760 generated language/{statements,expressions}/class tests, which dropped language parity 94.5% → 89.3%. Fix: drop the early return so the class-prototype delete falls through to the keys_array scan. For accessors the scan is a vacuous success (no entry); for methods it removes the real field. The vtable key is still marked deleted, so both member kinds become invisible to the reflective paths. Validation (test262 radar, node v26): - language/statements/class: parity 94.0%, 0 "descriptor should be configurable" - language/expressions/class: parity 95.6%, 0 "descriptor should be configurable" - accessor prop-desc tests (#5395's targets) still pass; no regressions. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../perry-runtime/src/object/delete_rest.rs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 9613b259f..3aeaae7ea 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -179,12 +179,21 @@ pub extern "C" fn js_object_delete_field( } } // A class-declaration prototype object: instance accessors (`get x()`) - // and methods live in the class vtable, not the keys_array, so the scan - // below would "succeed vacuously" while the member stayed visible to - // hasOwnProperty / getOwnPropertyDescriptor. Record the key as deleted - // so those reflective paths agree it is gone (test262 verifyProperty's + // live ONLY in the class vtable, so the scan below would "succeed + // vacuously" while the accessor stayed visible to hasOwnProperty / + // getOwnPropertyDescriptor. Record the key as deleted so those + // reflective paths agree it is gone (test262 verifyProperty's // `configurable` check: `delete obj[name]` then assert the key absent — // class/definition/{getters,setters}-prop-desc). + // + // Instance METHODS are different: `install_class_decl_prototype_method_fields` + // plants each one as a REAL keys_array data field on the prototype + // object (writable:true, enumerable:false, configurable:true). Marking + // the vtable key deleted is necessary but NOT sufficient — we must also + // fall through to the keys scan to drop that real field, or + // hasOwnProperty keeps finding the method via `own_key_present` and + // verifyProperty fails "m descriptor should be configurable" (#5441, + // ~760 generated language/{statements,expressions}/class tests). if let Some(cid) = super::class_registry::class_id_for_decl_prototype_object(obj as usize) { if let Some(name) = super::has_own_helpers::str_from_string_header(key) { if name != "constructor" @@ -192,7 +201,9 @@ pub extern "C" fn js_object_delete_field( || super::native_module::class_has_own_method(cid, name)) { super::class_registry::class_mark_key_deleted(cid, name); - return 1; + // Accessors have no keys_array entry, so the scan below is a + // vacuous success for them; methods DO, so fall through to + // remove it. Either way, don't early-return. } } }