Skip to content

refactor(runtime): split field_set_by_name.rs (#7402) - #7444

Merged
proggeramlug merged 2 commits into
mainfrom
refactor/7402-split-field-set-by-name
Aug 5, 2026
Merged

refactor(runtime): split field_set_by_name.rs (#7402)#7444
proggeramlug merged 2 commits into
mainfrom
refactor/7402-split-field-set-by-name

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Closes #7402. The file sat at exactly 2000 of 2000 allowed lines, so the next line anyone added turned lint red — and lint is required, so every merge after that bypasses a required gate. That already happened once today.

file lines
field_set_by_name.rs 567
field_set_by_name/tail.rs 963
field_set_by_name/fast_paths.rs 284
field_set_by_name/write_helpers.rs 170
field_set_by_name/attr_variants.rs 87

Pure move, proven. Every relocated region diffs IDENTICAL against origin/maintail.rs is 937 lines verbatim. Only 5 lines differ across the whole 2000: four file-private unsafe fns promoted to pub(super), and one now-dead #[allow(unused_assignments)] whose assignments left with the macro.

The split point is the RuntimeHandleScope creation, chosen deliberately so the refresh_roots_after_alloc!() macro (#7341), all 16 of its call sites, and both GC_STORE_AUDIT(INIT) markers stay in one file with their order untouched. No rooted step crosses a module boundary — that ordering was the subject of two PRs today and must not move.

Verification

  • Lib tests: 1697 passed / 0 failed / 3 ignored — byte-identical to an origin/main baseline run in the same session
  • Gap set (93 tests): 88 pass / 3 node_fail / 2 run_fail on both arms, with per-test verdicts diffing identical, not merely matching counts
  • check_file_size.sh exit 0, proven able to fail via PERRY_FILE_SIZE_THRESHOLD=900
  • addr_class_inventory.py passes; sums preserved across the split (handle-floor 11 → 1+3+6+1, lone-valid-obj-ptr 3 → 2+1, 544 ratcheted sites before and after)
  • Warnings 4 → 4, identical set; cargo check proven able to fail via a deliberate error

Latent hazard this exposed

Both addr-class gate inputs are path-keyed, so any file split trips them even with zero new sites. addr_class_allowlist.txt matches via rel_path.startswith(prefix) with an entry ending in .rs, and addr_class_ratchet_baseline.txt is a per-(rule, file) count. Re-keyed here; it will bite the next person told to split a file.

Note for #7435

Its hunk targets the URL-setter arm, now at field_set_by_name/tail.rs:317 — the patch is written against field_set_by_name.rs:1240 and needs retargeting. tail.rs has 1032 lines of headroom for its 5.

Summary by CodeRabbit

  • Refactor

    • Reorganized named-property assignment internals into focused components without changing behavior.
    • Preserved support for object, array, closure, class, namespace, descriptor, and prototype-related property writes.
    • Maintained garbage-collection safety and property publication ordering during assignments.
  • Documentation

    • Added release documentation describing the internal restructuring.

Ralph Küpper added 2 commits August 5, 2026 13:59
…ine cap (#7402)

Pure mechanical relocation of `js_object_set_field_by_name` and its
neighbours into topical sub-modules. The file sat at exactly 2000 of 2000
allowed lines, so the next line added anywhere in it turned the required
`lint` context red.

No behaviour change: every moved statement is byte-identical, and the
split point for the tail is the `RuntimeHandleScope` creation, so the
`refresh_roots_after_alloc!()` macro (#7341) and all 16 of its call
sites — plus both `GC_STORE_AUDIT(INIT)` markers — stay together in one
file with their ordering untouched.
…e split (#7402)

Both addr-class gate inputs are PATH-keyed, so a pure file split trips them
even though no site is added or removed:

  - scripts/addr_class_allowlist.txt matches by `rel_path.startswith(prefix)`,
    and the prefix ended in `.rs`, so it stopped covering the code the moment
    it moved into field_set_by_name/. Widened to the directory-and-file prefix.
  - scripts/addr_class_ratchet_baseline.txt is a per-(rule, file) COUNT, so the
    grandfathered sites land on new paths with no baseline entry and read as a
    regression. Regenerated with --write-baseline (the workflow its own header
    prescribes for exactly this case).

Sums are preserved: handle-floor 11 -> 1+3+6+1, lone-valid-obj-ptr 3 -> 2+1,
and the gate still reports 544 ratcheted sites total, same as before the split.

Also adds the changelog.d fragment required by the changeset gate, and drops a
stale clause from the module doc (the diagnostic helper moved to
write_helpers.rs).
@proggeramlug
proggeramlug merged commit 7c09145 into main Aug 5, 2026
@proggeramlug
proggeramlug deleted the refactor/7402-split-field-set-by-name branch August 5, 2026 12:00
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2575a279-1403-48e1-92a5-9d2243c236fb

📥 Commits

Reviewing files that changed from the base of the PR and between 73c7118 and a33a895.

📒 Files selected for processing (8)
  • changelog.d/7402-split-field-set-by-name.md
  • crates/perry-runtime/src/object/field_set_by_name.rs
  • crates/perry-runtime/src/object/field_set_by_name/attr_variants.rs
  • crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs
  • crates/perry-runtime/src/object/field_set_by_name/tail.rs
  • crates/perry-runtime/src/object/field_set_by_name/write_helpers.rs
  • scripts/addr_class_allowlist.txt
  • scripts/addr_class_ratchet_baseline.txt

📝 Walkthrough

Walkthrough

The field-setting implementation is split into topical Rust modules. The entry point delegates rooted assignment to the tail module, while fast paths, attribute setters, write helpers, and path-based metadata are updated.

Changes

Field setter split

Layer / File(s) Summary
Module wiring and metadata
crates/perry-runtime/src/object/field_set_by_name.rs, changelog.d/..., scripts/*
The entry point declares and re-exports the new modules, delegates the assignment tail, and updates allowlist and ratchet paths.
Attribute and write helper contracts
crates/perry-runtime/src/object/field_set_by_name/attr_variants.rs, .../write_helpers.rs
The split adds attribute-specific setters, key utilities, closure handling, class-static mirroring, and native namespace overrides.
Validated fast paths
crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs
The split adds overwrite and transition-cache paths with receiver, key, shape, storage, and GC validation.
Rooted assignment tail
crates/perry-runtime/src/object/field_set_by_name/tail.rs
The relocated path handles special objects, descriptors, prototype setters, shape transitions, storage, root refreshes, publication ordering, and mirroring.

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

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/7402-split-field-set-by-name

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.

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.

field_set_by_name.rs sits at exactly 2000/2000 lines — split it before the next change

1 participant