Skip to content

Remove dead code from the bun_aot manifest, rust-test.ts and .prettierignore - #48355

Open
robobun wants to merge 1 commit into
mainfrom
robobun/fb59e37b/dead-code-sweep
Open

robobun wants to merge 1 commit into
mainfrom
robobun/fb59e37b/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • AOT: front end, test runs and tests for the rewritten compiler #47942 (the AOT rewrite) left four items with nothing to use or match:
    • The bun_zstd dependency of bun_aot.
    • --skip realpath_under_the_marked_root_stays_under_it in scripts/rust-test.ts. The test is gone.
    • bun_sql_jsc in the crate list of that script. The crate has no test left.
    • Three entries of .prettierignore that match no file.
  • Nothing fails because of them.

Fix

  • Delete the four. startProgram now passes [].
  • Correct because no file names or matches a removed item, on main and in the 6,221 open pull requests.
  • Verified: bun bd, bun run rust:check-all, bun run rust:test (52 of 52 programs pass). That step compiles 21 fewer units (13 crates) and runs the same 6,005 tests.
  • Self-reviewed: 12 concerns raised, 11 addressed. Rejected: delete membersUnderNamesOfTheRuntime here too (see Background).

Behaviour change: none

Background

  • scripts/rust-test.ts is the cargo-test step of CI. It starts one test program for each listed crate. --skip NAME leaves out each test whose name contains NAME.
  • The build hashes the dependency list of a crate into its symbol names (hashesOf in scripts/build/rust/units.ts). So bun_aot compiles again under new names.
  • Also considered: delete membersUnderNamesOfTheRuntime (146 lines, no caller). It is an input of code generation and needs a decision (see Notes).

Downsides

  • A #[test] added later under src/sql_jsc runs in CI only when the crate is back in the list. No open pull request adds one.
  • CI links with the function order of the last main build. Its 553 names of bun_aot match nothing for one build. No function loses its place (see Notes).
Notes

Removed (12 lines)

Item Lines What it used or matched, deleted by #47942 Search on main Hits
bun_zstd in [dependencies] of src/aot/Cargo.toml, and its line in Cargo.lock -2 six lines of src/aot/report/source_texts.rs that called bun_zstd:: git grep bun_zstd -- src/aot 1: the manifest line
programArgs with its comment, scripts/rust-test.ts -4, +1 fn realpath_under_the_marked_root_stays_under_it at src/sema/standalone/lib.rs:143 git grep realpath_under_the_marked_root_stays_under_it 1: the skip
"bun_sql_jsc" in crates, scripts/rust-test.ts -1 the 11 tests of src/sql_jsc/shared/RowDecoder.rs (460 lines) git grep -c -E '#\[test\]|cfg\(test\)' -- src/sql_jsc 0 in 50 files
Three entries, one comment and one blank line of .prettierignore -5 four files of test/aot/parallel/errors/ git ls-files with each entry 0
  • bun_sema_standalone keeps its own bun_zstd dependency. Under cfg(any(test, feature = "test_support")) it compiles src/runtime/cli/typescript_libs.rs by #[path], and that file calls bun_zstd.
  • bun_zstd stays in the dependency graph of bun_aot through bun_builtin_sources, bun_bundler, bun_sema and bun_sema_standalone, so the build compiles the same crates.
  • rustc reports bun_zstd to Cargo as an unused extern of bun_aot, and Cargo does not show it. With the manifest of main, cargo check -p bun_aot --all-targets prints no warning. With CARGO_LOG=cargo::diagnostics::rules::unused_dependencies=debug it logs "ignoring unused extern bun_zstd". The rustc lint unused_crate_dependencies is not in [workspace.lints.rust].
  • libtest matches --skip by substring. No test name in the workspace contains the skipped name, so the argument changed nothing.
  • The comment above crates gives the rule: "To add a crate it must have at least one #[test]". bun_lint_react_compiler is the other row without a test. Remove dead code from bun_boringssl_sys and bun_lint_react_compiler #47918 already removes it.
  • cargo test --no-run --unit-graph for the list with and without bun_sql_jsc: 287 and 266 units. The 21 units that go are in 13 crates that no other listed crate reaches: bun_bunfig, bun_clap, bun_clap_macros, bun_ini, bun_install, bun_jsc, bun_output, bun_patch, bun_s3_signing, bun_spawn, bun_sql, bun_sql_jsc and bun_transpiler. No unit of another crate changes its feature set, so the other test programs are built from the same units. cargo check --workspace --all-targets in .github/workflows/rust-lints.yml still compiles the 13.
  • The five other entries of that block of .prettierignore each match one file or more. bun fmt skips these folders through ignorePatterns in bunfig.toml, so the entries matter only to a direct run of prettier.
  • The symbol order file. CI links a build with the order file of an earlier build of main (inheritOrderFile in scripts/build/ci.ts). A comment there says that the symbol names of a crate change with its dependencies. The self-review measured this change on the canary build of 2596ac5. The order file traced from that build has 553 names of bun_aot among 33,176, on 88 of 21,651 ordered addresses (29,247 bytes). They stop matching until CI traces the next build of main, and no function loses its place. Each change of a dependency list has this effect.

Checks

  • bun bd builds the head. bun:internal-for-testing loads in it.
  • bun run rust:check-all: 12 ok, 0 failed, 0 skipped (of 12).
  • cargo check -p bun_aot --all-targets --all-features and cargo check -p bun_aot -p bun_sema_standalone --all-targets on linux-x64. Both also compile the unit tests and tests/ir_tests.rs.
  • bun run rust:test on the head: 52 of 52 test programs pass, with 6,005 tests, 0 ignored and 0 filtered out. The same run before the row was removed had 53 programs and the same 6,005 tests. Each other program has the same count in both runs.
    • Both runs had test/node_modules hidden, as in the CI step, which installs only the root packages. With that folder present, lower in tests/ir_tests.rs fails on one fixture, on main too: require("n" + name) in test/aot/ir/lower/modules/names-that-can-only-be-paths/loads.cjs finds the packages there.
  • cargo metadata --locked accepts Cargo.lock.
  • tsc -p scripts/tsconfig.json --noEmit reports nothing. prettier --check scripts/rust-test.ts passes.
  • prettier --list-different test/aot prints the same list with the .prettierignore of main and with this one.
  • Open pull requests. The heads of all 6,197 were fetched at 04:18 UTC on 2026-11-09. The 130 that changed or opened until 11:38 UTC were fetched again (6,221 open then).

Found in this run, and left alone

How the candidates were found

  • What AOT: front end, test runs and tests for the rewritten compiler #47942 left: every identifier on a line that the commit removed was counted again in today's tree. The items above are those that now occur once, or that lost their last test.
  • Exports of bun:internal-for-testing: each of the 110 was searched in test/ and bench/. Two have no user there: membersUnderNamesOfTheRuntime and resolveComputedSpecifier.
  • Rust, by the linker. The debug objects were linked again with --gc-sections. Of 1,047,549 text symbols in the bun_* rlibs the link keeps all but 18,200. 227 of those are functions written in src/ (the rest are instances of generics, macro output and generated code). Each has a caller on Windows or macOS, in a unit test, at compile time, or from the debugger helpers in btjs.rs.
  • Rust, by rustc. cargo check --workspace with --force-warn dead_code --force-warn unused_macro_rules on linux-x64, windows-x64 and macos-arm64. Outside h2/ it reports the define_colorspace arm and the #[expect(dead_code)] items that are used on another platform.
  • Rust files: every .rs file under src/ that those three checks did not compile has a mod line behind a #[cfg] (tests, benches, FreeBSD, fuzzilli).
  • C and C++. clang -fsyntax-only per file with -Wunused-function -Wunused-member-function -Wunused-template -Wunused-macros -Wunneeded-internal-declaration -Wunreachable-code -Wunreachable-code-break -Wunreachable-code-return over the 623 entries of compile_commands.json for src/ and packages/. 513 compile alone. Each hit is in an open pull request, is used in another translation unit, or is used on another platform (us_quic_send_one).
  • TypeScript. tsc --noUnusedLocals over scripts/, scripts/build/, src/, src/js/ and src/runtime/bake/. Top-level definitions of scripts/, src/codegen/ and src/js/ whose name occurs once.
  • Build configuration. No reference to libspng, libarchive, HdrHistogram or bytemuck is left. Each file under patches/ has a reader. Each aot/... path in scripts/runner.node.ts exists.
  • Commented-out code. 80 blocks of two or more lines outside Rust, 49 of them in open pull requests. The others are notes or quotations of Node.js. The Rust files are younger than six months.

Self-review

  • The first cut of this change also deleted membersUnderNamesOfTheRuntime (152 lines in total). The review found the table rows and the layout change described above, so the function is out of the diff.
  • The review asked for the stale .prettierignore entries and for the rows of the crate list without a test. Both are in the diff, apart from the row that Remove dead code from bun_boringssl_sys and bun_lint_react_compiler #47918 holds.
  • Rejected: delete the function here too, and describe the effect in the pull request. A sweep deletes only what is certain.
  • Parts of other concerns that are not in this change:
    • Delete test/aot/ways-to-enter-a-function.ts. The sweep does not edit test/.
    • One pull request with all thirteen unused dependency lines and a check that keeps the count at zero. Open pull requests hold twelve of the lines, and a check is a separate change.

Open sweep pull requests


[policy-decision:dep] gate passed · iteration 0 · 4 files touched

passes on PR (with fix)
Dependency/toolchain change; no test proof to run.
diff hotspot
.prettierignore      | 5 -----
 Cargo.lock           | 1 -
 scripts/rust-test.ts | 6 +-----
 src/aot/Cargo.toml   | 1 -
 4 files changed, 1 insertion(+), 12 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                  reads  edits  tests
.prettierignore           1      1      9
Cargo.lock                0      0      6
scripts/rust-test.ts      2      3     13
src/aot/Cargo.toml        0      0      4

…rignore

Four leftovers of #47942, which deleted what used or matched them:

- The bun_zstd dependency of bun_aot. Its only user was
  src/aot/report/source_texts.rs.
- The --skip of realpath_under_the_marked_root_stays_under_it in
  scripts/rust-test.ts. The test was in src/sema/standalone/lib.rs.
- bun_sql_jsc in the crate list of scripts/rust-test.ts. Its 11 tests were
  in src/sql_jsc/shared/RowDecoder.rs, so its test program ran no test.
- Three entries of .prettierignore that match no file.
@robobun

robobun commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:08 AM PT - Nov 9th, 2026

✅ @robobun, your commit b6d9da592b509c5c01e8673b8ee8c7d0aaf63414 passed in Build #139298! 🎉


🧪   To try this PR locally:

bunx bun-pr 48355

That installs a local version of the PR into your bun-48355 executable, so you can run:

bun-48355 --bun

@robobun

robobun commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. The change passes every CI job that ran. Four jobs of one lane wait for an agent.

This pull request has no repro. It deletes 12 lines in 4 files, and nothing uses or matches them on main (commit b6d9da5 on 2596ac5).

Checks that were run on the head:

  • bun bd builds.
  • bun run rust:check-all: 12 of 12 targets pass.
  • bun run rust:test: 52 of 52 test programs pass, 6,005 tests. Before the change: 53 programs, the same 6,005 tests.
  • cargo test --no-run --unit-graph: 266 units, 287 before. No unit of another crate changes its feature set.
  • The heads of the open pull requests were searched for a use of each removed item. None has one.

CI on this head (Buildkite build 139298):

  • 184 of 188 jobs passed. That includes the 14 build-bun jobs and the cargo-test step with the new crate list (53 of 53 programs, 6,005 tests).
  • The other four jobs are darwin x64 - test-bun. They find no free agent and expire, and CI puts them in the queue again each time (first at 13:05 UTC, then after each one or two hours). At 16:07 UTC that lane had 198 waiting jobs for 9 agents.
  • Two tests failed once and passed on a retry: test/js/bun/http/bun-serve-html.test.ts and test/aot/runner-suites/mock-module-runs-3.test.ts. They fail the same way in 12 and 10 of the last 60 builds, on other branches. This change touches no code that they run.
  • I did not start CI again. A new run goes to the end of the same queue.

Two items need a decision and are not in the diff. The Notes of the description give the details:

  • membersUnderNamesOfTheRuntime in src/js/internal-for-testing.ts (146 lines, no caller, but an input of code generation).
  • The outbound half of the node:http2 engine in src/runtime/api/bun/h2/ (about 230 lines that only unit tests call).

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — a small, mechanical removal of four stale references, each confirmed dead by reading the tree.

What was reviewed:

  • bun_zstd in src/aot/Cargo.toml: no zstd use in src/aot, including #[path]-included files, the include!-ed aot_* codegen outputs and embed_compressed! sites; Cargo.lock changes only bun_aot's entry.
  • scripts/rust-test.ts: no test named realpath_under_the_marked_root_stays_under_it remains anywhere; src/sql_jsc has no #[test], cfg(test) or doctest fence (its RowDecoder.rs tests were deleted by #47942), so the row violated the list's own "at least one #[test]" rule.
  • .prettierignore: the three removed patterns match no file in the tree; the neighboring kept entries still do.
  • Not run here: cargo check/rust:test, so the "still compiles without the dependency" claim rests on the grep, not on a build.
Extended reasoning...

The change touches four files (+1/-12): drops the unused bun_zstd dependency from the bun_aot crate manifest and lockfile, removes a stale --skip filter and a test-less crate from the cargo-test driver script, and deletes three .prettierignore entries whose target fixtures no longer exist. It touches no runtime code and no security-sensitive surface. Every removed item was verified dead by searching the checkout, the base commit (#47942) is where the referenced test, fixtures and zstd callers were deleted, no CODEOWNERS entry covers the changed paths, and the bug hunt ran dry with no findings, which decided approve.

@robobun

robobun commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

On "not run here: cargo check / rust:test": the claim rests on builds of this head (b6d9da5), and not only on the search.

  • CI: the cargo clippy job passed. Its step cargo check --workspace --all-targets --keep-going compiles bun_aot with its unit tests and tests/ir_tests.rs, without the bun_zstd line.
  • Local: bun bd, bun run rust:check-all (12 of 12 targets) and cargo check -p bun_aot --all-targets --all-features pass.
  • Local: bun run rust:test passes 52 of 52 test programs with 6,005 tests. The run before the bun_sql_jsc row was removed had 53 programs and the same 6,005 tests.

The Buildkite build (#139298) is still in progress. Its cargo-test step runs scripts/rust-test.ts with the new list.

@coderabbitai

coderabbitai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The changes remove three fixture exclusions from .prettierignore, update Rust test selection and executable arguments, and remove the bun_zstd dependency declaration from the AOT package.

Changes

Prettier fixture exclusions

Layer / File(s) Summary
Fixture exclusions
.prettierignore
The ignore list no longer excludes the single-name-read, typed multi-line quoted-expression, and static-private-fields fixtures. Three other fixture exclusions remain.

Rust test runner

Layer / File(s) Summary
Test selection and launch
scripts/rust-test.ts
The crate list no longer includes bun_sql_jsc. Test executables now start without the skip argument for realpath_under_the_marked_root_stays_under_it.

AOT dependency declaration

Layer / File(s) Summary
AOT dependency declaration
src/aot/Cargo.toml
The bun_zstd workspace dependency declaration was removed.

Suggested reviewers: jarred-sumner

Merge Risk: Minimal · up to b6d9d

The cleanup has no established behavior regression and is mergeable after normal checks.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: removing dead code from the bun_aot manifest, scripts/rust-test.ts, and .prettierignore. The diff removes the bun_zstd dependency from src/aot/Cargo.toml, the --skip a…
Description check Passed The description covers the required template sections. 'What does this PR do?' is covered by the Problem, Fix, and Background sections, which explain the four removed items and why they are dead. 'How…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: removing dead code from the bun_aot manifest, scripts/rust-test.ts, and .prettierignore. The diff removes the bun_zstd dependency from src/aot/Cargo.toml, the --skip argument and bun_sql_jsc entry from scripts/rust-test.ts, and three stale entries from .prettierignore. The title matches the changeset and the PR objectives. It is concise and specific, and it does not list every file in the diff.

Full details: Description check

Explanation

The description covers the required template sections. 'What does this PR do?' is covered by the Problem, Fix, and Background sections, which explain the four removed items and why they are dead. 'How did you verify your code works?' is covered by the Checks section and the verification list (bun bd, rust:check-all, rust:test, cargo metadata, tsc, prettier). Extra detail such as open-PR searches and the Notes section is beyond the template but does not make the description incomplete. Some of the Notes text is long and hard to scan, but the required information is present.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 2596ac5 and b6d9da5.

Files ignored due to path filters: 1

  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 3

  • .prettierignore
  • scripts/rust-test.ts
  • src/aot/Cargo.toml

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants