fix(hir,runtime): per-evaluation class bindings must carry the capture BOX, and expose .name#6500
Conversation
…e BOX, and expose .name (PerryTS#6497) be2e23f (PerryTS#6470) routes capturing function-body class declarations through ClassExprFresh so each factory evaluation carries its own captured environment. Two interactions regressed (bisected in PerryTS#6497; gap tests test_gap_anon_shape_boxed_capture and test_gap_5952_mixin_factory_binding fail on main, which reds conformance shards 5/7 for every PR): 1. The PerryTS#5951 shared-mutable-capture pass rewrites every value read of a boxed capture into `cell[0]`. It exempts `RegisterClassCaptures` args (capture sites must snapshot the WHOLE box handle) but not the new `ClassExprFresh.captured_args` — so the fresh class object snapshotted the cell's current VALUE. Methods of a class that captures AND mutates a local then indexed into a number: reads came back `undefined`, writes were lost (`shared cell: undefined,10` vs node's `12,12`). Exempt captured_args exactly like RegisterClassCaptures; statics' initializer values are ordinary reads and still rewrite. 2. `.name` on a heap class-expression value read `undefined`: the class object's own fields hold only per-evaluation statics, and nothing routed the read to the template's registry name. Add a `name` arm next to the PerryTS#4949 `prototype` arm in the OBJECT_TYPE_CLASS tail — own static `name` members still win, and a deleted key still reads undefined. This also fixes `Tagged.name` for PerryTS#6470's own factory shape. Both gap tests now match node byte-for-byte, and PerryTS#6470's original repro (effect's makeException — per-evaluation `_tag`s) still passes, as does a boxed-counter variant through the same factory shape. perry-runtime suite 1321/0; perry-hir suites unchanged (the pre-existing c262_parity failure `logical_property_assignment_short_circuits_the_store_4586` fails identically on clean origin/main). Claude-Session: https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
📝 WalkthroughWalkthroughFresh class expression lowering now preserves indexed capture handles while rewriting statics and other captures. Runtime heap class objects use shared helpers to resolve ChangesFresh class behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs (1)
1331-1331: 📐 Maintainability & Code Quality | 🔵 TrivialRebuild static wrapper crates.
As per coding guidelines, when changing runtime or stdlib code, please ensure you rebuild the corresponding static wrapper crates (
perry-runtime-staticandperry-stdlib-static) because the runtime and stdlib crates themselves emit only rlibs and otherwise may leave stale archives linked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs` at line 1331, Rebuild the perry-runtime-static and perry-stdlib-static wrapper crates after the runtime change near the ClassExprFresh handling, ensuring their generated static archives reflect the updated runtime and stdlib code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs`:
- Around line 1344-1350: The `.name` deletion check in the class-name fallback
incorrectly uses shared `class_id` state, causing one instance’s deletion to
affect others. Replace `class_is_key_deleted` and its corresponding
`class_mark_key_deleted` usage for `.name` with an instance-local tombstone or
side table, and update the fallback near `class_name_for_id` to consult that
per-instance state.
---
Nitpick comments:
In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs`:
- Line 1331: Rebuild the perry-runtime-static and perry-stdlib-static wrapper
crates after the runtime change near the ClassExprFresh handling, ensuring their
generated static archives reflect the updated runtime and stdlib code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d2762008-4103-4248-81d7-48253a02dbd6
📒 Files selected for processing (2)
crates/perry-hir/src/lower/shared_mutable_capture.rscrates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs
…me_tail The `.name` arm pushed get_field_by_name_tail.rs past the 2000-line lint cap. Move it and the adjacent PerryTS#4949 `.prototype` arm into a `class_object_props` sibling module; behavior unchanged (gap tests re-verified byte-identical to node, perry-runtime 1321/0). Claude-Session: https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/perry-runtime/src/object/field_get_set/class_object_props.rs (1)
1-4: 📐 Maintainability & Code Quality | 🔵 TrivialRebuild static wrapper crates.
As per coding guidelines, when changing runtime or stdlib code, please rebuild the corresponding static wrapper crates (
perry-runtime-staticandperry-stdlib-static) because the runtime and stdlib crates themselves emit only rlibs and otherwise may leave stale archives linked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/field_get_set/class_object_props.rs` around lines 1 - 4, Rebuild the generated static wrapper crates perry-runtime-static and perry-stdlib-static after this runtime change, ensuring their archives are refreshed and include the updated runtime behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/perry-runtime/src/object/field_get_set/class_object_props.rs`:
- Around line 1-4: Rebuild the generated static wrapper crates
perry-runtime-static and perry-stdlib-static after this runtime change, ensuring
their archives are refreshed and include the updated runtime behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b7a5e00-2d11-4da5-aaf5-61f729946a2c
📒 Files selected for processing (3)
crates/perry-runtime/src/object/field_get_set.rscrates/perry-runtime/src/object/field_get_set/class_object_props.rscrates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs
Fixes #6497 — the main-tip regression from be2e23f (#6470) that currently reds conformance-smoke shards 5/7 on every PR.
What broke
#6470 routes capturing function-body class declarations through
ClassExprFresh(per-evaluation captures). Two interactions:Boxed (mutated) captures lost their cell. The class field-init closure mutating a captured outer local gets a split cell (class captures are value-based; need boxed-capture mode) #5951 shared-mutable-capture pass rewrites every value read of a boxed capture into
cell[0], and exemptsRegisterClassCapturesargs — capture sites must snapshot the whole box handle. The newClassExprFresh.captured_argshad no exemption, so the fresh class object snapshotted the cell's current value. A class that captures AND mutates a local then readundefinedand dropped writes:.namereadundefinedon the fresh class value (makeTagged("S").name): nothing routed the read to the template's registry name. Added anamearm next to the compat(nestjs): .prototype of a capturing class expression is undefined — tslib __decorate crashes @nestjs/common module init #4949prototypearm in theOBJECT_TYPE_CLASSproperty tail; explicitstatic namemembers still win.Fix
shared_mutable_capture.rs: exemptClassExprFresh.captured_argsfrom thecell[0]rewrite exactly likeRegisterClassCaptures; statics' initializer values are ordinary reads and still rewrite.get_field_by_name_tail.rs:.nameon heap class objects resolves viaclass_name_for_id, after own-field lookup, honoring key deletion.Validation
test_gap_anon_shape_boxed_captureandtest_gap_5952_mixin_factory_binding: byte-identical to node (both fail on cleanorigin/main).makeException, per-evaluation_tags) still passes, plus a boxed-counter variant through the same factory shape.perry-runtime1321/0 single-threaded;perry-hirunchanged (the c262_parity failurelogical_property_assignment_short_circuits_the_store_4586is pre-existing — fails identically on clean origin/main).https://claude.ai/code/session_01M44HoYS3CAYZyagm3ZzYDG
Summary by CodeRabbit
New Features
.nameproperty..prototypeand.namethrough consistent runtime logic.Bug Fixes
.namewhen no own data field is present (returnsundefinedwhen marked deleted).