Skip to content

Remove dead code from bun_windows_sys, bun_sys, and bun_libuv_sys - #44451

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

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

Conversation

@robobun

@robobun robobun commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Behaviour change: none

Problem

  • Five Rust items in bun_windows_sys, bun_sys and bun_libuv_sys have no user on any target.
  • dead_code exempts exported pub items. The unused_pub lint of mordant has two in its baseline and misses three, because a re-export counts as a use.

Fix

  • bun_windows_sys::externs: remove SYMBOLIC_LINK_FLAG_DIRECTORY, SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE and the ProcessParameters alias, with their re-exports in bun_sys::windows and bun_core::windows_sys.
  • bun_sys: remove the Windows posix::timespec struct. bun_libuv_sys: remove open_handles::count.
  • mordant-baseline.toml: remove the open_handles.rs entry and lower the src/sys/lib.rs count by one.
  • Verified: bun run rust:check-all (12 targets), bun run rust:mordant, the launcher binary on both Windows targets, Windows --release and --all-targets, bun bd.

Background

  • bun_windows_sys holds hand-written Win32 declarations. bun_sys::windows, bun_core::windows_sys and the .bin launcher (src/install/windows-shim) re-export them.
  • mordant is the lint tool of the mordant CI job. mordant-baseline.toml holds its known findings per lint and file. The job fails only when a count rises.
  • The compiler checked each item. In a scratch tree, 41,036 workspace items got #[deprecated]. cargo check then ran on 12 targets, the debug cfgs, --release, --all-targets and the launcher binary. Each use of a marked item prints a warning. These items printed none, apart from the re-export lines that this PR removes.

Downsides

  • None found. Checked: no use on any of 12 targets in debug, release and test configurations, no reference in C, C++, generated code or build scripts, and no overlap with the deletions of the open dead-code pull requests.
Notes

Removed symbols

Each item below printed no use in any of the 17 configurations or in the launcher binary.

  • bun_windows_sys::externs::SYMBOLIC_LINK_FLAG_DIRECTORY and SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE, and their re-exports. bun_sys uses private copies in win_symlink_impl (src/sys/lib.rs). The comment there says that bun_windows_sys does not export them.
  • bun_windows_sys::externs::ProcessParameters (an alias of RTL_USER_PROCESS_PARAMETERS) and its re-export in bun_core::windows_sys. Each ProcessParameters in code is the PEB field of that name.
  • bun_sys::posix::timespec on Windows (src/sys/lib.rs). The Unix re-export of libc::timespec stays.
  • bun_libuv_sys::open_handles::count (#[cfg(debug_assertions)]). No caller.

Kept, because an open pull request touches them

The mordant baseline

  • open_handles::count was the one unused_pub finding in src/libuv_sys/open_handles.rs, so that entry goes.
  • A regenerated baseline (bun run rust:mordant:baseline) reports 53 for src/sys/lib.rs. This PR writes 55, which is the old count minus the one item removed here. It also reports lower counts for three entries that this PR does not touch. Those stay as they are on main.
  • The three bun_windows_sys items were not in the baseline. Each had a pub use re-export, and mordant counts a re-export as a use.

What the scan covered

Unused, but left in place on purpose

  • The E-prefixed aliases in src/errno/windows_errno.rs (25 are unused). The file says that they exist so the same source compiles on both targets.
  • bun_core::strings::split_once. clippy.toml names it as the replacement for str::split_once.
  • host_fn_this_value and host_fn_setter in src/jsc/host_fn.rs. src/codegen/generate-classes.ts emits calls to them for class shapes that no class uses today.
  • GetExitCodeProcess and windows_enable_stdio_inheritance in bun_windows_sys::externs. Each caller declares a local safe fn copy. Which copy to keep is a design choice.
  • The JsClass::estimated_size default and __IsFreeze::IS_FREEZE. Method resolution selects them for types that do not exist yet.
  • O::EVTONLY for macOS, posix::mode_t for Linux, bun_spawn::Result and fault::SHUTDOWN. Each is one member of a table that has an entry per platform or per crate.

Dead after an open pull request merges

Tests run with the debug build

The Rust changes are Windows-only, so the Linux binary is the same as on main.

  • test/js/node/fs/fs-mkdir.test.ts: 21 pass, 3 skip.
  • test/js/node/fs/fs-stats-truncate.test.ts: 3 pass.

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

…nJSCModule.h

No code uses these items on any target:

- bun_windows_sys::externs: MAX_PATH, SYMBOLIC_LINK_FLAG_DIRECTORY,
  SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE and the ProcessParameters
  alias, with their re-exports in bun_sys::windows and
  bun_core::windows_sys.
- bun_sys: the Windows posix::timespec struct.
- bun_libuv_sys: open_handles::count.
- BunJSCModule.h: the unused StringPrintStream local in
  functionStartSamplingProfiler.
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: Essentials

Run ID: 85d70844-019b-4dbb-bb00-041dab30a5fe

📥 Commits

Reviewing files that changed from the base of the PR and between 3c09109 and f59b7c0.

📒 Files selected for processing (3)
  • mordant-baseline.toml
  • src/sys/windows/mod.rs
  • src/windows_sys/externs.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 changes remove unused public Windows declarations and re-exports, the Windows-only posix::timespec struct, and a debug-only libuv handle-count function. The unused-public baseline is updated.

Changes

Unused public declaration removals

Layer / File(s) Summary
Remove Windows public declarations
src/windows_sys/externs.rs, src/bun_core/windows_sys.rs, src/sys/windows/mod.rs, src/sys/lib.rs, mordant-baseline.toml
Removes the symbolic-link flag constants, the ProcessParameters alias, and the Windows-only posix::timespec struct with their re-exports. The src/sys/lib.rs unused-public baseline count decreases from 56 to 55.
Remove debug handle count
src/libuv_sys/open_handles.rs, mordant-baseline.toml
Removes the debug-only count() function and its unused-public baseline entry.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to f59b7

No material in-repository behavior change or merge blocker was identified. Compatibility expectations for out-of-repository Rust consumers are not established by repository evidence.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
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 and concisely describes the primary change: removing dead code from the three affected Rust packages. It is related to the changeset, although it does not mention the additional C++ …
Description check ✅ Passed The description provides a detailed change summary, verification results, scope, rationale, and downsides. It does not use the exact template headings, but it covers both required topics: what the PR …
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding (which is pre-existing, not introduced here), I also checked the Windows-gated consumers that Linux CI does not type-check: the only bun_sys::posix::timespec user (src/io/posix_event_loop.rs:1507) is #[cfg(target_os = "macos")], the .bin launcher's pub use bun_windows_sys::* glob only touches the PEB.ProcessParameters field rather than the deleted alias, win_symlink_impl keeps its own private SYMBOLIC_LINK_FLAG_* copies, and StringPrintStream is still used later in BunJSCModule.h. The now-stale unused_pub slack in mordant-baseline.toml is harmless since the ratchet only fails when a count exceeds the baseline.

Extended reasoning...

Six-file dead-code removal (30 deletions, 2 insertions) across bun_windows_sys, bun_sys, bun_core, bun_libuv_sys and one unused C++ local; no security-sensitive surface. Every Rust hunk is Windows-gated so it is not type-checked on this Linux host, but grepping the glob importers and cfg gates found no remaining references to any removed item. Not approving because a verified finding is posted inline and further verified findings were withheld from posting.

Comment thread src/jsc/modules/BunJSCModule.h Resolved
@robobun

robobun commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

How each deletion was verified:

  • Marker pass in a scratch tree: each removed Rust item got #[deprecated]. cargo check --workspace ran on the 12 targets of bun run rust:check-all, with the debug cfgs, with --release, with --all-targets, and with -p bun_shim_impl --features shim_standalone on both Windows targets. No use of a removed item printed a warning.
  • After the deletion: bun run rust:check-all reports 12 ok. The launcher binary check, Windows --release and Windows --all-targets pass. bun bd builds.
  • rg -w for each removed name over src/, packages/, scripts/ and build/debug/codegen/.

…fixes

open_handles::count was the one finding in src/libuv_sys/open_handles.rs.
The Windows posix::timespec struct was one of the findings in
src/sys/lib.rs.

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

…e counts

- An open pull request uses bun_sys::windows::MAX_PATH, so the constant and
  its re-export stay.
- Another open pull request already removes the StringPrintStream local in
  functionStartSamplingProfiler, so this change leaves that line alone.
- mordant-baseline.toml: open_handles::count was the one unused_pub finding
  in src/libuv_sys/open_handles.rs, and the Windows posix::timespec struct
  was one of the findings in src/sys/lib.rs.

@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 — straightforward dead-code deletion, verified safe on the cfg-gated side.

What was reviewed:

  • Grepped the whole tree (including #[cfg(windows)]/debug-only code) for every removed name: no remaining references; bun_windows_sys re-exports externs::* by glob, so nothing names the deleted items individually.
  • The only bun_sys::posix::timespec user (src/io/posix_event_loop.rs:1507) is #[cfg(target_os = "macos")], so dropping the Windows struct cannot break the Windows build of that shared module.
  • win_symlink_impl's private flag copies are independent and its "not exported by bun_windows_sys" comment is now accurate; OPEN.handles/files still have readers after count() goes.
  • One note: the description still lists MAX_PATH and the pathOut local under "Fix", but f59b7c0 restored both, so the description is slightly stale relative to the final diff.
Extended reasoning...

The change deletes six unused pub Rust items across the bun_windows_sys, bun_sys, bun_core, and bun_libuv_sys leaf crates (+3/-29) and lowers two unused_pub entries in mordant-baseline.toml to match. It touches no security-sensitive surface: the removed items are Win32 constant duplicates, a type alias, a debug-only counter, and an unused struct, none of which are on an auth, crypto, or input-validation path. Every removed name was grepped repo-wide with no remaining users, the sole posix::timespec consumer is macOS-gated, and the baseline edits correspond one-to-one with the deletions, which is what decided approve. No CODEOWNERS entry covers the changed files and the timeline shows no outstanding third-party objection; the latest commit narrowed scope by restoring MAX_PATH and a C++ local, so the final diff is Rust-only.

@robobun robobun changed the title Remove dead code from bun_windows_sys, bun_sys, bun_libuv_sys, and BunJSCModule.h Remove dead code from bun_windows_sys, bun_sys, and bun_libuv_sys Oct 2, 2026
@robobun

robobun commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Two more items in the crates of this PR have no user on main (3f3a9ff). They are not in this branch.

  • UvHandle::data() in src/libuv_sys/libuv.rs (5 lines).
  • umask() in bun_sys::windows_impl, src/sys/lib.rs (6 lines). The only callers of bun_sys::umask are under #[cfg(not(windows))] in src/install/bin.rs.

Evidence: mordant at the pinned revision reports both as unused_pub on Linux, Windows and macOS. Without them, bun run rust:check-all passes on all 12 targets and bun bd builds.

History: #45698 withdrew both on 2026-10-18, because #42819 called the Windows umask and deleted src/libuv_sys. #42819 closed without a merge on 2026-11-01. Its branch claude/remove-libuv-windows-2d63e8 still exists, so I did not add the two items here. If that work does not return, they can join this PR.

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