refactor: split gc/layout.rs and codegen/artifacts.rs under the 2000-line cap (unblocks lint on main) - #8212
Merged
Merged
Conversation
…line cap #8204 pushed both files over scripts/check_file_size.sh's hard cap (layout.rs 1975 -> 2110, artifacts.rs 2000 -> 2005), turning the required lint context red on main for every PR. Pure code moves, no logic change: - gc/layout.rs: the typed-shape layout installation protocol (TypedShapeProof, mask_words, init_typed_shape_layout, install_typed_shape_layout_slow, typed_shape_layout_entry, js_gc_init_typed_shape_layout, js_gc_declare_typed_shape_layout) moves to gc/layout/typed_shape.rs, next to the existing layout/slot_mask.rs. 2110 -> 1778 lines. The two extern "C" entry points keep their crate::gc:: paths via an explicit named re-export. - codegen/artifacts.rs: synthesized_ctor_param_count moves to a new sibling codegen/ctor_arity.rs. 2005 -> 1930 lines. Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change extracts constructor arity calculation and typed-shape layout installation into dedicated modules. Existing codegen integration and typed-shape C ABI paths remain available through module wiring and re-exports. ChangesConstructor and typed-shape layout modularization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
added 2 commits
August 16, 2026 16:45
…ray site Pure path rename in the exact callsite multiset: the one keys_array access inside the moved typed-shape install block now lives in gc/layout/typed_shape.rs (raw_member_files 65 -> 66, total sites unchanged). Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
proggeramlug
pushed a commit
that referenced
this pull request
Aug 16, 2026
Rebase moved the base to current main, so both arms were rebuilt there and the whole measurement retaken. Counters are bit-identical (releases == allocs, residue constant at 65,915) and peak RSS reproduces within 0.3 MB, so none of #8204/#8196/#8211/#8212/#8162 moves this residue. Also records, rather than rounds away, the fixed +80 KB per-process startup cost the change adds: it is page-granular first touch, not code size (binary +80 B, __TEXT unchanged) and not the pool data (144 B of empty Vec headers). Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
proggeramlug
pushed a commit
that referenced
this pull request
Aug 16, 2026
Rebase moved the base to current main, so both arms were rebuilt there and the whole measurement retaken. Counters are bit-identical (releases == allocs, residue constant at 65,915) and peak RSS reproduces within 0.3 MB, so none of #8204/#8196/#8211/#8212/#8162 moves this residue. Also records, rather than rounds away, the fixed +80 KB per-process startup cost the change adds: it is page-granular first touch, not code size (binary +80 B, __TEXT unchanged) and not the pool data (144 B of empty Vec headers). Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
This was referenced Aug 16, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Aug 16, 2026
Rebase moved the base to current main, so both arms were rebuilt there and the whole measurement retaken. Counters are bit-identical (releases == allocs, residue constant at 65,915) and peak RSS reproduces within 0.3 MB, so none of #8204/#8196/#8211/#8212/#8162 moves this residue. Also records, rather than rounds away, the fixed +80 KB per-process startup cost the change adds: it is page-granular first touch, not code size (binary +80 B, __TEXT unchanged) and not the pool data (144 B of empty Vec headers). Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lintis red onmain; this unblocks it./scripts/check_file_size.shexits 1 against the hard 2000-line cap:Bisected to #8204 (
bf8fd868e, the 56 → 48 B header shrink):gc/layout.rswent 1975 → 2110, andcodegen/artifacts.rswas sitting at exactly 2000 — right on the cap, so any addition at all broke it.check_file_size.shruns inlint, a required status context, so every open PR is currently blocked behind a failure that has nothing to do with it. That is the "required and red for a pre-existing reason" state — it trains everyone to bypass, and the next genuine failure arrives indistinguishable from the standing one.The split
codegen/artifacts.rscodegen/ctor_arity.rs(new)gc/layout.rsgc/layout/typed_shape.rs(new)Both land with real headroom rather than scraping under at 1999 —
artifacts.rsbeing at exactly 2000 is what made #8204 break it, and leaving it there would just re-arm the trap.This is relocation-only, and here is the evidence
A file-size fix is the easiest place for an accidental semantic change to hide behind a large mechanical diff, so the claim is checked rather than asserted. Comparing the multiset of non-blank source lines before against (remainder + extracted) after:
layout.rs→typed_shape.rsartifacts.rs→ctor_arity.rsAll six "vanished" lines reappear in adjusted form, and they are exactly the two categories a relocation produces:
super::shape_install::{words_contain_slot, words_intersect, hit, record}becameshape_install::…, because the extracted module sits insidegc/layout/and no longer needs thesuper::hop.fn synthesized_ctor_param_count(becamepub(super) fn, plususe super::ctor_arity::synthesized_ctor_param_count;at the call site.The 23 + 8 new lines are module headers,
usestatements and doc comments. No logic edits, no renames visible outside the modules.Seam choice
Both extractions are cohesive rather than "the last N lines":
typed_shape.rsis the typed-shape layout family,ctor_arity.rsis the synthesized-constructor arity helper and its single caller.Deliberately not touched, both near the cap and owned by in-flight work:
gc/cycle.rs(1991) andgc/barrier/mod.rs(1995).Note for anyone splitting in this area next: adding a matching
pub use <new_mod>::*;is an unused-import warning — hence a-D warningsfailure — when the extracted items are#[no_mangle]and reached by symbol rather than by path. The module needsmod <new_mod>;only, plus fixing any internalcrate::…::<moved_fn>call sites.No version bump.
Summary by CodeRabbit