Skip to content

fix: preserve factory class identity and dynamic heritage - #9753

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9502-fresh-class-heritage
Closed

fix: preserve factory class identity and dynamic heritage#9753
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9502-fresh-class-heritage

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

A function-body declaration such as class D extends (P ?? Object) {} now creates a distinct constructor on every evaluation. Factory chains retain their chosen superclass: mk(null) !== mk(A) and Object.getPrototypeOf(B) === A.

Changes

  • Materialize declarations with dynamic heritage through ClassExprFresh, and replace the stale lowering comment.
  • Use each evaluation's prototype chain for reflection, instanceof, and inherited property/method lookup. Mark these links separately from ordinary runtime wiring, without flushing unrelated caches on every construction.
  • Preserve static forward-construction compatibility and inherited Promise statics; recognize heap class values as constructors.
  • Add two parity regressions, extend the prototype metadata test, and add a changelog fragment. No version bump.

Related issue

Closes #9502.

Test plan

Developed and tested via SSH on root@perrymaster.skelpo.net, based on 12efed122.

  • Release compiler and matching runtime/stdlib/network archives built successfully.
  • cargo test --lib -p perry-hir: 382 passed, 1 ignored.
  • cargo test --lib -p perry-codegen: 1,410 passed, 1 ignored.
  • cargo test --lib -p perry-runtime (single-threaded): 3,123 passed, 4 ignored.
  • Both new fixtures fail on the baseline and pass through run_parity_tests.sh --filter test_gap_9502_ after the fix.
  • All 97 runnable heritage fixtures match Node 26.5.1 byte-for-byte. The 98th, test_gap_derived_param_props, is rejected by the Node oracle on both versions.
  • Language documentation examples: 5/5 passed.
  • 20,000 three-level factory chains pass normally and with PERRY_GC_SCHEDULE_SEED=999 PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_PROTECT_FROMSPACE=1: 10,006 copying collections, 108,818 moved objects.
  • Script/format lint gates: 62 passed; workspace compile tier and 2 CI-only commands skipped.

One repeated runtime run hit async_hooks::test_support::tests::native_async_resource_accepts_string_and_symbol_expandos; its isolated rerun and the subsequent full runtime run passed. The full 702-fixture gap suite was not completed locally.

Checklist

  • No workspace version bump or edits to CLAUDE.md / CHANGELOG.md.
  • Regression coverage and changelog fragment included.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed dynamically inherited classes created by factories so each evaluation produces a distinct class and prototype chain.
    • Corrected instanceof, prototype reflection, inherited method lookup, and static state for independently evaluated classes.
    • Preserved Promise static methods when accessed through dynamically created derived classes.
  • Tests

    • Added coverage for dynamic heritage, chained factories, distinct prototypes, static initialization, and instanceof behavior.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 119538c4-6057-4bd8-b73f-ac26671ce05f

📥 Commits

Reviewing files that changed from the base of the PR and between 12efed1 and 6aa570a.

📒 Files selected for processing (15)
  • changelog.d/9753-factory-class-heritage-identity.md
  • crates/perry-hir/src/lower_decl/body_stmt.rs
  • crates/perry-runtime/src/object/class_registry/construct.rs
  • crates/perry-runtime/src/object/class_registry/construct/class_object.rs
  • crates/perry-runtime/src/object/field_get_set/class_object_props.rs
  • crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
  • crates/perry-runtime/src/object/field_get_set/prototype_override.rs
  • crates/perry-runtime/src/object/instanceof.rs
  • crates/perry-runtime/src/object/mod.rs
  • crates/perry-runtime/src/object/native_call_method.rs
  • crates/perry-runtime/src/object/native_call_method/primitive_methods.rs
  • crates/perry-runtime/src/object/object_ops/prototype.rs
  • crates/perry-runtime/src/object/prototype_chain.rs
  • test-files/test_gap_9502_factory_decl_heritage_forms.ts
  • test-files/test_gap_9502_factory_decl_heritage_identity.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Function-nested classes with dynamic heritage now create distinct class objects per evaluation. Runtime prototype links, constructor checks, instanceof, inherited methods, Promise statics, and static state use each class evaluation's parent and prototype chain.

Changes

Dynamic heritage class evaluation

Layer / File(s) Summary
Fresh class lowering
crates/perry-hir/src/lower_decl/body_stmt.rs
Function-nested classes with dynamic extends expressions now use fresh per-evaluation class bindings.
Evaluation-specific prototype wiring
crates/perry-runtime/src/object/mod.rs, crates/perry-runtime/src/object/prototype_chain.rs, crates/perry-runtime/src/object/class_registry/construct.rs, crates/perry-runtime/src/object/class_registry/construct/class_object.rs, crates/perry-runtime/src/object/field_get_set/class_object_props.rs
Runtime metadata identifies class-evaluation prototypes. Instance and class-object links preserve each evaluated parent and prototype. Heap class objects are recognized as constructors.
Prototype lookup and dispatch
crates/perry-runtime/src/object/object_ops/prototype.rs, crates/perry-runtime/src/object/instanceof.rs, crates/perry-runtime/src/object/field_get_set/prototype_override.rs, crates/perry-runtime/src/object/native_call_method.rs, crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs, crates/perry-runtime/src/object/native_call_method/primitive_methods.rs
Prototype reflection, instanceof, inherited method lookup, and Promise static dispatch follow evaluation-specific class chains.
Regression coverage and changelog
test-files/test_gap_9502_factory_decl_heritage_forms.ts, test-files/test_gap_9502_factory_decl_heritage_identity.ts, changelog.d/9753-factory-class-heritage-identity.md
Tests cover dynamic heritage forms, class identity, prototype chains, instanceof, inherited methods, and per-evaluation static state. The changelog records the fix.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 6aa57

Function-body classes with dynamic heritage now retain distinct per-evaluation constructors and superclass chains, with coverage for identity, reflection, inheritance, static state, and Promise statics. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 14 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary fix: preserving class identity and dynamic heritage in factory-created classes.
Description check ✅ Passed The description includes the required summary, concrete changes, related issue, detailed test plan, and checklist. Optional screenshots are not needed because this is not a visual change.
Linked Issues check ✅ Passed The implementation satisfies issue [#9502] by creating distinct per-evaluation classes for dynamic heritage, preserving selected superclass chains, updating prototype and instanceof behavior, and addi…
Out of Scope Changes check ✅ Passed The changes remain within the linked issue and stated objectives. Runtime updates for prototype identity, constructors, Promise statics, reflection, and inheritance support the dynamic-heritage fix, a…
Full details: Docstring Coverage

Explanation

Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 14 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Hot-path note from the cc performance campaign (see secret-tests/cc-perf-campaign/ARCHITECTURE.md): the new arm in js_object_get_field_by_name evaluates promise_parent_in_chain(class_id) before promise_static_function_spec(name). The first walks up to 32 parent classes, each level doing js_get_dynamic_parent_value + identify_global_builtin_constructor; the second is a name test against a fixed static list. Swapping the operands makes the common case (a class-object property read whose name is not a Promise static) cost one name comparison instead of a chain walk, with identical semantics since && is short-circuiting:

if promise_static_function_spec(name).is_some() && promise_parent_in_chain(class_id)

Property reads on class objects are on the measured hot path for the compiled claude-code TUI (js_object_get_field_by_name shows up as a top leaf in every sample), which is why this is worth the swap even though the arm is a miss path.

https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9798 (rebase-merged, so your commits keep their authorship). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A non-capturing function-body class declaration shares one class across evaluations, so a factory's heritage chain collapses (mk(null) === mk(A))

1 participant