Analyze and fix across a configured list of compilation targets - #150
Open
Jarred-Sumner wants to merge 1 commit into
Open
Jarred-Sumner wants to merge 1 commit into
Jarred-Sumner wants to merge 1 commit into
Conversation
6 of 7 tasks
Jarred-Sumner
added a commit
to oven-sh/bun
that referenced
this pull request
Jul 29, 2026
…he code it proves dead (#36184) ## Summary Runs a multi-target visibility analysis over the Rust workspace with a fork of Astral's `hawk` (multi-target support upstreamed as astral-sh/hawk#150) so that `pub` items with no cross-crate use on **any** shipped platform are narrowed to `pub(crate)`/private. Once narrowed, rustc's denied `dead_code`/`unreachable_pub` lints can prove code dead across crate boundaries, and that code is deleted. Three commits: 1. **Make workspace test and doctest targets compile** — `cargo check --workspace --all-targets` and `cargo test --doc --workspace` were rotted (test targets never compile in the normal build); needed as a precondition for the analysis. Rotted test helpers deleted, doc-comment diagrams fenced. 2. **Narrow crate-internal visibility across all targets and delete the code it proves dead** — the narrowing (~1000 files, net-zero LOC) plus the deletions it enables (~230 crate files): unused fns/methods/consts/statics/fields (with their write sites)/enums/imports, APIs used only by their own tests (deleted with those tests; surviving APIs keep coverage), and the never-constructed `bun_safety::CriticalSection` module. Items read only on one platform are `#[cfg(...)]`-gated instead of deleted. 3. **codegen: exempt generated Rust from dead_code/unreachable_pub lints** — generators emit `#[allow(...)]` on generated items so an unused generated item is a generator concern rather than a workspace compile error. Kept as a separate tip commit; happy to narrow to `unreachable_pub` only or drop it if you'd rather keep `dead_code` live on generated output. **Net: −1,941 lines over 1,061 files.** (The narrowing is the enabler; the dead-public deletion pass — items no product root reaches at all — is the follow-up.) ### Deliberately kept (reads invisible to rustc) - Fields of `MultiArrayList` element structs (`WatchItem`, `EntryPoint`, `JSMeta`, `InputFile`, `Store::Entry/Node`, …) — read through generated `items::<"field">` accessors. - Ownership/liveness anchors — `AsyncModule::ast_alloc_state`, `CurrentBundle::{bv2,heap,ast_alloc_state}`, GC-protect handles, `KEventWaker::machport_buf`. - Types type-punned through raw-pointer casts (`SerializedSourceMap` ↔ `SerializedSourceMapLoaded`). - FFI structs still named in `pub extern` signatures; `#[repr(...)]` layouts untouched; no `#[no_mangle]`/`extern` items removed. - `PEFile::init` keeps its per-section alignment-overflow validation after dropping the write-only fields it computed. ## Test plan - [x] `cargo check --workspace --all-targets` clean on `x86_64-unknown-linux-gnu`, `aarch64-unknown-linux-musl`, `x86_64-unknown-freebsd`, `aarch64-apple-darwin`, `x86_64-pc-windows-msvc` - [x] `cargo check` with `--cfg=bun_asan --cfg=socket_fault_injection` clean - [x] `cargo test --doc --workspace` passes - [x] `bun bd` builds and links; debug binary smoke-tested (eval, TS run, `Bun.serve` round-trip, `node:fs`/`path`/`crypto`, spawn) - [x] `test/js/bun/util/inspect.test.js` (73/73), `test/js/node/path/parse-format.test.js` via `bun bd test` - [x] Restored crate unit tests pass: `bun_collections` 34, `bun_paths` 14, `bun_clap` 9, `bun_base64` 3 - [ ] Full CI matrix --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
liooil
pushed a commit
to liooil/poly
that referenced
this pull request
Aug 7, 2026
…he code it proves dead (#36184) ## Summary Runs a multi-target visibility analysis over the Rust workspace with a fork of Astral's `hawk` (multi-target support upstreamed as astral-sh/hawk#150) so that `pub` items with no cross-crate use on **any** shipped platform are narrowed to `pub(crate)`/private. Once narrowed, rustc's denied `dead_code`/`unreachable_pub` lints can prove code dead across crate boundaries, and that code is deleted. Three commits: 1. **Make workspace test and doctest targets compile** — `cargo check --workspace --all-targets` and `cargo test --doc --workspace` were rotted (test targets never compile in the normal build); needed as a precondition for the analysis. Rotted test helpers deleted, doc-comment diagrams fenced. 2. **Narrow crate-internal visibility across all targets and delete the code it proves dead** — the narrowing (~1000 files, net-zero LOC) plus the deletions it enables (~230 crate files): unused fns/methods/consts/statics/fields (with their write sites)/enums/imports, APIs used only by their own tests (deleted with those tests; surviving APIs keep coverage), and the never-constructed `bun_safety::CriticalSection` module. Items read only on one platform are `#[cfg(...)]`-gated instead of deleted. 3. **codegen: exempt generated Rust from dead_code/unreachable_pub lints** — generators emit `#[allow(...)]` on generated items so an unused generated item is a generator concern rather than a workspace compile error. Kept as a separate tip commit; happy to narrow to `unreachable_pub` only or drop it if you'd rather keep `dead_code` live on generated output. **Net: −1,941 lines over 1,061 files.** (The narrowing is the enabler; the dead-public deletion pass — items no product root reaches at all — is the follow-up.) ### Deliberately kept (reads invisible to rustc) - Fields of `MultiArrayList` element structs (`WatchItem`, `EntryPoint`, `JSMeta`, `InputFile`, `Store::Entry/Node`, …) — read through generated `items::<"field">` accessors. - Ownership/liveness anchors — `AsyncModule::ast_alloc_state`, `CurrentBundle::{bv2,heap,ast_alloc_state}`, GC-protect handles, `KEventWaker::machport_buf`. - Types type-punned through raw-pointer casts (`SerializedSourceMap` ↔ `SerializedSourceMapLoaded`). - FFI structs still named in `pub extern` signatures; `#[repr(...)]` layouts untouched; no `#[no_mangle]`/`extern` items removed. - `PEFile::init` keeps its per-section alignment-overflow validation after dropping the write-only fields it computed. ## Test plan - [x] `cargo check --workspace --all-targets` clean on `x86_64-unknown-linux-gnu`, `aarch64-unknown-linux-musl`, `x86_64-unknown-freebsd`, `aarch64-apple-darwin`, `x86_64-pc-windows-msvc` - [x] `cargo check` with `--cfg=bun_asan --cfg=socket_fault_injection` clean - [x] `cargo test --doc --workspace` passes - [x] `bun bd` builds and links; debug binary smoke-tested (eval, TS run, `Bun.serve` round-trip, `node:fs`/`path`/`crypto`, spawn) - [x] `test/js/bun/util/inspect.test.js` (73/73), `test/js/node/path/parse-format.test.js` via `bun bd test` - [x] Restored crate unit tests pass: `bun_collections` 34, `bun_paths` 14, `bun_clap` 9, `bun_base64` 3 - [ ] Full CI matrix --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This branch has not been deployed
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.
Note
This PR — the code, tests, docs, and this description — was entirely generated by Claude Code, at the direction of @Jarred-Sumner for use in Bun.
Summary
Add a
targetsconfiguration list so a singlecargo hawk checkcompiles the workspace for several target triples and unions the reachability evidence across all of them before producing diagnostics:Hawk already unions evidence across
[[feature-profile]]entries; this extends that same dimension to compilation targets. Every production binary and workspace non-production target is compiled once per (feature profile, target), so a declaration required on any configured platform is preserved. With notargetsconfigured, behavior is unchanged (the host target, or--target).--targeton the command line is rejected when atargetslist is configured.Motivation
We hit this analyzing Bun's Rust workspace (~200 crates, 11 shipped triples). A single-target run reports Windows-only FFI and Darwin-only code as dead, so to get a set that is safe to delete we currently run Hawk once per triple and intersect the findings by hand — and
--fixis unusable because it would restrict surface another platform still needs. With this change, a two-target run over the workspace produces the platform-correct set directly (verified: nocfg(windows)-only declarations reported when analyzing from a Linux host).--fixwith multiple configurationsTo make the union actionable,
--fixnow computes one fix plan from the combined evidence and applies it under every configured feature profile and target before re-analyzing, so a declaration compiled in only one configuration is edited by that configuration's pass. This lifts the existing guard that rejected--fixwith multiple feature profiles. I'm happy to split that behavior into a separate PR if you'd prefer to review it independently.Tests
multi_targetsfixture: a library withcfg(unix)-only andcfg(windows)-only public APIs and helpers, an app calling both under matching cfgs, and a genuinely deadpub fn. Tests assert the union keeps the platform APIs, still reports the truly dead item, and that--fix --allow-no-vcsrestricts only what is unused on every target. Also covers the--targetvstargetsconflict.feature_profile_fixesfixture covering--fixacross feature profiles.targetslist (defaults, duplicates, empty entries).rust-toolchain.tomlgainsx86_64-pc-windows-msvcsorustup showin CI installs the second target's std for the fixture.cargo test --workspace --all-features --locked,cargo fmt, andcargo clippy --workspace --all-targets --all-features -- -D warningsare clean locally.