Remove dead code from the bake client, uSockets, uWS HTTP/2, uws_sys, and the CI pipeline script - #43976
Merged
Conversation
… and the CI pipeline script - bake client: remove `DataViewReader.u16()`, the write-only `DataViewWriter.capacity` field with its constructor parameter, and the write-only `totalCount` local in `updateBuildErrorOverlay`. - node:dgram: remove a commented-out block from 2024 that names two functions which do not exist. - uSockets: remove the write-only `us_udp_socket_t.connected` bit, the write-only `us_quic_stream_s.headers_delivered` field, and an `#undef` of a macro that is never defined. - uWS HTTP/2: remove the write-only `Http2ResponseData::totalSize` field. - uws_sys: remove four opaque types that no Rust code names. - .buildkite/ci.ts: remove the `dryRun` option (parsed, never read), `Platform.features`, and the emoji, distro and tier entries that no platform produces.
Collaborator
Author
|
Updated 5:34 AM PT - Sep 25th, 2026
✅ @robobun, your commit 8c2a4372bfc1b5c9d0e9a2c9bb6220c685ab0ede passed in 🧪 To try this PR locally: bunx bun-pr 43976That installs a local version of the PR into your bun-43976 --bun |
Collaborator
Author
|
Status: ready for review.
|
Contributor
|
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 configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. WalkthroughThe PR removes obsolete CI options and type members, native socket state, HTTP/2 response state, runtime data-view APIs, and unused runtime code. QUIC header callbacks are no longer limited by a stored delivery flag. ChangesObsolete state and option cleanup
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The reviewed cleanup preserves the examined request, response, and bake-client behavior. No material current-head issue is identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
Collaborator
Author
|
The review reports no actionable comments, so the PR stays as it is. One line of the walkthrough needs a correction, because it reads as a behavior change. The walkthrough says that QUIC header callbacks are no longer limited by a stored delivery flag. No flag limited them before this change:
To check: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
LGTM — pure dead-code deletions, each removed item verified to have no remaining readers.
What was reviewed:
- Grepped all Rust sources and src/codegen for the four opaque types dropped from
opaque_extern!— only doc comments name them;udp.rsdeclares its own opaqueSocket/PacketBuffer, so no Rust type mirrorsus_udp_socket_tlayout and theconnectedbitfield removal cannot shift anything Rust reads. - Checked C/C++ callers:
FD_BITSis defined nowhere in the repo,headers_deliveredhad one write and no read inquic.c, andHttp2ResponseData::totalSizehad nodata.totalSizereads (all remainingtotalSizehits are theinternalEndparameter). - Confirmed
DataViewWriteris constructed only viainitCapacity,.u16()has no callers in src/ or test/, anddryRun/Platform.features/removed emoji keys have no remaining references in.buildkiteorscripts/.
Extended reasoning...
The change is a single commit (+6/-43) that only deletes write-only fields, an uncalled method, unused type aliases/union members, and a never-read CI option across the bake client, uSockets, uWS HTTP/2, uws_sys, and .buildkite/ci.ts. It touches no security-sensitive surface: no auth, crypto, parsing, or input-validation paths change, and the C struct field removals affect only structs that are opaque to Rust and private to their C translation units. None of the changed files is covered by CODEOWNERS, and the bug hunt ran dry with five candidates ruled out. Static greps independently confirmed every removed symbol has zero remaining readers, which is what decided approve.
alii
added a commit
that referenced
this pull request
Sep 25, 2026
… 10 crates (#43010) 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. <details><summary>Notes</summary> 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. </details> <!-- robobun:evidence:begin --> --- **[policy-decision:dep]** gate passed · iteration 6 · 14 files touched <details><summary>passes on PR (with fix)</summary> ```console Dependency/toolchain change; no test proof to run. ``` </details> <details><summary>diff hotspot</summary> ``` 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(-) ``` </details> **gate history** · 1 passed · 3 rejected · iteration 6 <details><summary>evidence per changed file</summary> ``` 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 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: Alistair Smith <hi@alistair.sh>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Behaviour change: none
Problem
x += nas a read. The Rust types come from a macro.Fix
DataViewReader.u16(),DataViewWriter.capacity, thetotalCountlocal inupdateBuildErrorOverlay, and a commented-out block from 2024 insrc/js/node/dgram.ts.us_udp_socket_t.connected,us_quic_stream_s.headers_deliveredandHttp2ResponseData::totalSize(each is only written), and#undef FD_BITS(nothing defines it).us_loop_t,us_socket_context_t,us_udp_socket_t,us_udp_packet_buffer_tfromsrc/uws_sys/lib.rs. In.buildkite/ci.ts, removedryRun,Platform.features, four emoji entries, and the union members"amazonlinux"and"eol".rg -wfor each symbol oversrc,packages,scripts,testandbuild/debug/codegenfinds no other use.bun bd,bun run rust:check-all(12 targets) andtscpass. Self-reviewed: 1 concern raised, 1 addressed.Background
DataViewReaderandDataViewWriterdecode and encode the binary messages between the dev server and its browser client.us_udp_socket_tandus_quic_stream_sare 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 namesLoop,udp::Socketandudp::PacketBuffer, not the four removed types.Downsides
Notes
Evidence per removal
DataViewReader.u16()rg '\.u16\('oversrc/runtime/bake,test/bake,test/cli/inspect: no hit.DataViewWriter.capacity.capacityin the bake TypeScript is the assignment in the constructor.initCapacityis the only caller of the constructor.totalCount+=.dgram.tsblockgit blame: 589f941, 2024-04-26.replaceHandleandstartListeningare not defined in the file.us_udp_socket_t.connectedudp->connected = 0;.us_quic_stream_s.headers_deliveredquic.c: the field and one= 1. The struct is private toquic.c.Http2ResponseData::totalSizedata.totalSize = totalSize;. The other hits oftotalSizeare the function parameter.#undef FD_BITSFD_BITSinsrcandpackages.dryRunci.ts: the field, two assignments, one destructure. Nothing reads the binding.Platform.featuresDistro,Tierentriesci.tsor image inscripts/build/ci-images/spec.tscarries them.Emojiiskeyof typeof emojiMap, so a remaining caller would failtsc -p scripts/tsconfig.json. It passes.Taken out because an open pull request uses or removes the item
DataViewWriter.u8(): dead on main, but dev server: let the error page drop failures that a build fixed before its HMR socket subscribed #42075 adds its first caller (check.u8(IncomingMessageId.check_errors)inhmr-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 dev server: let the error page drop failures that a build fixed before its HMR socket subscribed #42075 has no type error foru8.declare module "bun:wrap"inbake.private.d.ts: no importer, but bake: consolidate the open robobun dev-server, HMR runtime, production build and router fixes #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.jspasses 3 of 4: the IPv6 multicast test fails withENODEVin the test container, with and without this change.prettierandcargo fmt --checkreport 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.handquic.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
--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.#[no_mangle]exports are unreachable in the Linux link. Each has a caller on another platform, or Remove dead code from bun_jsc, bun_runtime, bun_core, and the JSC bindings #40824, Remove dead code from the JSC bindings, headers.h, the console builtin, and bun_jsc #40232 or Remove dead code from the inspector protocol package, bun_jsc, bun_install, and the Windows named pipe modules #40557 removes it. A count of references for all 58,055 Rust definitions found no other item without a user. Of the 144allowattributes for the unused and unreachable lints, each covers code that depends oncfgor is macro output.bun_resolver -> bun_zstdis used undercfg(bun_codegen_embed).bun_wyhash -> bstris used by unit tests.USE(BIGINT32)andENABLE(MALLOC_BREAKDOWN)are never true. Remove dead code from SerializedScriptValue, ncrypto, and the WebCore IDL converters #43644 and Remove dead code from the inspector protocol package, bun_jsc, bun_install, and the Windows named pipe modules #40557 remove those branches.src/js,src/node-fallbacks,src/codegen,scripts/,misctools/,patches/(every patch file has a user),packages/exceptbun-types.Probably dead, left alone on purpose
PerformanceResourceTimingcluster undersrc/jsc/bindings/webcore(about 2,000 lines:PerformanceResourceTiming,PerformanceServerTiming,ResourceTiming,NetworkLoadMetrics,ResourceLoadTiming,ServerTimingand the two JS wrappers). The linker drops every constructor, so no instance can exist. The two globals are public andtest/js/web/web-globals.test.jschecks 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 anX_getterfunction for 50 classes. 45 have no user. A removal needs a second macro and saves no source lines.byte,shortandlong long, and mostClampandEnforceRangespecializations inJSDOMConvertNumbers.cpp. No binding uses them, butsrc/codegen/bindgen.tsmapst.i8,t.i16andt.i64to them.src/js/bun/sql.ts: the export propertiessql,Query,postgresand the four error classes. Native code reads onlydefaultandSQL. It is not certain that no loader path exposes the module object.us_nq_settings_set_scid_lenandus_nq_settings_set_delay_onclose(node_quic_shim.c, declared insrc/lsquic_sys/lib.rs): no caller.node:quicis under active work.Event::currentTargetIsInShadowTree()and its bit: no reader. The lines sit next to a hunk of Remove dead code from the WebCore bindings, the JSSink codegen, and two Rust helpers #39929.WebSocketWrapper.close()and[Symbol.dispose](),streamingStarted, thelineandcolumnbookkeeping and seven enum members inJavaScriptSyntaxHighlighter.ts, andexternalsinsrc/node-fallbacks/build-fallbacks.ts. Each sits next to a hunk of Remove dead code from the class and sink generators, JSBuffer, the bake client, bun-error, and the build scripts #43378, Remove dead code from the JSC bindings, the codegen scripts, the CI scripts, bun_core, and bun_jsc #40492, Remove dead code from the build scripts, the vscode and debug-adapter packages, the bake overlay, and two WebCore files #40122 or Remove dead code from bun_core, the JSC bindings, the debugger, and 24 other crates #41385.internal: trueproperty option of the class generator. A guard throws on it, so the branches behind it cannot run. Five open pull requests touchgenerate-classes.ts.H2App::getNativeHandle(next to a hunk of Remove dead code from the WebCore IDL converters, exception reporting, and the uws HTTP/2 shim #41195) anduws_app_listen_config_t(its last user goes with Remove dead code from the WebCore, node, and sqlite bindings, uWS/uSockets, SecretsLinux, and five Rust crates #42431).UWS_ALLOW_SHARED_AND_DEDICATED_COMPRESSOR_MIX,UWS_ALLOW_8_WINDOW_BITSandLIBUS_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.shwithscripts/lldb-inline-tool.cpp, andpackages/h3blast: nothing references them. They read as tools that a person runs by hand.napi_internal_get_version. Re-land #41330's build fixes and link-time checks, without JSC/ICU in the build graph #42556 renamed that function toBun__napi_get_versionon main, and it still has no caller.