fix(runtime): resolve a shadowed inherited field to its most-derived slot

[proggeramlug]

Summary

A field a subclass overrides used to read as the base class's value from
inside an inherited accessor (Object.defineProperty getter), even though a
direct (non-accessor) read of the same field on the same instance already saw
the subclass's override correctly. The bug is not Symbol- or accessor-specific:
any dynamic (name-not-known-at-compile-time) read of a shadowed field — a
computed obj[key], Reflect.get, hasOwnProperty — hit the same defect.

Root cause

crates/perry-codegen/src/codegen/mod.rs (~line 1290) builds a class's packed
inline-slot layout by walking the extends chain root → leaf and appending
every keyable field name it finds, without deduplicating a name a subclass
re-declares:

for (_parent_name, parent_fields) in parent_chain.iter().rev() {
    for f in parent_fields { packed_keys.push_str(&f.name); ... }
}
for f in &c.fields { packed_keys.push_str(&f.name); ... }

So class Sub extends Base { tag = "sub-tag" } (with Base { tag = "base-tag" })
gives Sub two inline slots named "tag" — the ancestor's (index 0) and
the override's (index 1) — instead of one shared slot. Every writer in the
constructor chain resolves a field's index via
crates/perry-codegen/src/type_analysis_class_fields.rs::class_field_global_index,
which already searches "leaf → root" (most-derived wins, TS shadowing) —
so only index 1 is ever written; index 0 sits uninitialized. A
compile-time-typed obj.field read/write (the direct-read path) uses that
same most-derived index and was always correct.

Every dynamic by-name field lookup in perry-runtime instead scanned
root-to-leaf and returned the first match — the ancestor's uninitialized
slot — disagreeing with the compile-time path:

1. crates/perry-runtime/src/object/keys_lookup.rs
keys_find_slot_by_bytes / keys_find_slot_by_key_ptr — the shared
   by-name lookup behind own_data_field_by_name, Reflect.*,
   hasOwnProperty, delete, and a computed obj[key] read/write.
2. crates/perry-runtime/src/object/shapes.rs::shape_slot_lookup_verdict —
   the indexed lookup for objects with ≥32 keyable fields
   (KEYS_INDEX_THRESHOLD).
3. crates/perry-runtime/src/object/field_get_set/ic_miss.rs::get_field_ic_miss_impl —
   the fast-path inline scan the emitted per-callsite inline cache falls into
   on a miss for a static string-keyed this.field read. This is the one
   the issue's repro (an inherited getter's this.tag) actually hits.

An inherited accessor's receiver was already threaded correctly (confirmed via
runtime instrumentation before writing the fix) — the getter really does run
with this === sub; the defect is purely in how this.tag resolves once
inside it.

Fix

Three call sites, no storage-layout change:

• keys_lookup.rs: both linear scans now iterate (0..n).rev() — for the
  common (no-shadowing) case this is a no-op cost-wise (one candidate either
  way); for a duplicate name it now returns the most-derived (last) slot,
  matching class_field_global_index.
• shapes.rs: shape_slot_lookup_verdict scans every hash-bucket candidate
  and keeps the highest slot index among matches, instead of returning on
  the probe order's first hit — robust to open-addressing table growth
  reshuffling insertion order.
• ic_miss.rs: same (0..key_count).rev() in the inline fast-path scan.
  This file was already sitting at exactly the 2000-line file-size gate;
  the 1-line fix pushed it 1 line over, so scripts/check_file_size.sh's
  ALLOWLIST gets a new entry with rationale (see below) rather than a
  forced structural split.

The write side (obj[key] = v on a small object) already routes through
keys_find_slot_by_key_ptr for the common < KEYS_INDEX_THRESHOLD (32
keyable fields) case, so it is fixed transitively — no separate write-path
defect was found.

Gate: scripts/check_file_size.sh

ic_miss.rs was already exactly 2000 lines (the gate's own threshold) before
this change. The 1-line fix (plus a 1-line comment) puts it 1 line over. Per
this repo's own precedent for the same situation (see the existing
body_stmt.rs / then.rs allowlist entries), I added ic_miss.rs to the
ALLOWLIST with a short rationale rather than force a structural split of an
unrelated ~2000-line IC-miss ladder into this fix's diff. check_file_size.sh
passes clean after the addition.

Tests

• test-files/test_gap_10595_inherited_accessor_field_shape.ts — covers a
  string-key getter, a Symbol-key getter, a getter+setter pair, a two-level
  subclass (SubSub), a field overridden with a different runtime type
  (string → number), a computed obj[key] read, and direct-read controls
  (which already passed pre-fix, proving the accessor/computed-key path is
  the only thing regressing).
  • Fails on baseline (9df5075fbe, built fresh, unmodified): 6 of 15
    printed lines diverge from Node — e.g. sub.tagViaGetter base-tag
    instead of sub-tag, subsub[tagSym] sub-tag instead of subsub-tag.
  • Passes byte-identical to Node on this branch.
• crates/perry-runtime/src/object/keys_lookup.rs — 3 new unit tests
  (tests_10595 module): duplicate-name resolves to the last occurrence,
  single-occurrence names are unaffected by scan direction, an absent key
  still misses.
• crates/perry-runtime/src/object/shapes_tests.rs — 1 new unit test for the
  indexed (≥32-key) lookup path, same duplicate-name assertion.

Validation

• cargo test --release -p perry-runtime --lib (RUST_TEST_THREADS=1):
  3996 passed, 2 failed — both failures reproduce identically on a fresh,
  unmodified baseline build at the same commit (9df5075fbe):
  gc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check
  and gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds
  — both are debug_assert!-gated guards that a --release build compiles
  out (see CLAUDE.md's "Verifying a runtime change" / --profile gcaudit
  note); pre-existing, unrelated to this change. The 4 new tests all pass.

• Lint: 76/77 script gates passed (compile tier not run, per this
  campaign's host notes). The one failure, "Public benchmark evidence
  freshness" (benchmarks/ci_public_baseline_check.py), is known-red on
  main for everyone. scripts/check_file_size.sh passes (see above).
  cargo fmt --all made no changes. check_test_registration.py: clean
  (334 files checked against 4 registries, no new entries needed).

• Gap suite (--filter field, 55 tests, PERRY_SKIP_BUILD=1 PERRY_NO_AUTO_OPTIMIZE=1): baseline 50 pass / 2 fail / 3 compile-fail →
  fixed 51 pass / 1 fail / 3 compile-fail. The fixed-run's one remaining
  failure (test_issue_341_typed_field_native, a Node
  package_json_reader-related module-resolution mismatch) and all 3
  compile failures (test_issue_10155_textfield_singleline,
  test_issue_640_navstack_textfield, test_issue_763_reactive_textfield —
  native-UI TextField tests needing an AppKit/iOS SDK this Linux host
  doesn't have) reproduce identically on the untouched baseline. No new
  failures.

• Performance (perf stat -e instructions,task-clock, --profile perry-dev — this host's disk was at 96–97% free during validation, so I
  avoided a second --release runtime build rather than risk exhausting a
  shared box; the instruction-count comparison is still baseline-vs-fix on
  identical build settings), 3 runs each, baseline vs fixed:

  probe	baseline instructions (avg)	fixed instructions (avg)	delta
  static this.a in a 20M-iteration loop (IC-primed after warmup)	365,104,827	365,148,450	+0.012%
  computed w[key] (key="a", the first-declared field of an 8-field class — the deliberate worst case for a back-to-front scan) in a 5M-iteration loop	2,819,584,887	2,819,490,109	−0.003%

  Both are within the ±1% noise floor; compiled .ll/binary sizes are
  byte-identical between baseline and fixed builds for both probes (codegen
  is untouched — only the runtime .a differs), confirming the comparison
  isolates the runtime change. The static-key case is unaffected because the
  emitted per-callsite inline cache amortizes the one-time scan after the
  first hit; the dynamic-key worst case shows no measurable cost because the
  handful of extra short-string comparisons is dwarfed by the call's existing
  per-iteration overhead (shape/hash resolution, typed-feedback bookkeeping).

What I did NOT verify

• The ≥32-keyable-field indexed shape-lookup fix (shapes.rs) is covered
  by a unit test but not by an end-to-end gap test (no gap fixture in this
  PR has a class that large).
• Did not run the full (non-filtered) gap suite locally, per this campaign's
  guidance — relying on CI's gap-suite shards plus the targeted --filter field sweep above (55 tests spanning fields/inheritance/prototypes).
• Did not re-run under --profile gcaudit (the two pre-existing
  debug_assert!-gated test failures noted above were only confirmed
  pre-existing via an identical --release A/B, not resolved).

Related, out of scope

Confirmed, not fixed here: Object.keys(sub) on the repro above prints
["tag","tag"] (Node: ["tag"]) — the underlying storage still has two
physical inline slots for the shadowed name even though every lookup now
resolves to the correct one, and Object.keys/for...in enumerate the raw
keys array rather than deduplicating by name. JSON.stringify(sub) is
unaffected ({"tag":"sub-tag"}, correct) — it must already dedupe or resolve
differently. Worth a follow-up issue; left out of this PR to keep the diff to
the read-path defect #10595 actually reports.

Fixes #10595

Summary by CodeRabbit

• Bug Fixes

  • Fixed dynamic property lookups so subclass fields correctly take precedence when they share a name with an inherited field.
  • Updated computed, accessor-based, symbol-based, and other runtime reads to resolve the most-derived field consistently.
  • Preserved existing behavior for fields with unique names.

• Tests

  • Added coverage for inherited field overrides and duplicate-name lookup scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 57464e9f-5b1e-4342-b4bd-3bff770d4bc5

📥 Commits

Reviewing files that changed from the base of the PR and between 9df5075 and d55450f.

📒 Files selected for processing (7)

• changelog.d/10607-shadowed-field-most-derived-slot.md
• crates/perry-runtime/src/object/field_get_set/ic_miss.rs
• crates/perry-runtime/src/object/keys_lookup.rs
• crates/perry-runtime/src/object/shapes.rs
• crates/perry-runtime/src/object/shapes_tests.rs
• scripts/check_file_size.sh
• test-files/test_gap_10595_inherited_accessor_field_shape.ts

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

---

📝 Walkthrough

Walkthrough

Changes

The runtime now resolves duplicate field names to the most-derived slot across indexed shape lookup, key-array scans, and IC-miss lookup. Tests cover direct, computed, symbol-keyed, and inherited accessor reads.

Derived field lookup

Layer / File(s)	Summary
Indexed shape selection crates/perry-runtime/src/object/shapes.rs, crates/perry-runtime/src/object/shapes_tests.rs	Indexed lookup checks all matching candidates and returns the highest slot index. Tests cover duplicate names and lookup verdicts.
Runtime key resolution crates/perry-runtime/src/object/field_get_set/ic_miss.rs, crates/perry-runtime/src/object/keys_lookup.rs, scripts/check_file_size.sh	Dynamic key scans use reverse order so shadowed fields resolve to the most-derived slot. Lookup tests cover duplicate, single, and absent keys. The size-check allowlist records the affected file.
Inherited accessor regression test-files/test_gap_10595_inherited_accessor_field_shape.ts, changelog.d/10607-shadowed-field-most-derived-slot.md	The regression test exercises direct, computed, symbol-keyed, and inherited accessor paths. The changelog records the fix and the remaining duplicate enumeration behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d5545

The derived-field lookup fix is covered across its stated runtime paths, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary runtime fix: resolving shadowed inherited fields to the most-derived slot.
Description check	✅ Passed	The description is detailed and covers the summary, root cause, changes, related issue, tests, validation results, limitations, and out-of-scope behavior. It does not include the template's explicit c…
Linked Issues check	✅ Passed	Issue #10595 requires shadowed inherited fields to resolve to the receiver's most-derived slot during inherited accessors, for string and Symbol keys. The PR updates linear lookup, indexed shape looku…
Out of Scope Changes check	✅ Passed	The changes stay within issue #10595. The lookup changes and regression tests implement or validate the required field-slot behavior. The changelog and file-size allowlist entry support the implementa…
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (1 skipped: 1 …

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[proggeramlug]

Landed via merge train #10652 (v0.5.1596). All source commits preserve authorship; merged main matches the validated train exactly.