Skip to content

Remove dead code from bun_css, bun_semver, and the re-export lists of 10 crates - #43010

Merged
alii merged 4 commits into
mainfrom
robobun/9e242044/dead-code-sweep
Sep 25, 2026
Merged

alii merged 4 commits into
mainfrom
robobun/9e242044/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Behaviour change: none

Problem

  • Six trait impls in bun_css and bun_semver have no user on any target. A --gc-sections link of the debug objects discards all their methods, and no source line needs them.
  • Ten crates re-export 24 names that no other crate imports. With pub rewritten to pub(crate), rustc reports each one as unused_imports.

Fix

  • Delete the impls: DeepClone and CssEql for the arena ArrayList and for bun_ast::Loc, the helper eql_list, PartialCmp for CSSInteger, and Slicable for ExternalString.
  • Trim the 24 names from the pub use lists. Four bun_ini structs lose their only outside path and become pub(crate). The non-unix FdT alias loses its only user and is deleted.
  • Correct because nothing instantiates the removed impls, and the workspace compiles on every target triple with the deny lints on (what ran where is in Notes).
  • No new test: no caller can observe a difference. Existing tests that cover this code: test/js/bun/css/css.test.ts, test/cli/install/semver.test.ts, test/js/bun/ini/ini.test.ts, test/cli/run/env.test.ts.

Background

  • The workspace denies dead_code, but rustc exempts each pub item that a crate root can reach. A pub impl or re-export can stay after its last user is gone.
  • DeepClone, CssEql, and PartialCmp are the CSS value protocol traits in src/css/generics.rs. Generated and hand-written CSS types call them through trait bounds.
  • Slicable lets Lockfile::str read a semver string out of the lockfile string buffer. Only semver::String is passed to it.
Notes

Removed items:

  • src/css/generics.rs: impl DeepClone for ArrayList<'bump, T>, impl DeepClone for bun_ast::Loc, fn eql_list, impl CssEql for ArrayList<'bump, T>, impl CssEql for bun_ast::Loc, impl PartialCmp for CSSInteger. rg -w eql_list src has no other hit.
  • src/semver/lib.rs: impl Slicable for ExternalString.
  • src/dotenv/lib.rs: re-export DirEntryProbe.
  • src/ini/lib.rs: re-exports ConfigIterator, ScopeItem, ScopeIterator, ToStringFormatter. The four structs become pub(crate) because unreachable_pub is denied. All four are still used inside the crate.
  • src/install/isolated_install/Store.rs: re-export StoreKeyFormatter.
  • src/js_parser_jsc/lib.rs: re-export toml_datetime_to_js.
  • src/paths/lib.rs: re-exports PlatformT, RelPathFacts, windows_volume_name_len, EnvPathInput, PathComponentBuilder.
  • src/spawn_sys/lib.rs: re-exports FdT, IoCounters, WinRusage, WinTimeval.
  • src/spawn_sys/spawn_process.rs: the alias #[cfg(not(unix))] pub type FdT = i32;. Every use of FdT is in #[cfg(unix)] code, so the re-export was the only thing that named the non-unix alias. The mordant job found this (unused_pub, 1 finding over a baseline of 0). The comment on the bun_windows_sys dependency in src/spawn_sys/Cargo.toml named the removed re-export, so it now names the alias in spawn_process.rs.
  • src/tcc_sys/lib.rs: re-exports ErrorFunc, TCCErrorFunc, TCCState.
  • src/threading/lib.rs: re-export GuardedLock.
  • src/uws_sys/lib.rs: re-export PosixLoop.
  • src/watcher/lib.rs: re-exports MAX_EVICTION_COUNT, WatchItem, WatchItemIndex.

For each re-exported name, rg -w <name> finds it in no other crate, in no generated Rust under the codegen directory, and in no path of the form crate::<name> or <crate>::<name>. Except for the non-unix FdT alias, the items themselves stay, because each one is still used inside its own crate.

How the candidates were found:

  1. The debug objects were linked again with --gc-sections --print-gc-sections. 615 Rust functions, methods, and trait impls outside the files of the open dead-code PRs have no surviving symbol.
  2. All 615 were deleted at once. cargo check ran in a loop for linux-gnu, linux-gnu --tests, windows-msvc, apple-darwin, freebsd, android, and musl. Each item that an error named was restored. 12 items survived. Most of the others are Windows-only or macOS-only code.
  3. Each crate was checked once with pub rewritten to pub(crate). This shows the re-exports and items that are unused inside their own crate. Names that appear in any other crate were kept.

Candidates that compiled without them but are not dead, and so are not in this PR:

  • ArrayBufferSink::flush, FetchRequestBodySink::flush, BorderImageSideWidth::deep_clone. A macro-generated trait method forwards to each inherent method (Self::flush(self), <$t>::deep_clone(self, bump)). Without the inherent method the trait method calls itself. The unconditional_recursion lint catches it.
  • bun_zstd::inflate_embedded and inflate_embedded_nul. embed_compressed! calls them only under cfg(bun_codegen_embed), which only release builds set.

A removed impl is not always a compile error. A call like a.eql(&b) on two ArrayList values falls back to impl CssEql for [T] through auto-deref. That impl compares the length and then each element, the same as the removed eql_list. On the base of the first commit, no symbol of a removed impl survived the link, so no reachable code called one.

Overlap with other open dead-code PRs: #43976 also edits src/uws_sys/lib.rs, in other hunks. No other file in this PR is touched by one.

What ran on which tree:

  • First commit (base 2dee3bd): bun bd, bun run rust:check-all (12 of 12), and these test files with the debug build: test/js/bun/css/css.test.ts, test/cli/install/semver.test.ts, test/js/bun/ini/ini.test.ts, test/cli/run/env.test.ts, test/internal/source-lints/ (174 tests), test/js/bun/resolve/toml/toml.test.js, test/js/node/path/resolve.test.js, test/js/bun/ffi/cc.test.ts, test/cli/install/bun-lock.test.ts.
  • After the merge of main (f063852) plus the FdT commit: cargo check --workspace for the host, and bun run rust:check-all for both Windows triples (2 of 2). The FdT commit changes only code that is compiled out on unix, and CI built the merge commit on every platform (build 120695). The debug build and the test files were not run again locally.

[policy-decision:dep] gate passed · iteration 6 · 14 files touched

passes on PR (with fix)
Dependency/toolchain change; no test proof to run.
diff hotspot
src/css/generics.rs                   | 52 -----------------------------------
 src/dotenv/lib.rs                     |  4 +--
 src/ini/lib.rs                        | 13 ++++-----
 src/install/isolated_install/Store.rs |  2 +-
 src/js_parser_jsc/lib.rs              |  3 +-
 src/paths/lib.rs                      |  7 ++---
 src/semver/lib.rs                     |  6 ----
 src/spawn_sys/Cargo.toml              |  2 +-
 src/spawn_sys/lib.rs                  |  4 +--
 src/spawn_sys/spawn_process.rs        |  2 --
 src/tcc_sys/lib.rs                    |  4 +--
 src/threading/lib.rs                  |  2 +-
 src/uws_sys/lib.rs                    |  2 +-
 src/watcher/lib.rs                    |  6 ++--
 14 files changed, 21 insertions(+), 88 deletions(-)

gate history · 1 passed · 3 rejected · iteration 6

evidence per changed file
file                                   reads  edits  tests
src/css/generics.rs                        0      0      1
src/dotenv/lib.rs                          0      0      1
src/ini/lib.rs                             0      0      1
src/install/isolated_install/Store.rs      0      0      1
src/js_parser_jsc/lib.rs                   0      0      1
src/paths/lib.rs                           0      0      1
src/semver/lib.rs                          0      0      1
src/spawn_sys/Cargo.toml                   1      2      0
src/spawn_sys/lib.rs                       0      0      2
src/spawn_sys/spawn_process.rs             1      1      2
src/tcc_sys/lib.rs                         0      0      1
src/threading/lib.rs                       0      0      1
src/uws_sys/lib.rs                         0      0      2
src/watcher/lib.rs                         0      0      1

… 10 crates

Delete six trait impls that no target uses: DeepClone and CssEql for the
arena ArrayList and for bun_ast::Loc (plus the eql_list helper),
PartialCmp for CSSInteger, and Slicable for ExternalString.

Trim 24 names that no other crate imports from the pub use lists of
bun_dotenv, bun_ini, bun_install, bun_js_parser_jsc, bun_paths,
bun_spawn_sys, bun_tcc_sys, bun_threading, bun_uws_sys and bun_watcher.
Four bun_ini structs lose their only outside path and become pub(crate).
@robobun

robobun commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • Head ae7797c is the merge of main plus one commit. That commit deletes the non-unix FdT alias, which the mordant job flagged after the FdT re-export was removed.
  • Scope: 14 files, 88 lines removed, 21 added. No behaviour changes: the diff only deletes unused trait impls, unused re-exports, and one unused type alias.
  • Checked locally on this head: cargo check --workspace for the host and for both Windows triples, with the deny lints on. The first commit was also checked on all 12 target triples and with the debug build and the test files named in the PR body.
  • CI built the merge commit 39b8b5e on every platform (build #120695). No job of that build had failed when ae7797c was pushed.
  • Remove dead code from the bake client, uSockets, uWS HTTP/2, uws_sys, and the CI pipeline script #43976 also edits src/uws_sys/lib.rs, in other hunks. No other file here is touched by an open dead-code PR.

@coderabbitai

coderabbitai Bot commented Sep 17, 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: 8ddf3391-07af-4bd9-a9ad-74019b9335d9

📥 Commits

Reviewing files that changed from the base of the PR and between 39b8b5e and ae7797c.

📒 Files selected for processing (2)
  • src/spawn_sys/Cargo.toml
  • src/spawn_sys/spawn_process.rs
💤 Files with no reviewable changes (1)
  • src/spawn_sys/spawn_process.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


Walkthrough

The pull request narrows public API surfaces across multiple crates. It removes selected CSS trait implementations, restricts four INI types to crate visibility, and removes selected crate-root re-exports. It also removes a non-Unix FdT alias and rewords a dependency comment.

Changes

API surface cleanup

Layer / File(s) Summary
CSS trait implementation cleanup
src/css/generics.rs
Removes selected DeepClone, CssEql, and PartialCmp implementations and the private eql_list helper.
INI visibility reduction
src/ini/lib.rs
Removes four public re-exports and changes the corresponding types to pub(crate).
Crate-root re-export reduction
src/dotenv/lib.rs, src/install/isolated_install/Store.rs, src/js_parser_jsc/lib.rs, src/paths/lib.rs, src/semver/lib.rs, src/spawn_sys/*, src/tcc_sys/lib.rs, src/threading/lib.rs, src/uws_sys/lib.rs, src/watcher/lib.rs
Removes selected public re-exports. In spawn_sys, the non-Unix FdT alias is removed, and a dependency comment is reworded.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to ae779

The changes narrow Rust API surfaces as intended, with no production behavior change apparent. No concrete merge-blocking risk is established.

🚥 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.
Description check ✅ Passed The description clearly explains what the PR changes and provides detailed verification notes, including checks, tests, target triples, and known limitations.
Title check ✅ Passed The title is concise, specific, and accurately summarizes the dead-code removal and re-export cleanup.

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.

6 verified lower-impact observations (convention, logging or cleanup points) were not posted.

@robobun

robobun commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:03 PM PT - Sep 25th, 2026

✅ @robobun, your commit ae7797c39cb9dff564724852428e34f8c9bfe95d passed in Build #120698! 🎉


🧪   To try this PR locally:

bunx bun-pr 43010

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

bun-43010 --bun

Jarred-Sumner pushed a commit that referenced this pull request Sep 21, 2026
### Problem
- 49 hand-written trait impls across 12 crates have no user: no call,
and no bound that needs them.
- rustc's `dead_code` lint exempts trait impls, so the workspace's deny
lints never report them.

### Fix
- Delete the 49 impls and `hash_array_list` (`src/css/generics.rs`),
whose only caller was a removed impl. 25 files, 436 lines removed.
- Candidates come from the linker: in a relink of the debug build with
`--gc-sections`, an impl block with no kept function address is a
candidate. The compiler decides next: an impl that a bound, a
supertrait, another target, or a unit test needs does not compile away,
so it stays. That kept 128 of 233 candidates.
- 56 more stayed by choice: open PRs use them, docs describe them, or
they are std ergonomics impls on shared containers. See the Notes.
- Verified: `bun run rust:check-all` (12 targets), `cargo check
--release` (3 targets), `cargo check --tests`, `cargo clippy`, `bun bd`,
11 test files, and a scan of all open PRs for users.

### Background
- To rustc a trait impl is always reachable: generic code could call it.
Only a whole-program view shows that nothing does.
- The debug build uses opt-level 0 and one section per function, so
`--gc-sections` drops every function with no caller. Callers in generic
instantiations and macro-expanded code count.
- That link proves linux only. `cargo check` covers the other targets.
- #39795 removed 92 impls this way.

<details><summary>Notes</summary>

**Removed impls, by file**

- `src/ast/e.rs`
  - `impl From<f64> for Number`
- `src/bun_alloc/baby_vec.rs`
  - `impl<'a, 'b, T: Copy> Extend<&'b T> for BabyVec<'a, T>`
  - `impl<'a, 'b, T> IntoIterator for &'b BabyVec<'a, T>`
  - `impl<'a, 'b, T> IntoIterator for &'b mut BabyVec<'a, T>`
  - `impl<'a, T> core::borrow::Borrow<[T]> for BabyVec<'a, T>`
  - `impl<'a, T> AsRef<[T]> for BabyVec<'a, T>`
- `src/bun_alloc/lib.rs`
  - `impl<const N: usize> BSSAppendable for [&[u8]; N]`
- `src/bun_core/atomic_cell.rs`
  - `unsafe impl<U> Atom for *const U`
- `src/bun_core/fmt.rs`
  - `impl From<InvalidCharacter> for crate::CrateError`
- `src/bun_core/output.rs`
  - `impl<T: fmt::Display> FmtTuple for &[T]`
- `src/bundler/Chunk.rs`
  - `impl core::ops::Index<usize> for CompileResultSlots`
- `src/bundler/bundle_v2.rs`
  - `impl Ord for StableRef`
  - `impl PartialOrd for StableRef`
- `src/collections/array_hash_map.rs`
- `impl<K, V, C, A: MapAllocator> ArrayHashMapExt for ArrayHashMap<K, V,
C, A>`
- `src/css/generics.rs`
  - `impl<'bump, T: DeepClone<'bump>> DeepClone<'bump> for &'bump T`
  - `impl<T: CssEql, const N: usize> CssEql for [T; N]`
  - `impl CssHash for ()`
  - `impl<T: CssHash, const N: usize> CssHash for [T; N]`
  - `impl<'bump, T: CssHash> CssHash for ArrayList<'bump, T>`
  - `impl CssHash for bun_ast::Loc`
  - `impl<T: IsCompatible, const N: usize> IsCompatible for [T; N]`
  - `impl<'bump, T: ToCss> ToCss for ArrayList<'bump, T>`
  - `impl ToCss for CustomIdent`
  - `impl ToCss for DashedIdent`
  - `impl ToCss for Ident`
- `src/css/media_query.rs`
  - `impl crate::generic::ToCss for MediaList`
- `src/css/properties/mod.rs`
- `impl crate::generics::ParseWithOptions for
css_values::length::Length`
- `impl<S, const P: u8> crate::generics::Parse for GenericBorder<S, P>`
- `src/css/rules/supports.rs`
  - `impl crate::generics::CssEql for SupportsCondition`
  - `impl css::generic::ToCss for SupportsCondition`
- `src/css/selectors/parser.rs`
  - `impl<Impl: SelectorImpl> Default for GenericSelector<Impl>`
  - `impl<Impl: BunSelectorImpl> CssEql for GenericSelectorList<Impl>`
  - `impl<Impl: BunSelectorImpl> CssHash for GenericSelectorList<Impl>`
  - `impl<Impl: BunSelectorImpl> CssEql for GenericComponent<Impl>`
  - `impl<Impl: BunSelectorImpl> CssHash for GenericComponent<Impl>`
- `src/css/values/alpha.rs`
  - `impl crate::generics::CssHash for AlphaValue`
- `src/install/lockfile/lockfile_json_stringify_for_debugging.rs`
  - `impl<const N: usize> JsonScalar for &[u8; N]`
- `src/install/resolution.rs`
  - `impl Default for Tag`
- `src/install_types/resolver_hooks.rs`
  - `impl<I: VersionInt> Default for ResolutionValue<I>`
  - `impl Default for Resolution`
- `src/jsc/JSPropertyIterator.rs`
  - `impl IntoIterObject for *const JSObject`
- `src/jsc/JSValue.rs`
  - `impl FromAny for ()`
  - `impl FromAny for &str`
  - `impl<T: FromAny> FromAny for Option<T>`
- `src/jsc/host_fn.rs`
  - `impl<T> IntoHostConstructReturn for *mut T`
- `src/react_compiler/hir/mod.rs`
  - `impl From<FloatValue> for f64`
- `src/runtime/bake/DevServer.rs`
  - `impl From<OpaqueFileId> for OpaqueFileIdOrOptional`
- `src/runtime/error.rs`
  - `impl From<Error> for bun_jsc::CrateError`
- `src/tcc_sys/tcc.rs`
  - `impl<ErrCtx> Default for Config<ErrCtx>`

**Candidates that stayed (184 of 233)**
- 128: the workspace does not compile without the impl (a bound, a
supertrait, another target, or a unit test needs it).
- 8: open PRs use them. The six `SmallList` impls (`Deref`, `DerefMut`,
`IntoIterator` x2, `FromIterator`, `Extend`) for #39497, #38692 and
#36605. `Default for Wyhash` for #40372. `AsRef<[u8]> for Utf8Bytes` for
#40708. This branch merged with #39497, #38692 and #40708 passes `cargo
check --workspace`. #36605 and #40372 conflict with main on their own.
- 19: std ergonomics impls on widely shared types. This is a design
call, so it needs a maintainer decision and its own PR: `Interned`
(`Deref`, `AsRef<[u8]>`, `Borrow<[u8]>`), `ZStr` (`AsRef<ZStr>`,
`PartialEq<[u8]>`, `PartialEq<&[u8; N]>`), `StoreStr`
(`PartialEq<[u8]>`), `StoreSlice` (`AsRef<[T]>`, `From<&[T]>`,
`From<ArenaVec>`), `IntoIterator for &HashMap` and `&mut HashMap`,
`BoundedArrayAligned` (`Deref`, `DerefMut`, `fmt::Write`), `AsRef<[T]>
for RawSlice`, `Deref for ArenaString` (also the only caller of
`ArenaString::as_str`), `Display for GenericIndex`, `Display for
CowSliceZ`.
- 21: docs or comments describe the impl as part of a design.
`ArrayHashContext<Box<[u8], A>> for BoxedSliceContext<C>` (its doc
comment records it as the fix for lookups reached through `Deref`),
`Default for Ordinal` (the default is `INVALID`, not 0), `IntoLogWrite
for &mut W`, `ThrowFmtArgs for fmt::Arguments`, `PoolStorage for
UnwiredStorage`, `StringBuilder for semver_string::Builder`, `fmt::Write
for FmtAdapter`, `AsFd for &File`, and the css blanket impls that the
`css_derive` module docs and the `Parse` section of
`src/css/generics.rs` name (`Option<T>`, `SmallList`, `bool`, `str`,
`[u8]`, `[T]`, `Size2D`, `Rect`).
- 6: next to a hunk of an open dead-code PR (#40690, #41335, #43010,
#42278), left alone to avoid a conflict.
- 2: the removal leaves something else half-used. See the follow-up
list.

**Method details**
- Link: the debug link command from `build.ninja` plus
`-Wl,--gc-sections`. `-rdynamic`, `--dynamic-list` and the version
script stay, so the exported NAPI/V8/uv surface is still a root.
- Liveness: `llvm-nm` lists the 669,524 kept function symbols and
`llvm-symbolizer` maps each address to `file:line`. An impl block is a
candidate when no kept address falls inside it and a rust-analyzer SCIP
index shows that linux compiles every method in it.
- Limit of the finder: it reads impl blocks from source text. Impls that
a `macro_rules!` type list generates are not candidates, so unused arms
of such lists are not in this PR.
- Restore loop: delete all candidates, run `cargo check --workspace
--keep-going --message-format=json`, put back each impl that an error
names, repeat until clean. Targets in the loop: linux-gnu, windows-msvc,
darwin, freebsd, android, musl, then `--tests`.
- Method resolution: on linux the link proves that no call to a removed
impl remains, so no call can resolve differently. On other targets a
call that used a removed impl fails to compile.
- Open PRs: main has no merge queue, so a green PR that uses a removed
impl would break main when it merges. I fetched the heads of all 5474
open non-draft PRs, took the Rust lines each one adds against its merge
base, and searched them for the types and traits of the 49 impls. No PR
uses one.
- Base: main at 18fe86e. The liveness data is from cf71211. The
commits between them do not touch the 25 files, and every check above
ran again on 18fe86e.
- `cargo check --workspace --tests` reports one error,
`src/bundler/options.rs:2328` (`expected ContentHash, found integer`).
Clean main has the same error. This PR does not touch it.
- Three tests fail in the local debug build with and without this
change: `ffi.test.js` "FTL-compiled call site" (5 s timeout under ASAN),
and two `serve.test.ts` tests that depend on the container (root can
bind low ports, loopback check).

**Other scans in this run**
- Free functions, inherent methods, types, consts and statics: a name
scan, the linker scan, and a rust-analyzer zero-reference scan for
linux, windows and darwin. Every hit was platform code with a caller on
another target, test-only code, or a deletion that an open PR already
carries.
- Also clean or already claimed: `.rs` files outside every mod tree,
unused Cargo dependencies, extern declarations without a Rust caller,
commented-out code in C++/TS, unused `src/js` modules and internal
exports, linker-dropped C++ functions in `src/jsc/bindings`.

**Follow-up candidates, not in this diff**
- 25 hand-written `Debug` impls that nothing formats. They are a
debugging aid, so this PR keeps them.
- `Clone for JsPoster` has no caller, and it is the only reader of the
`JsPosterVTable::clone` slot. Removing both is a small refactor in
`src/event_loop` and `src/jsc/VmHandle.rs`.
- `Display for ErrorLocation` (`src/css/error.rs`) has no caller, and it
is the only reader of `ErrorLocation::filename`.
- The 19 std ergonomics impls above, if a maintainer wants shared
containers to go through inherent methods only.
- `Bun__napi_get_version` in `src/jsc/bindings/napi.cpp` has no caller.
- The class generator emits `<Type>Class__call` shims for classes that
set `call: true` and `noConstructor: true` (the seven `Expect*` matchers
in `jest.classes.ts`). Nothing references those shims.

</details>
@robobun

robobun commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator Author

@alii Head 39b8b5e made the mordant job fail, and the cause was this diff. Commit ae7797c fixes it. Please merge only a head that includes ae7797c.

  • With the FdT re-export gone from src/spawn_sys/lib.rs, the #[cfg(not(unix))] pub type FdT = i32; alias in src/spawn_sys/spawn_process.rs had no user. Every use of FdT is in #[cfg(unix)] code.
  • mordant-baseline.toml allows 0 unused_pub findings in that file, so the job reported 1 finding over the baseline.
  • ae7797c deletes the alias. Checked locally before the push: cargo check --workspace for the host and for both Windows triples, with the deny lints on. The alias is compiled only on non-unix targets.

The push restarted CI.

Every use of FdT is in cfg(unix) code. The crate-root re-export was the
only thing that named the non-unix alias, and that re-export is gone.

The comment on the bun_windows_sys dependency named the removed
IoCounters re-export. It now names the alias in spawn_process.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 — mechanical dead-code sweep; the new FdT commit checks out too.

What was reviewed:

  • The removed #[cfg(not(unix))] FdT alias: every remaining FdT use in src/spawn_sys/spawn_process.rs is inside the #[cfg(unix)] spawn function (line 632 onward) or the linux/android-gated PidFdType, so nothing names it on Windows.
  • Each trimmed re-export, grepped across all .rs files including cfg-gated code: out-of-crate hits are doc comments only; in-crate uses go through crate::/still-pub module paths (bun_threading::guarded, bun_paths::resolve_path).
  • The bun_ini structs narrowed to pub(crate) are only constructed inline in-crate and appear in no pub signature; the removed Slicable for ExternalString has no caller passing an ExternalString to Lockfile::str/str_detached.
Extended reasoning...

The diff (+21/-88 across 14 Rust files) deletes six unused trait impls in bun_css/bun_semver, trims 24 names from crate-root pub use lists in ten crates, narrows four bun_ini structs to pub(crate), drops the non-unix FdT alias, and rewords one Cargo.toml comment; it touches no security-sensitive surface and no behavior. Since the prior review, the only substantive change is the FdT alias removal plus a merge of main, and I verified by reading the cfg gates that the alias has no non-unix consumer. No changed path is covered by CODEOWNERS, the hunt ran dry, and grep of the whole tree (including #[cfg(windows)] code) found no consumer of any removed name, which decided approve; the one limitation is that cargo could not be run in this sandbox, so the cross-target check rests on grep plus the author's reported rust:check-all runs.

@alii
alii merged commit 601af5a into main Sep 25, 2026
11 checks passed
@alii
alii deleted the robobun/9e242044/dead-code-sweep branch September 25, 2026 22:18
Jarred-Sumner pushed a commit that referenced this pull request Sep 26, 2026
… and the CI pipeline script (#43976)

Behaviour change: none

### Problem
- Eleven files hold items that nothing reads or calls: write-only
fields, an uncalled method, four unused types, and CI options that are
parsed and dropped.
- No tool reports them. C and C++ have no dead-code lint. TypeScript
counts `x += n` as a read. The Rust types come from a macro.

### Fix
- TypeScript: remove `DataViewReader.u16()`, `DataViewWriter.capacity`,
the `totalCount` local in `updateBuildErrorOverlay`, and a commented-out
block from 2024 in `src/js/node/dgram.ts`.
- C and C++: remove `us_udp_socket_t.connected`,
`us_quic_stream_s.headers_delivered` and `Http2ResponseData::totalSize`
(each is only written), and `#undef FD_BITS` (nothing defines it).
- Rust and CI: remove the opaque types `us_loop_t`,
`us_socket_context_t`, `us_udp_socket_t`, `us_udp_packet_buffer_t` from
`src/uws_sys/lib.rs`. In `.buildkite/ci.ts`, remove `dryRun`,
`Platform.features`, four emoji entries, and the union members
`"amazonlinux"` and `"eol"`.
- Verified: `rg -w` for each symbol over `src`, `packages`, `scripts`,
`test` and `build/debug/codegen` finds no other use. `bun bd`, `bun run
rust:check-all` (12 targets) and `tsc` pass. Self-reviewed: 1 concern
raised, 1 addressed.

### Background
- `DataViewReader` and `DataViewWriter` decode and encode the binary
messages between the dev server and its browser client.
- `us_udp_socket_t` and `us_quic_stream_s` are private C structs of
uSockets. Rust holds them as opaque pointers, so no Rust struct mirrors
their layout.
- `bun_core::opaque_extern!` declares a zero-sized Rust type for a C
struct. Rust code names `Loop`, `udp::Socket` and `udp::PacketBuffer`,
not the four removed types.

### Downsides
- None found. Checked each removed symbol for users in Rust, C, C++,
TypeScript, generated code, tests, and open pull requests.

<details><summary>Notes</summary>

**Evidence per removal**

| Item | Evidence |
| --- | --- |
| `DataViewReader.u16()` | `rg '\.u16\('` over `src/runtime/bake`,
`test/bake`, `test/cli/inspect`: no hit. |
| `DataViewWriter.capacity` | The only hit of `.capacity` in the bake
TypeScript is the assignment in the constructor. `initCapacity` is the
only caller of the constructor. |
| `totalCount` | Two hits: the declaration and one `+=`. |
| `dgram.ts` block | `git blame`: 589f941, 2024-04-26.
`replaceHandle` and `startListening` are not defined in the file. |
| `us_udp_socket_t.connected` | Two hits, both `udp->connected = 0;`. |
| `us_quic_stream_s.headers_delivered` | Two hits in `quic.c`: the field
and one `= 1`. The struct is private to `quic.c`. |
| `Http2ResponseData::totalSize` | One member access: `data.totalSize =
totalSize;`. The other hits of `totalSize` are the function parameter. |
| `#undef FD_BITS` | The only hit of `FD_BITS` in `src` and `packages`.
|
| Four opaque types | Each name has one non-comment hit in all Rust
sources and generated Rust: the macro call. |
| `dryRun` | Four hits in `ci.ts`: the field, two assignments, one
destructure. Nothing reads the binding. |
| `Platform.features` | One hit. |
| Emoji, `Distro`, `Tier` entries | No platform in `ci.ts` or image in
`scripts/build/ci-images/spec.ts` carries them. `Emoji` is `keyof typeof
emojiMap`, so a remaining caller would fail `tsc -p
scripts/tsconfig.json`. It passes. |

**Taken out because an open pull request uses or removes the item**

- `DataViewWriter.u8()`: dead on main, but #42075 adds its first caller
(`check.u8(IncomingMessageId.check_errors)` in `hmr-runtime-error.ts`).
Git merges the two without a conflict, so the method stays. The tree
that results from a merge of this branch with #42075 has no type error
for `u8`.
- `declare module "bun:wrap"` in `bake.private.d.ts`: no importer, but
#39488 already has the same hunk.

**Tests run with the debug build, all pass**

`test/js/bun/udp/udp_socket.test.ts` (218),
`test/js/bun/udp/dgram.test.ts` (62),
`test/js/bun/http/serve-http2.test.ts` (93),
`test/js/bun/http/serve-http3.test.ts` (73),
`test/bake/dev/bundle.test.ts` (23), `test/bake/dev/esm.test.ts` (17),
`test/bake/hmr-socket-protocol.test.ts` (4),
`test/cli/inspect/BunFrontendDevServer.test.ts` (7).
`test/js/node/dgram/node-dgram.test.js` passes 3 of 4: the IPv6
multicast test fails with `ENODEV` in the test container, with and
without this change. `prettier` and `cargo fmt --check` report no
change.

**Overlap with open pull requests**

Each removed line was compared with the diffs of the 35 open dead-code
pull requests. None removes the same lines. Five files are also touched
by an open pull request, in hunks more than 6 lines away: `internal.h`
and `quic.c` (#40294, #42431), `dgram.ts` (#42431), `overlay.ts`
(#43378, #42075, #39488), `src/uws_sys/lib.rs` (#43010). The added lines
of the 122 open pull requests that were updated since 2026-09-18 and
touch the bake, uSockets, uWS, uws_sys, server, socket or CI sources
name none of the removed symbols.

**What was scanned**

- C and C++: the debug binary was linked a second time with
`--gc-sections`, and the two symbol tables were compared. 414 functions
in bun's own C and C++ are unreachable on Linux. Open pull requests
remove 263 of them. The remainder is in the list below, has a caller on
Windows or macOS, or comes from a macro.
- Rust: 37 `#[no_mangle]` exports are unreachable in the Linux link.
Each has a caller on another platform, or #40824, #40232 or #40557
removes it. A count of references for all 58,055 Rust definitions found
no other item without a user. Of the 144 `allow` attributes for the
unused and unreachable lints, each covers code that depends on `cfg` or
is macro output.
- Cargo: five dependency edges are unused on all 12 targets. #40294
removes three. `bun_resolver -> bun_zstd` is used under
`cfg(bun_codegen_embed)`. `bun_wyhash -> bstr` is used by unit tests.
- Preprocessor: `USE(BIGINT32)` and `ENABLE(MALLOC_BREAKDOWN)` are never
true. #43644 and #40557 remove those branches.
- Also scanned and clean: `src/js`, `src/node-fallbacks`, `src/codegen`,
`scripts/`, `misctools/`, `patches/` (every patch file has a user),
`packages/` except `bun-types`.

**Probably dead, left alone on purpose**

- The `PerformanceResourceTiming` cluster under
`src/jsc/bindings/webcore` (about 2,000 lines:
`PerformanceResourceTiming`, `PerformanceServerTiming`,
`ResourceTiming`, `NetworkLoadMetrics`, `ResourceLoadTiming`,
`ServerTiming` and the two JS wrappers). The linker drops every
constructor, so no instance can exist. The two globals are public and
`test/js/web/web-globals.test.js` checks them. This needs a decision:
keep it for a future resource-timing implementation, or reduce it to the
two constructors.
- `WEBCORE_GENERATED_CONSTRUCTOR_GETTER` (`ZigGlobalObject.cpp`) emits
an `X_getter` function for 50 classes. 45 have no user. A removal needs
a second macro and saves no source lines.
- The WebIDL converters for `byte`, `short` and `long long`, and most
`Clamp` and `EnforceRange` specializations in `JSDOMConvertNumbers.cpp`.
No binding uses them, but `src/codegen/bindgen.ts` maps `t.i8`, `t.i16`
and `t.i64` to them.
- `src/js/bun/sql.ts`: the export properties `sql`, `Query`, `postgres`
and the four error classes. Native code reads only `default` and `SQL`.
It is not certain that no loader path exposes the module object.
- `us_nq_settings_set_scid_len` and `us_nq_settings_set_delay_onclose`
(`node_quic_shim.c`, declared in `src/lsquic_sys/lib.rs`): no caller.
`node:quic` is under active work.
- `Event::currentTargetIsInShadowTree()` and its bit: no reader. The
lines sit next to a hunk of #39929.
- Bake client: `WebSocketWrapper.close()` and `[Symbol.dispose]()`,
`streamingStarted`, the `line` and `column` bookkeeping and seven enum
members in `JavaScriptSyntaxHighlighter.ts`, and `externals` in
`src/node-fallbacks/build-fallbacks.ts`. Each sits next to a hunk of
#43378, #40492, #40122 or #41385.
- The `internal: true` property option of the class generator. A guard
throws on it, so the branches behind it cannot run. Five open pull
requests touch `generate-classes.ts`.
- `H2App::getNativeHandle` (next to a hunk of #41195) and
`uws_app_listen_config_t` (its last user goes with #42431).
- `UWS_ALLOW_SHARED_AND_DEDICATED_COMPRESSOR_MIX`,
`UWS_ALLOW_8_WINDOW_BITS` and `LIBUS_NO_SSL`: never defined, but they
are documented opt-in switches of the upstream libraries.
- `scripts/debug-coredump.ts`, `scripts/gamble.ts`,
`scripts/github-metrics.ts`, `scripts/lldb-inline.sh` with
`scripts/lldb-inline-tool.cpp`, and `packages/h3blast`: nothing
references them. They read as tools that a person runs by hand.
- #40232 removes `napi_internal_get_version`. #42556 renamed that
function to `Bun__napi_get_version` on main, and it still has no caller.

</details>
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.

3 participants