Skip to content

Remove dead code from bun_sys, bun_errno, and bun_libuv_sys - #45698

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/72e5c55a/dead-code-sweep
Closed

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

Conversation

@robobun

@robobun robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Seven pub items in bun_sys, bun_errno and bun_libuv_sys have no user on any target: O::NOATIME, O::NOFOLLOW_ANY, posix::s_isdir, the Windows umask, the Windows UTIME_NOW, the Linux and Android posix::mode_t alias, and UvHandle::data.
  • rustc's dead_code lint exempts pub items. The unused_pub lint of the mordant job reports them, but mordant-baseline.toml allows the 160 findings that predate the job.

Fix

  • Delete them: 35 lines in 3 files. Lower two unused_pub counts in mordant-baseline.toml. The Notes give each proof.
  • Each removed name has no other occurrence in the Rust sources or the generated code, or only occurrences that a cfg for another platform holds.
  • Verified: bun run rust:check-all (12 targets), cargo check --workspace --all-targets, bun run rust:mordant, cargo fmt --all --check, bun bd, and three test files with the debug build (Notes).
  • Self-review: did not finish. It reported no result.

Background

  • The finder marks every pub fn, const, static, type alias and trait method #[deprecated] in a scratch tree. --force-warn deprecated then prints every use. Visibility does not change, so names resolve as in the real build.
  • 15,100 items were checked for linux, windows and macOS. 158 have no use. Open dead-code pull requests already delete 111 of them.
  • No other design was weighed: a deletion has one place.

Downsides

  • None found. Checked each removed name with rg over src/, packages/, scripts/, test/ and build/debug/codegen/, and cargo check for 12 targets.

Behaviour change: none

Notes

Removed, by file

  • src/sys/lib.rs
    • O::NOATIME, three cfg variants. No Rust code names it. The comment above the Windows variant named a bit-test in openat_windows_impl that is gone. The comment now names only O::PATH, which that function still tests.
    • O::NOFOLLOW_ANY, two cfg variants, and its comment. No Rust code names it.
    • posix::s_isdir. No caller. s_islnk beside it has callers and stays.
    • UTIME_NOW, the cfg(windows) variant. bun_sys::c re-exports the constant only under cfg(unix). The one reader, from_now in src/runtime/node/time_like.rs, is cfg(not(windows)).
    • umask in windows_impl. The one caller, exec_mode in src/install/bin.rs, is cfg(not(windows)).
  • src/errno/lib.rs
    • posix::mode_t, the Linux and Android alias. The one import, bun_sys::posix::mode_t in src/spawn_sys/posix_spawn.rs, is cfg(target_os = "macos"). All other code uses libc::mode_t.
  • src/libuv_sys/libuv.rs
    • UvHandle::data. A default trait method with no caller and no override. Callers read the data field or call get_data.
  • mordant-baseline.toml
    • unused_pub:src/sys/lib.rs 55 to 48 and unused_pub:src/libuv_sys/libuv.rs 41 to 40. These are the values that bun run rust:mordant:baseline writes on this branch. That command also lowers one generic_body_not_generic count that this diff does not cause, so that line is not in this diff.
    • mordant counts seven of the eight removed bun_sys definitions. It does not count the mode_t alias, which a glob re-export reaches.

Base

  • main at 7f3bc6a. The later commits on main touch none of these four files, and git merge-tree merges them without a conflict. Every check below ran on this base.

Method

  • Marker: #[deprecated(note = "DC#<id>")] #[allow(dc_live_<id>)] in front of each item, in a scratch state of the tree. --force-warn deprecated lists each use with its site, also a use inside another marked item. --force-warn unknown_lints prints dc_live_<id> once for each marked item that survives cfg stripping. That tells an item that a target does not compile apart from an item that the target compiles and never uses.
  • Pass 1: 15,100 pub fns, consts, statics, type aliases and trait methods. 220,321 uses on linux. 158 items have no use on linux, windows or macOS (#[no_mangle] and other exported items count as used). 111 are in open dead-code pull requests.
  • Cross-check: the per-file counts of pass 1 equal the unused_pub counts of mordant-baseline.toml for 17 of its 22 files. Three of the other five files hold types, which pass 2 covers.
  • Pass 2: 6,149 pub types and foreign items. 32 have no use. 28 are in open dead-code pull requests. Three libuv declarations lose their last user only when Remove dead code from bun_core, the JSC bindings, the debugger, and 24 other crates #41385 merges. The last one, SliceIterator in bun_clap, has unit tests.
  • Pass 3: 8,106 pub fields. A field that no other crate accesses became pub(crate), and rustc then reported 86 never-read fields on linux. 36 are in open dead-code pull requests. None of the other 50 is in this diff (see the leads).
  • The 47 items of pass 1 that no open pull request deletes: 7 are in this diff. The others stay:
    • Used by unit tests: CowSlice::init_dupe, simdutf::Status::TOO_SHORT, __IsFreeze::IS_FREEZE (it also makes a static assertion fail on purpose).
    • Used on FreeBSD: UV_ENOTSUP, UV_ENODATA (posix), O::EVTONLY.
    • Named by a codegen template: host_fn_setter, host_fn_this_value, and the JsClass::estimated_size default.
    • Used by a macro arm of the embed build: inflate_embedded, inflate_embedded_nul.
    • A slot in a positional table: Tag::fchownat.
    • 24 E-prefixed aliases of the Windows E enum (see the leads).

Tests, debug build (all with --timeout 180000, the ASAN build is slow)

  • test/js/bun/io/bun-write.test.js: 106 pass, 0 fail.
  • test/cli/install/bun-lockb.test.ts: 66 pass, 0 fail.
  • test/js/node/fs/fs.test.ts: 612 pass, 3 fail, 8 skip. The three failures are the readdir(path, {recursive: true}) should work x 100 tests. Each has its own 10 s limit, and each also fails when it runs alone. One recursive readdir of test/js/node returns the same 6,758 entries on this debug build and on the installed release build (1.4.3-canary). It takes 12 times (sync) to 37 times (async) longer on the debug build of this machine. The installed release build passes the three tests in 1.2 s to 2.3 s. I did not build the merge base to compare.

Leads, not in this diff

  • src/errno/windows_errno.rs: 24 aliases such as E::E2BIG and E::ECONNRESET have no user. The comment of the table says that it exists so that cross-platform source compiles on Windows. Remove dead code from the WebCore bindings and 20 Rust crates #40525 removes four entries of that table. The owner of the table decides.
  • Never-read pub fields that need an owner decision: ReactiveInstruction.effects (14 stores, no read), FunctionSignature.return_type, InstrumentationConfig.gating and ExternalFunctionConfig.source in bun_react_compiler. JsDoc.start, JsDoc.end, Finished.step and Finished.index in bun_sema. 16 fields of the lint AST and rule API.
  • Not dead, although rustc reports them: the columns of WatchItem and LineOffsetTable (read through MultiArrayList), four backing-storage fields (OutputFile.owned_src_path_text, ParseResult.source_contents_backing, HTTPResponseMetadata.owned_buf, AsyncModule.ast_alloc_state), and DumpStackTraceOptions.skip_* (read on macOS and Windows).

Open dead-code pull requests


no test proof · iteration 0 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check

Seven pub items have no user on any target:

- bun_sys: O::NOATIME, O::NOFOLLOW_ANY, posix::s_isdir, the Windows
  umask, and the Windows UTIME_NOW
- bun_errno: the Linux and Android posix::mode_t alias
- bun_libuv_sys: UvHandle::data

Lower the two unused_pub counts in mordant-baseline.toml to match.
@robobun

robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 6:27 AM PT - Oct 17th, 2026

❌ @robobun, your commit 72d96eb has 2 failures in Build #127345 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 45698

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

bun-45698 --bun

@robobun

robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 18997bb6-b2fd-48d3-b791-d37844b4dea1
📥 Commits

Reviewing files that changed from the base of the PR and between ff8ad61 and 72d96eb.

📒 Files selected for processing: 4
  • mordant-baseline.toml
  • src/errno/lib.rs
  • src/libuv_sys/libuv.rs
  • src/sys/lib.rs
💤 Files with no reviewable changes: 2
  • src/libuv_sys/libuv.rs
  • src/errno/lib.rs

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

Walkthrough

This change removes selected platform-specific declarations from the system and errno modules, removes UvHandle::data while retaining get_data<T>, and updates two unused-public-item baseline counts.

Changes

API cleanup

Layer / File(s) Summary
Platform-specific declarations
src/errno/lib.rs, src/sys/lib.rs, mordant-baseline.toml
Removes the Linux and Android mode_t alias and selected platform-specific constants and functions. Updates the unused-public-item baseline count for src/sys/lib.rs.
Libuv handle data accessor
src/libuv_sys/libuv.rs, mordant-baseline.toml
Removes UvHandle::data; get_data<T> remains. Updates the corresponding unused-public-item baseline count.

Suggested reviewers: alii

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 72d96

The cleanup removes unused declarations while retaining the in-repository handle-data access path. No concrete behavior change or merge-blocking risk is established, so the PR appears ready for normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
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.
Title check Passed The title clearly summarizes the primary change: removing unused public items from the three named components.
Description check Passed The description explains the problem, lists the removed items, documents the verification commands and test results, and states the behavior impact. It does not use the template headings exactly, but …
Full details: Description check

Explanation

The description explains the problem, lists the removed items, documents the verification commands and test results, and states the behavior impact. It does not use the template headings exactly, but it provides the required information in equivalent sections.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • 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.

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

@robobun

robobun commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

Two open PRs call bun_sys::posix::s_isdir, which this PR deletes.

Neither PR changes the definition. If this PR lands first, each of them stops compiling after a merge with main. Two ways out:

  • Keep s_isdir in this PR.
  • Change the two call sites to sys::S::ISDIR(mode), which does the same test.

@robobun

robobun commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this pull request. I did not check the removed items against the other open pull requests before I opened it. That check now shows that six of the seven items have a user in an open pull request:

Main has no merge queue. #45749 and #44937 merge with this branch without a text conflict, so each would stop compiling on main after this one merged. The seventh item, UvHandle::data, is in src/libuv_sys, which #42819 deletes, so a pull request for it alone has no value.

The check: the Rust lines that each of the 5,637 open pull requests adds against its merge base with main, searched for the seven names.

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