Skip to content

Remove dead code from bun_sys, bun_format, boringssl_sys, and two unused Cargo dependencies - #46728

Open
robobun wants to merge 3 commits into
mainfrom
robobun/e7484a2e/dead-code-sweep
Open

robobun wants to merge 3 commits into
mainfrom
robobun/e7484a2e/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Eight items have no user on any build target: two bun_sys items, one BoringSSL declaration, one Display impl, two log scopes, and two Cargo dependency edges.
  • rustc does not report them. Its dead_code lint exempts pub items, trait impls, and the pub static that declare_scope! emits.

Fix

  • Delete bun_sys::O::NOFOLLOW_ANY, bun_sys::posix::s_isdir, the SSL_set0_verify_cert_store declaration, impl Display for bun_format::verify::Difference, and the CLI and PathWatcherManager log scopes.
  • Remove bstr from bun_css_jsc and bun_crash_handler from bun_spawn. Remove the bun-asan-valgrind branch of bunExeName, because resolveConfig rejects that combination.
  • Correct because nothing names these items. The workspace compiles without them on all 12 targets, and a search finds no other use of each name.
  • Verified: cargo check --workspace --all-targets on linux, cargo check --workspace on the other 11 targets, a check with the release cfgs, cargo fmt --all --check, bun bd, and 10 test files.

Background

  • A log scope is a logger static that declare_scope! declares. A scope with no scoped_log! call does nothing.
  • bunExeName maps a build configuration to the name of the binary.
  • The candidates come from two whole-program checks. One tags each candidate with #[deprecated] and reads the use sites that rustc reports. The other relinks the debug build with --gc-sections. The Notes list what the checks rejected.

Downsides

  • None found. Checked the use sites rustc reports on 12 targets. Searched src/, packages/, scripts/, docs/, clippy.toml, hawk.toml, and the generated code for each name.

Behaviour change: none

Notes

Removed items

  • bun_sys::O::NOFOLLOW_ANY (src/sys/lib.rs, both cfg arms and their comment). rg -w NOFOLLOW_ANY finds only the two definitions.
  • bun_sys::posix::s_isdir (src/sys/lib.rs). rg -w s_isdir finds only the definition.
  • SSL_set0_verify_cert_store (src/boringssl_sys/boringssl.rs). The only caller is C code (packages/bun-usockets/src/crypto/openssl.c:1769), which uses the BoringSSL header.
  • impl Display for Difference (src/format/verify.rs). verify::compare has one caller (src/lint/driver/fmt/mod.rs:940). It tests .is_ok() and never formats the value. The linker drops the fmt function from the debug build.
  • declare_scope!(CLI, hidden) (src/runtime/cli/mod.rs) and declare_scope!(PathWatcherManager, visible) (src/runtime/node/win_watcher.rs). No scoped_log! names them. The two Windows targets compile without the second one.
  • bstr in src/css_jsc/Cargo.toml. No source in the crate names it. 6292c65 removed the last use.
  • bun_crash_handler in src/spawn/Cargo.toml. No source in the crate names it. a950fc5 removed the last use. The crate doc in src/spawn/lib.rs listed it, so that sentence changes.
  • if (cfg.asan && cfg.valgrind) return "bun-asan-valgrind" in scripts/build/config.ts. resolveConfig asserts !(asan && valgrind). rg asan-valgrind finds no other use.

Method

  • Demote every pub in one crate to pub(crate) and run cargo check on that crate. rustc then reports each item that the crate itself does not use. This gave 41,479 candidates in 116 crates.
  • Tag each candidate with #[deprecated(note = "<id>")] and run cargo check --workspace --all-targets with --force-warn deprecated. rustc reports each resolved use site, which includes method calls and names that a text search cannot tell apart.
  • A candidate stays only if each use site is inside another candidate that is deleted too. 474 candidates had no outside use on linux.
  • The same check ran for those 474 on the 11 other targets, with the debug cfgs (bun_debug, bun_asan), with the release cfgs (bun_codegen_embed, --release, a release codegen directory), with --cfg miri, with bun_react_compiler/fixtures, and for the Windows shim binary. Seven items had no use. A second pass started from the Windows target to cover code that linux does not compile.
  • Trait impls: a relink of the debug build with --gc-sections --print-gc-sections lists 135 hand-written impl blocks with no kept method. Open PRs already delete 70 of them, and 37 more are Debug impls. I deleted nine of the other 28 as a test: one is in this diff (Display for Difference), and the workspace needs the other eight. I kept the remaining 19 after reading them: a doc comment describes them, a bound needs them, they are Drop impls, a unit test uses them, or they print a value for debugging (Display for FilePoll, CodePath, Segment).

Candidates that stayed

  • Used on another target: Default for store::Bytes (Windows, src/runtime/webcore/blob/read_file.rs), From<bun_sys::Error> for bun_watcher::Error (macOS and FreeBSD, KEventWatcher.rs).
  • Needed for a bound: the workspace does not compile without CloneIn for ArenaBox and five Spanned impls in src/format/js/print/. HasLen for bool and u64, JobContext for Never, and PartialEq for PackQueueItem serve a bound, a const assertion, and a supertrait. I read those from the source and did not delete them.
  • Used only by the output of #[derive(css::Parse, css::ToCss)]: Ellipse::parse, Ellipse::to_css, Circle::parse, Circle::to_css (src/css/values/gradient.rs). rustc does not report deprecated uses inside derive output, so I checked these by hand.
  • Named by a policy file: bun_core::output::pretty_errorln has no caller, but clippy.toml names it.
  • Table entries: Tag::fchownat and 23 errno constants in src/errno/windows_errno.rs have no user. They are entries of complete tables, so they stay.
  • Not checked on each platform: the linux arm of bun_errno::posix::mode_t has no user on linux. The macOS and FreeBSD arm was not checked.

Not in this diff

  • bun_sys::O::NOATIME has no user on any target (three cfg arms). The comment above the Windows arm says openat_windows_impl tests it, and it does not. Left for a follow-up because it needs a comment change.
  • The Windows arms of bun_sys::UTIME_NOW and bun_sys::umask have no user on Windows.
  • LibUVBackend, ResolveInfoRequest (src/runtime/dns_jsc/dns.rs) and ModuleLoader (src/jsc/ModuleLoader.rs) are log scopes with no log statement. They are next to lines that Remove dead code: 17 log scopes nothing logs to and four fields nothing reads, in 10 crates #44050 and Remove dead code from bun_collections, bun_core, the JSC bindings, and 37 re-exports #41268 delete, so this PR leaves them to avoid a conflict.
  • 39 hand-written Debug impls are never formatted. Remove dead code: 49 trait impls nothing uses, across 12 crates #43664 kept them as a debugging aid.
  • WebCore::ResourceTiming has a private constructor and no factory, so PerformanceResourceTiming::create has no caller. Removing that path is a refactor of about 130 lines of C++.
  • bun_core::output::IS_GITHUB_ACTION is written once and never read. Its store also makes the first call to is_github_action(), so removing it can change when the environment is read.
  • scripts/build/deps/libarchive.ts defines HAVE_STRUCT_STAT_ST_BIRTHTIM for FreeBSD. libarchive 3.8.9 reads only HAVE_STRUCT_STAT_ST_BIRTHTIME and HAVE_STRUCT_STAT_ST_BIRTHTIMESPEC_TV_NSEC. This looks like a misspelling, not dead code.
  • mordant-baseline.toml records 55 unused_pub findings for src/sys/lib.rs. This PR removes three pub items from that file. I did not regenerate the baseline, because cargo mordant is not installed where I ran the checks. bun run rust:mordant:baseline can lower the count.

Tests (debug build with ASAN)

  • test/cli/format/sort-imports.test.ts (107 pass), test/cli/format/toml-layout.test.ts (38), test/js/bun/spawn/spawn-env.test.ts (2), test/js/bun/spawn/exit-code.test.ts (5), test/js/node/fs/fs-stats-constructor.test.ts (7), test/js/node/fs/fs-open-cloexec.test.ts (5), test/cli/bun.test.ts (53), test/js/node/dns/dns-resolver-class.test.ts (16), test/js/node/dns/dns-lookup-keepalive.test.ts (4), test/js/bun/resolve/import-meta.test.js (34).
  • sort-imports.test.ts is not stable in the debug build: of four runs, two passed all 107 tests and two had tests that hit the 5 s default timeout (6 tests, then 1 test).
  • test/cli/format/format.test.ts ran once on an earlier build of this branch: 1546 pass, 81 fail. Each failure is the 5 s default timeout. A sample of nine tests from the same groups passes when run alone.

[policy-decision:dep] gate passed · iteration 1 · 10 files touched

passes on PR (with fix)
Dependency/toolchain change; no test proof to run.
diff hotspot
Cargo.lock                      |  2 --
 scripts/build/config.ts         |  1 -
 src/boringssl_sys/boringssl.rs  |  1 -
 src/css_jsc/Cargo.toml          |  1 -
 src/format/verify.rs            | 15 ---------------
 src/runtime/cli/mod.rs          |  2 --
 src/runtime/node/win_watcher.rs |  1 -
 src/spawn/Cargo.toml            |  1 -
 src/spawn/lib.rs                |  2 +-
 src/sys/lib.rs                  | 11 -----------
 10 files changed, 1 insertion(+), 36 deletions(-)

gate history · 1 passed · 0 rejected · iteration 1

evidence per changed file
file                             reads  edits  tests
Cargo.lock                           0      0      0
scripts/build/config.ts              0      0      2
src/boringssl_sys/boringssl.rs       0      0      0
src/css_jsc/Cargo.toml               0      0      0
src/format/verify.rs                 0      0      1
src/runtime/cli/mod.rs               0      0      0
src/runtime/node/win_watcher.rs      0      0      0
src/spawn/Cargo.toml                 0      0      0
src/spawn/lib.rs                     0      0      0
src/sys/lib.rs                       0      0      3

…sed Cargo dependencies

- bun_sys: O::NOFOLLOW_ANY and posix::s_isdir have no user on any target.
- bun_boringssl_sys: the SSL_set0_verify_cert_store declaration has no Rust caller.
- bun_format: nothing formats verify::Difference with Display.
- bun_runtime: the CLI and PathWatcherManager log scopes have no log statement.
- bun_css_jsc does not use bstr. bun_spawn does not use bun_crash_handler.
- scripts/build/config.ts: resolveConfig rejects asan with valgrind, so the
  bun-asan-valgrind name in bunExeName is unreachable.
@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:15 AM PT - Oct 24th, 2026

✅ @robobun, your commit 2ac1d8587d8570f5c466a8c676d197e14eeb2e42 passed in Build #131951! 🎉


🧪   To try this PR locally:

bunx bun-pr 46728

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

bun-46728 --bun

@robobun

robobun commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • The diff only removes items that nothing uses, so it adds no test.
  • Checked with this diff: cargo check --workspace --all-targets on linux, cargo check --workspace on the other 11 targets, a check with the release cfgs, cargo fmt --all --check, bun bd, and 10 test files. The PR description lists them.
  • CI on 2ac1d85 (Buildkite build 131951): 179 of 181 jobs passed and none failed when I last looked. Two jobs were still queued. Five test files passed on a retry: spawn.test.ts, spawn-stdin-readable-stream.test.ts, nth-anplusb-ident.test.ts, webview-chrome.test.ts, and node-net.test.ts. The only changes to bun_spawn in this PR are one line in Cargo.toml and one doc line, so the two spawn tests do not run changed code.

@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.

Code review found no issues

No high-confidence issues detected in this change.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 179664e and ef39e45.

Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 9
  • scripts/build/config.ts
  • src/boringssl_sys/boringssl.rs
  • src/css_jsc/Cargo.toml
  • src/format/verify.rs
  • src/runtime/cli/mod.rs
  • src/runtime/node/win_watcher.rs
  • src/spawn/Cargo.toml
  • src/spawn/lib.rs
  • src/sys/lib.rs
Files with no reviewable changes: 8
  • src/css_jsc/Cargo.toml
  • src/spawn/Cargo.toml
  • src/format/verify.rs
  • src/boringssl_sys/boringssl.rs
  • src/runtime/cli/mod.rs
  • scripts/build/config.ts
  • src/runtime/node/win_watcher.rs
  • src/sys/lib.rs

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

Walkthrough

The pull request removes a build-name special case, several Rust declarations and helpers, two crate dependencies, and logging-scope declarations.

Changes

Build and Rust Surface Cleanup

Layer / File(s) Summary
Executable naming
scripts/build/config.ts
When ASAN and Valgrind are both enabled, bunExeName now returns bun-asan through the existing ASAN branch.
Rust declarations and helpers
src/boringssl_sys/boringssl.rs, src/format/verify.rs, src/sys/lib.rs
The changes remove SSL_set0_verify_cert_store, the Difference formatter, O::NOFOLLOW_ANY, and posix::s_isdir.
Dependencies and logging scopes
src/css_jsc/Cargo.toml, src/spawn/Cargo.toml, src/spawn/lib.rs, src/runtime/cli/mod.rs, src/runtime/node/win_watcher.rs
The changes remove the bstr and bun_crash_handler dependencies, update the spawn dependency documentation, and remove the CLI and PathWatcherManager logging-scope declarations.

Suggested reviewers: dylan-conway

Priority: Low

Merge Risk: Minimal · up to ef39e

No concrete behavior regression or repository-contract violation is established; the PR is mergeable.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly summarizes the main change by identifying dead-code and unused dependency removal across the affected crates. It is specific and concise enough for repository history.
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.
Description check Passed The description is complete and relevant. It explains the problem, lists the removed items and dependency changes, states the behavior impact, and documents verification steps and test limitations. It…
Full details: Description check

Explanation

The description is complete and relevant. It explains the problem, lists the removed items and dependency changes, states the behavior impact, and documents verification steps and test limitations. It does not use the exact template headings, but it provides the required information.

  • 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

Comment @coderabbitai help to get the list of available commands.

Comment thread src/spawn/lib.rs Resolved

@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.

Code review found no issues

No high-confidence issues detected in this change.

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