refactor: split the 16 files over the 2000-line lint gate - #7256
Conversation
`scripts/check_file_size.sh` is a required step of the `lint` job and 16
tracked Rust files had drifted past the 2,000-line cap. Split each along an
existing topical seam into sibling modules, re-exporting so every existing
path resolves unchanged.
Pure refactor: no renames, no signature changes, no behaviour change. The only
non-move edits are visibility widenings forced by a move (`fn` ->
`pub(super) fn`), the `mod`/`use` plumbing, and removal of imports left unused
in a trunk.
Two subtleties worth recording:
- The `native_module` oracle tests harvest every string literal in their own
source via `include_str!("<self>.rs")` and assert a coverage floor. Splitting
a file silently shrinks that literal universe, so those sites now
`concat!(include_str!(a), include_str!(b))` to keep it exactly as it was.
- `CALLABLE_EXPORT_ARITY_TABLE` and `CALLABLE_EXPORT_TABLE` are `binary_search`
targets, so element order is load-bearing. Each table moved as one intact
item; no table was partitioned.
Evidence: `cargo check --tests` clean on all six crates; crate-wide token
multisets conserved (5.39M tokens, zero non-plumbing losses); GC test files
reassemble byte-identically and `cargo test` reports the same 47 / 77 passing;
`#[no_mangle]` export lists unchanged in the FFI-heavy files. The
`addr_class_inventory.py` and `gc_store_site_inventory.py` finding sets are
byte-identical to origin/main, so no gate-script or allowlist edit was needed.
|
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 (61)
📝 WalkthroughWalkthroughThis PR splits 16 oversized Rust files into sibling modules across the compiler, runtime, network extension, stdlib, and CLI crates. Existing implementations, tests, and public paths are preserved through re-exports; the changelog documents the refactor and remaining unrelated lint failures. ChangesCompiler code generation
Network extension decomposition
HIR lowering decomposition
Array and garbage-collection runtime
Runtime API module decomposition
Compiler tooling tests
Changelog
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Splits the 16 tracked Rust files that had drifted past
scripts/check_file_size.sh's 2,000-line cap.bash scripts/check_file_size.shnow exits 0 with no new allowlist entries — every file was split, none was deferred.This does NOT turn
lintgreen on its own — please readlinthas been red onmainfor weeks, but not because of the file-size gate. The job dies two steps earlier and never reaches it. Verified against the last four nightly runs:Running every
lintstep locally atorigin/main(cdb3934) gives the true blocker set — four independent failures, of which this PR fixes one:cargo fmt --all -- --checkworkspace_architecture.py --checkci_public_baseline_check.pycheck_file_size.shbinding_pins.mjs --checkgc_store_site_inventory.pyaddr_class_inventory.pygap_snapshot.py --self-testparity_known_failures.py --self-testgc_gate_wiring_check.pyBecause the steps run in order and the job stops at the first failure, #3 must be fixed before this PR's effect is even observable in CI.
The splits
All 16 under the cap, most well under 1,900 to leave headroom.
perry-runtime/.../native_module/callable_exports.rsCALLABLE_EXPORT_ARITY_TABLE+ lookup + tests ->callable_export_arity_table.rsperry-runtime/.../native_module/constants.rsconstants_tables.rsperry-runtime/.../native_module/callable_export_check.rsCALLABLE_EXPORT_TABLEstatic ->callable_export_table.rsperry-codegen/src/expr/index_get.rsindex_get/perry-stdlib/src/worker_threads.rsworker_threads/perry/src/commands/compile/library_search.rs#[cfg(test)] mod->library_search/perry-runtime/src/gc/tests/layout_trace.rslayout_trace/perry-codegen/src/codegen/typed_abi.rstyped_abi/perry-runtime/src/process/env_misc.rsexec_env.rs+attributes.rsperry-hir/src/lower_decl/block.rsblock/perry-hir/src/lower/expr_call/native_module.rsnative_module/perry-runtime/src/gc/tests/runtime_roots.rsruntime_roots/perry-runtime/src/array/generic.rsgeneric_object.rsperry-stdlib/src/tls.rstls/perry-runtime/src/node_submodules/test.rstest_snapshot.rs+test_reporters.rsperry-ext-net/src/lib.rsTransport->transport.rs; tests ->tests.rsThe only non-move edits are visibility widenings forced by a move (
fn->pub(super) fn),mod/useplumbing, and removal of imports left unused in a trunk.Two subtleties that would have caused silent behaviour changes
1. Self-harvesting oracle tests. Three
native_modulefiles contain tests that harvest every string literal in their own source text viainclude_str!("<self>.rs")and cross-product them —callable_export_check.rseven assertschecked > 100_000. Splitting a file silently shrinks that literal universe, weakening the test while it still passes. Those sites now readso the harvested set is exactly what it was.
2. Order-sensitive tables.
CALLABLE_EXPORT_ARITY_TABLEandCALLABLE_EXPORT_TABLEarebinary_searchtargets — element order is load-bearing. Each moved as one intact item; no table was partitioned. TheBUFFER_*/SQLITE_*/ASSERT_*slices arefor-iterated (installation order is observable infor...in), so they were not moved at all.Evidence of no behaviour change
cargo check --testsclean (exit 0) on all six crates:perry,perry-runtime,perry-stdlib,perry-codegen,perry-hir,perry-ext-net.cargo check -p perry-runtime --testsemits the same 4 warnings atorigin/mainand on this branch — all in files this PR never touches..rsfiles per crate,origin/mainvs branch: 5,392,125 -> 5,394,082 tokens, zero non-plumbing losses. The only two flagged differences were backslash-line-continuation string literals whose continuation indentation changed; Rust strips that whitespace at compile time, and the resulting runtime values were verified byte-identical.gc::tests::layout_trace47 passed,gc::tests::runtime_roots77 passed, matching pre-split. (The naive grep count of 79 differs becausecallback_scanners.rshas two#[cfg(feature = "ohos-napi")]-gated tests the runner correctly excludes.)pub extern "C" fnsymbol lists diffed empty: perry-stdlib 68->68 (60#[no_mangle]attributes before and after), perry-ext-net 23->23. Moving a#[no_mangle]fn between modules cannot change its exported symbol, and none was made private.addr_class_inventory.pyandgc_store_site_inventory.pyproduce byte-identical finding sets toorigin/main(3 and 19 findings respectively, same files, same counts). No allowlist or ratchet-baseline edit was needed — this PR touchescrates/only.Byte-identical compiler output (differential codegen A/B)
Two
perrybinaries built fromorigin/mainand this branch (perry-devprofile, one target dir each), then run over a 40-file TypeScript corpus with--trace llvm, comparing the emitted LLVM IR.All 42 emitted IR modules are byte-identical. The two binaries themselves are not identical (43M vs 44M, differing md5), so this is not a vacuous comparison of one build against itself.
Per CLAUDE.md's hazard #4 — "a gate must assert its subject was live" — the corpus was checked to actually exercise the restructured code, counting real
callinstructions (not extern declarations) in the emitted IR:hir .../native_module/process_module.rsjs_nm_install_processx14,js_process_emit_before_exitx40,js_process_argvx3,js_process_cwdx3hir .../native_module/buffer_statics.rsjs_buffer_validate_sizex9,js_buffer_allocx3,js_uint8array_newx11hir .../native_module/reflect_statics.rsjs_reflect_get_metadatax16,js_reflect_define_metadatax16hir .../native_module/object_statics.rsjs_object_alloc_class_inline_keysx10,js_build_class_keys_arrayx25codegen/expr/index_get/js_dyn_index_getx8,js_typed_array_getx1,js_array_push_f64_temp_rootedx250codegen/codegen/typed_abi/js_typed_feedback_object_get_field_by_name_f64x52This matters most for
perry-hir/.../native_module.rs, where five dispatch blocks were extracted into helpers chained in the original order — the one change whose arm-ordering could not be proven by token conservation alone.The addr-class trap, and how it was avoided
The ratchet baseline is path-keyed with exact counts, and a new file gaining a ratcheted site has baseline 0 — a hard failure. Four of the 16 files carried baseline entries (
array/generic.rs4,node_submodules/test.rs2,process/env_misc.rs3+1,perry-stdlib/src/{tls,worker_threads}.rs1 each). The whole-file allowlist entries (array/generic.rs | *) are path-keyed too, so allowlisted code moving to a sibling loses its suppression.Both traps fired during the work and were resolved by re-picking seams so every ratcheted and allowlisted site stays in its original file —
proto_chain_contains_real_array,non_array_object_receiver, andplain_object_valuewere deliberately kept ingeneric.rsfor exactly this reason. Hence zero gate-file edits.What this PR does NOT verify
cargo testsuite, the parity/gap suites,conformance-smoke, and any Linux-only behaviour.test-files/corpus, and it compares compiler output — it therefore says nothing aboutperry-runtime/perry-stdlibruntime behaviour, which is only covered here bycargo check, the GC test runs, and the unchanged#[no_mangle]export lists.Follow-ups for the other three
lintblockersgc_store_site_inventory, 19 unaudited store sitesaddr_class_inventory, 2 ratchet regressions + a stale allowlist substringIn short: the public-baseline artifact's
source_fingerprintandharness_fingerprinthave both been stale for 40+ commits and can only be cleared by regenerating the artifact with the full Node/Bun/Perry suite on the pinned host; thegc_store_site_inventoryfailure is 19 store sites needing per-site audit-class judgement; theaddr_class_inventoryfailure is 2 ratchet regressions plus one allowlist entry whose line-substring no longer matches the code it was written for.Summary by CodeRabbit
node:testsnapshot assertions and reporter support.