From e42a3b81c4902c506dc4fd5d3bd56a7ee6906241 Mon Sep 17 00:00:00 2001 From: Ralph Date: Sat, 18 Jul 2026 00:29:20 -0700 Subject: [PATCH 1/2] fix(runtime): per-evaluation inherited static wins over last-wins registry props (#6552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A subclass of a class-EXPRESSION value evaluated more than once (`function make(a){return class{static ast=a}}`, then `class Number$ extends make(x) {}` / `class Widget$ extends make(y) {}`) records THIS evaluation's parent object as its static prototype (`class_prototype_object`, #1788). But #6443's static-DATA-field inheritance walk reads the parent's `CLASS_DYNAMIC_PROPS`, which are keyed by the class-expression TEMPLATE id — shared, last-wins across every evaluation — and it ran BEFORE the per-evaluation prototype-object walk. So every renamed effect-Schema class read via `import * as M` (`M.Number.ast`/`M.Widget.ast`) collapsed to the LAST `make(...)`'s static `ast` (the direct export's `DirectKeyword`), re-breaking effect Schema decode (test_gap_renamed_class_export_namespace, the #1758/#1812 guard). Interleave the two walks: at each level of the class-object proto chain, consult the class's pinned per-evaluation parent object BEFORE the parent's registry props. The pinned object is authoritative (it carries this evaluation's own edge); the registry read stays the fallback for a plain declaration parent (#6443: Auth.js `SignInError.kind`), which has no pinned object. Depth order is preserved, so a closer registry static still shadows a deeper per-evaluation one. test_gap_renamed_class_export_namespace is byte-identical to node again across renamed (global-colliding + not) and direct exports; #6443 static-field inheritance + deleted-key fallthrough, #6438 per-eval statics, effect factory static dispatch, and the rest of the class-expr/static suite (18/18) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../object/field_get_set/get_field_by_name.rs | 69 +++++++++++++------ 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index 44c5ba30d4..c1025dac2f 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -1033,33 +1033,58 @@ pub extern "C" fn js_object_get_field_by_name( // chain walk so the #2059 own-name synthesis below answers // with THIS class's registered name instead of an ancestor's. if name != "name" { - let mut cid = class_id; + // Walk the class-object proto chain for an inherited static + // DATA field. At EACH level the class's pinned + // per-evaluation parent OBJECT is consulted BEFORE the + // parent's registry props (`CLASS_DYNAMIC_PROPS`). + // + // #6552: a subclass of a class-EXPRESSION value evaluated + // more than once (`function make(a){return class{static + // ast=a}}`, then `class Number$ extends make(x) {}` / + // `class Widget$ extends make(y) {}`) records THIS + // evaluation's parent object as its static prototype + // (`class_prototype_object`, #1788), but the parent's + // `CLASS_DYNAMIC_PROPS` are keyed by the class-expression + // TEMPLATE id — shared, last-wins across every evaluation. + // Reading the registry entry for such a parent collapses + // sibling subclasses to the LAST `make(...)` (effect Schema: + // `Number$.ast`/`Widget$.ast` both read the last parent's + // `ast`). The pinned object carries this evaluation's own + // edge, so it is authoritative; the registry read remains + // the fallback for a plain declaration parent (#6443: + // Auth.js `SignInError.kind`), which has no pinned object. + let mut child = class_id; let mut depth = 0usize; while depth < 32 { - match get_parent_class_id(cid) { - Some(p) if p != 0 && p != cid => { - cid = p; - depth += 1; + let proto = + super::super::class_registry::class_prototype_object(child); + if !proto.is_null() { + let v = js_object_get_field_by_name(proto as *const _, key); + if !v.is_undefined() && !v.is_null() { + return v; } - _ => break, } - if super::super::class_registry::class_is_key_deleted(cid, name) { - // A key deleted on THIS ancestor is not provided by - // it, but a higher ancestor may still define it — - // `delete Mid.foo` must let `Sub.foo` inherit - // `Base.foo`, not resolve to undefined. Skip this - // level and keep walking up (safe: `cid`/`depth` - // advance at the top of every iteration). - continue; - } - let inherited = CLASS_DYNAMIC_PROPS.with(|m| { - m.borrow() - .get(&cid) - .and_then(|props| props.get(name).copied()) - }); - if let Some(v) = inherited { - return JSValue::from_bits(v.to_bits()); + let p = match get_parent_class_id(child) { + Some(p) if p != 0 && p != child => p, + _ => break, + }; + // A key deleted on THIS ancestor is not provided by it, + // but a higher ancestor may still define it — `delete + // Mid.foo` must let `Sub.foo` inherit `Base.foo`, not + // resolve to undefined. Skip the registry read for the + // deleted level and keep walking up. + if !super::super::class_registry::class_is_key_deleted(p, name) { + let inherited = CLASS_DYNAMIC_PROPS.with(|m| { + m.borrow() + .get(&p) + .and_then(|props| props.get(name).copied()) + }); + if let Some(v) = inherited { + return JSValue::from_bits(v.to_bits()); + } } + child = p; + depth += 1; } } if super::super::class_registry::lookup_static_method_in_chain(class_id, name) From bdd300aadba43715d1ec930dbb7007be5f882544 Mon Sep 17 00:00:00 2001 From: Ralph Date: Sat, 18 Jul 2026 00:42:31 -0700 Subject: [PATCH 2/2] fix(runtime): return a present `null` inherited static, don't fall through (#6552 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review follow-up. The interleaved per-evaluation proto-object read guarded on `!is_undefined() && !is_null()`, so a static explicitly set to `null` on one evaluation (`class NullAst$ extends make(null) {}`) was treated as "absent" and fell through to the parent's last-wins `CLASS_DYNAMIC_PROPS`, returning a SIBLING evaluation's `ast` instead of `null` (verified: perry read `{_tag:'OBJ'}` where node reads `null`). `null` is a present, authoritative value — only `undefined` means "absent here". Guard on `!is_undefined()` alone so a present `null` is returned and only a true miss continues to the registry / a higher ancestor. Extends the gap test + helper with a renamed null-static export (`M.NullAst.ast === null`), placed before the last non-null `make(...)` so `null` is a genuine discriminator; byte-identical to node. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/object/field_get_set/get_field_by_name.rs | 10 +++++++++- test-files/_helpers/renamed_class_export.ts | 7 ++++++- test-files/test_gap_renamed_class_export_namespace.ts | 5 +++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index c1025dac2f..3ffb076a07 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -1060,7 +1060,15 @@ pub extern "C" fn js_object_get_field_by_name( super::super::class_registry::class_prototype_object(child); if !proto.is_null() { let v = js_object_get_field_by_name(proto as *const _, key); - if !v.is_undefined() && !v.is_null() { + // Return a value present on the pinned object even + // when it is `null` — a static explicitly set to + // `null` on THIS evaluation is authoritative and + // must not fall through to the last-wins registry + // entry (a sibling evaluation's value). Only + // `undefined` means "absent here", which continues + // the walk to the parent's registry props / a higher + // ancestor. + if !v.is_undefined() { return v; } } diff --git a/test-files/_helpers/renamed_class_export.ts b/test-files/_helpers/renamed_class_export.ts index 4116949e8f..c5703223b5 100644 --- a/test-files/_helpers/renamed_class_export.ts +++ b/test-files/_helpers/renamed_class_export.ts @@ -12,8 +12,13 @@ export function make(a: any) { class Number$ extends make({ _tag: "NumberKeyword" }) {} class Widget$ extends make({ _tag: "WidgetKeyword" }) {} +// A per-evaluation static explicitly set to `null` — its own value must win +// over any sibling's (last-wins) registry `ast`. Declared before `DirectCls` +// so the last `make(...)` to register is a NON-null value, making `null` a real +// discriminator (regression guard for the #6552 fix's proto-vs-registry order). +class NullAst$ extends make(null) {} export class DirectCls extends make({ _tag: "DirectKeyword" }) {} // Renamed exports. `Number` deliberately collides with the JS global // `Number` — pre-fix, `M.Number` resolved to the global constructor. -export { Number$ as Number, Widget$ as Widget }; +export { Number$ as Number, Widget$ as Widget, NullAst$ as NullAst }; diff --git a/test-files/test_gap_renamed_class_export_namespace.ts b/test-files/test_gap_renamed_class_export_namespace.ts index 22ddfd34d8..46b31ce904 100644 --- a/test-files/test_gap_renamed_class_export_namespace.ts +++ b/test-files/test_gap_renamed_class_export_namespace.ts @@ -29,3 +29,8 @@ console.log("(3) M.DirectCls.ast._tag:", (M as any).DirectCls.ast?._tag); // (4) the global `Number` is still itself (not shadowed by the import). console.log("(4) global Number(42):", Number("42")); + +// (5) a renamed export whose per-evaluation static is explicitly `null` reads +// back `null` — it must not fall through to a sibling evaluation's (last-wins) +// `ast`. Guards the proto-object-before-registry ordering of the #6552 fix. +console.log("(5) M.NullAst.ast:", (M as any).NullAst.ast);