Conversation
Contributor
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (20)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…int fail without the removal, sweep bindgen docs
Contributor
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
src/js/internal/quic/quic.ts:5227-5230— RemovinggetQuicEndpointStatebreaks two currently-passing Node.js compat tests:test/js/node/test/parallel/test-quic-internal-endpoint-listen-defaults.mjs(lines 18, 25) andtest-quic-internal-endpoint-stats-state.mjs(lines 26, 31, 61, 69) both destructure it from(await import('internal/quic/quic')).defaultand call it. These tests are not skipped (process.features.quicistrueper BunProcess.cpp:4490) and cannot be modified pertest/js/node/test/parallel/CLAUDE.md, so keep the declaration, static-block assignment, and export entry, and drop the/\bgetQuicEndpointState\b/check from the new source-lint at line 110.Extended reasoning...
What the bug is
The PR deletes
getQuicEndpointStatefromsrc/js/internal/quic/quic.tsin three places — thelet getQuicEndpointState;declaration, the assignment inside theQuicEndpointstatic block (getQuicEndpointState = function (endpoint) { ... }), and its entry in the module's default export object — on the claim that it is "assigned but never called" and that "every symbol was grepped across src/, scripts/, packages/, test/". That claim is false fortest/.The specific code path that triggers it
Two upstream Node.js parallel tests import and call this function directly from the internal module's default export:
test/js/node/test/parallel/test-quic-internal-endpoint-listen-defaults.mjs— line 18 destructuresconst { getQuicEndpointState } = (await import('internal/quic/quic')).default;, then line 25 callsgetQuicEndpointState(endpoint).test/js/node/test/parallel/test-quic-internal-endpoint-stats-state.mjs— lines 26–27 destructuregetQuicEndpointStatefrom the same default export, then call it at lines 31, 61, and 69.
After this PR,
(await import('internal/quic/quic')).default.getQuicEndpointStateisundefined, so the destructured binding isundefinedand the first call throwsTypeError: getQuicEndpointState is not a function.Why existing code doesn't prevent it
Both tests are gated behind
skip(!hasQuic), wherehasQuic = hasCrypto && !!process.features.quic(test/js/node/test/common/index.js:89). Bun setsprocess.features.quic = trueunconditionally (src/jsc/bindings/BunProcess.cpp:4490), so the skip guard is not taken and the tests execute. Both test files appear intest/expected-durations.jsonwith real per-platform timings (default/asan/musl/windows), meaning they currently run and pass in CI on every lane. Pertest/js/node/test/parallel/CLAUDE.md, these are upstream Node.js tests that "we cannot modify", so editing the tests to stop using the function is not an option.The PR's smoke tests only ran
test/js/node/quic/quic-endpoint.test.ts, not thetest/js/node/test/parallel/test-quic-internal-*files. The grep also apparently missed these hits — likely because the search targetedsrc/js/node/quic.tsconsumers (which indeed never destructuresgetQuicEndpointState) rather than the fulltest/tree.Step-by-step proof
- Before this PR,
src/js/internal/quic/quic.tsexportsdefault { …, getQuicEndpointState }, assigned in theQuicEndpointstatic block tofunction (endpoint) { assertIsQuicEndpoint(endpoint); return endpoint.#inner.state; }. - This PR removes all three sites, so the default export no longer contains a
getQuicEndpointStatekey. test-quic-internal-endpoint-listen-defaults.mjs:18runsconst { getQuicEndpointState } = (await import('internal/quic/quic')).default;→getQuicEndpointState === undefined.- Line 25 runs
const state = getQuicEndpointState(endpoint);→TypeError: getQuicEndpointState is not a function. - Same failure at
test-quic-internal-endpoint-stats-state.mjs:31. - Both tests currently pass in CI (present in
test/expected-durations.json), so this is a concrete regression: two green tests turn red.
Impact
Two currently-passing Node.js compat tests will fail in CI on every platform. Per REVIEW.md: "Never silently weaken, skip, or delete an existing test or safety net." This is a merge-blocking regression, not a cosmetic issue.
How to fix
- Revert the
getQuicEndpointStateremovals insrc/js/internal/quic/quic.ts: restore thelet getQuicEndpointState;declaration, the assignment inside theQuicEndpointstatic block, and thegetQuicEndpointState,entry in the default export. - Drop the
/\bgetQuicEndpointState\b/check fromtest/internal/source-lints/dead-symbols-bindgen-scripts-misc.test.ts:110— otherwise the lint will fail once the function is re-added. - Update the PR description's Built-in JS section to remove the
getQuicEndpointStateclaim.
The other two removed export entries (
getQuicStreamState,getQuicSessionState) are fine — grep oftest/shows no external consumers for those; the functions themselves stay defined and are used in-file.
Collaborator
Author
|
Correct, and this was addressed in b8b0bf6, which landed while that review was running: src/js/internal/quic/quic.ts is fully restored (declaration, static-block assignment, and all three export entries, since the internal module is API surface for tests running under --expose-internals), the getQuic* checks were dropped from the source lint, and the PR description was updated. Both vendored tests (test-quic-internal-endpoint-listen-defaults.mjs and test-quic-internal-endpoint-stats-state.mjs) pass against the debug build of this branch, and the current diff no longer touches quic.ts. |
Contributor
There was a problem hiding this comment.
Beyond the inline nit, I spot-checked two removals that touch shipped code: the five dropped symbols entries in src/js/internal/sql/query.ts — grepped every require("internal/sql/query") site (mysql.ts, postgres.ts, shared.ts, sqlite.ts, bun/sql.ts) and only _handle/_results/_strings/_values are destructured; and src/jsc/bindings/v8/v8config.h — no #include of it anywhere in src/ outside the file itself.
Extended reasoning...
The one finding this run is a nit (write-only ArgStrategyChildItem chain in the build-time codegen script; generated output byte-identical). I verified two of the runtime-facing removals independently: the internal/sql/query symbols export trim has no destructuring consumers for the removed keys, and the deleted v8/v8config.h shim is not included by any source file. Given the breadth (20 files across codegen, Rust crates, C++ bindings, built-in JS, and orphaned scripts) and the four correction rounds already on this PR, deferring to a human for the final pass rather than approving.
Collaborator
Author
|
Updated 10:49 AM PT - Aug 8th, 2026
✅ @robobun, your commit b0d33de48871e2d8790db942e7f5fae95d54061e passed in 🧪 To try this PR locally: bunx bun-pr 37208That installs a local version of the PR into your bun-37208 --bun |
Contributor
There was a problem hiding this comment.
All five rounds of prior feedback have been addressed (docs sweep, unused typedefs destructuring, the accidental v8config.h re-add, the ArgStrategyChildItem chain), and this pass found no new issues. Given the breadth — ~1,280 lines removed across codegen infrastructure, several Rust crates, a built-in JS module, and five developer scripts — a maintainer glance would still be worthwhile, particularly on whether the deleted scripts (gamble.ts, github-metrics.ts, debug-coredump.ts, lldb-inline) are still in anyone's local workflow.
What was reviewed:
- bindgen.ts / bindgen-lib*.ts: confirmed the removed Zig-emission machinery,
zigEnum,implNamespace/exposedOn,allFunctions,snake, andArgStrategyChildItemhave zero remaining references; codegen output is claimed byte-identical. internal/sql/query.ts: verified the five droppedsymbolsentries are not destructured by any requirer (mysql/postgres/shared/sqlite).- Rust removals (
QuoteEscapeFormatFlags.ascii_only,QueryStringMap.name_count,InternalDNSRequest, napi stub): write-only or comment-only, no callers. v8config.h: no#includereferences anywhere insrc/.
Extended reasoning...
Overview
This PR removes ~1,280 lines of dead code across 20 files: the Zig-emission machinery in src/codegen/bindgen*.ts (left behind after GeneratedBindings.zig stopped being written), five orphaned developer scripts under scripts/, the v8config.h include shim, write-only fields in bun_core::string::immutable and bun_url, an unused re-export alias in dns_jsc, a commented-out napi stub, unused symbols export entries in internal/sql/query.ts, the never-read isEventEmitter class-def option, and the implNamespace bindgen option. A new source-lint test pins the removals.
Security risks
None. This is deletion-only (plus a docs paragraph and a source-lint test). No auth/crypto/permissions/parsing of untrusted input is touched. The one runtime-visible surface change is dropping five entries from an internal-module symbols export object; I verified none of the four requirers destructure those keys.
Level of scrutiny
Medium-high, driven by breadth rather than depth. Each individual removal is mechanical and independently verifiable, and the author's methodology (grep across src/test/packages/codegen output, byte-identical bindgen.ts output, full bun bd, rust:check-all 10/10) is sound. The bindgen changes are the largest single chunk and carry the most risk, but the byte-identical-output check is the right verification. I've reviewed this across five prior rounds; every issue raised (stale docs, an accidental file re-add, two newly-dead-code chains) was fixed.
Other factors
- Why defer rather than approve: the PR spans codegen infrastructure, several Rust crates, a built-in JS module, docs, and wholesale-deletes five developer scripts. The scripts in particular (
gamble.ts,debug-coredump.ts,github-metrics.ts,lldb-inline) may be in a maintainer's local muscle memory without any in-repo reference — that's a judgment call for a human, not a grep. The earlier quic.ts near-miss (caught by CI and reverted before this review) also argues for one human pass. - What I confirmed this round:
zigEnum/implNamespace/exposedOn/isEventEmitterhave zero remaining hits insrc/;v8config.his not included anywhere; theinternal/sql/querysymbolsconsumers only destructure the four kept keys; the bindgen main-loop destructuring and theArgStrategyChildItemchain are gone per the diff (fixed in b0d33de). - All prior inline comments are resolved and no new findings surfaced from the bug-hunting pass.
Contributor
There was a problem hiding this comment.
I reviewed this PR across five iterations and this pass found nothing further — all earlier findings (stale bindgen docs, the typedefs/ArgStrategyChildItem leftovers, the accidental v8config.h re-add, the rustfmt blank line) are addressed at b0d33de. Given the breadth — ~1,300 lines out of build-critical bindgen codegen plus five orphaned dev scripts a maintainer may still use locally — a human sign-off would still be worthwhile.
What was reviewed:
- Bindgen Zig-emission removal: spot-checked that the surviving
resolve*ArgumentStrategycalls keep theircommunicationStruct.addside effects and that the C++ emission path is untouched; byte-identical output claim is consistent with the diff. internal/sql/querysymbols export trim: confirmed the four remaining entries (_handle,_strings,_values,_results) are exactly whatpostgres.ts/mysql.ts/shared.tsdestructure; the five removed keys have no requirers.isEventEmitter,v8config.h,InternalDNSRequest,QuoteEscapeFormatFlags.ascii_only,QueryStringMap.name_count: grepped src/ — no remaining readers.
Extended reasoning...
Overview
Net -1,162 line dead-code removal across 20 files: the never-flushed Zig-emission half of src/codegen/bindgen.ts and its supporting types in bindgen-lib{,-internal}.ts; the unused isEventEmitter class option; five orphaned dev scripts (gamble.ts, github-metrics.ts, debug-coredump.ts, lldb-inline.{sh,cpp}) plus their .gitignore entry; the v8config.h include shim; write-only Rust fields in bun_core::string::immutable, bun_url, and a dead alias in dns_jsc; a commented napi stub; five unread symbols export entries in internal/sql/query.ts; and doc updates to bindgen.mdx. A new source-lint test pins each removal.
Security risks
None. This is subtractive only — no new parsing, validation, or trust-boundary code. The removed code was either already unreachable (Zig emission fed a buffer that was never written to disk) or developer-local tooling.
Level of scrutiny
Medium-high, because bindgen.ts is build-critical: a mistake there would corrupt every generated JSC binding. The PR mitigates this well — the author verified GeneratedBindings.cpp and all Generated*.h are byte-identical before/after, and re-verified after each fixup commit. The Rust removals are trivially safe (write-only fields, a type alias with zero users, commented-out code) and covered by rust:check-all across all 10 targets. The sql/query.ts change I verified directly by grepping every requirer.
Other factors
This PR has been through five review iterations; every prior finding (stale implNamespace docs, the newly-dead typedefs binding, the ArgStrategyChildItem chain, the accidental v8config.h restore, the rustfmt double-blank-line) was promptly fixed and is confirmed present in the current diff. I spot-checked the load-bearing claims: no src/ reference to isEventEmitter or v8config.h; symbols consumers in postgres.ts/mysql.ts/shared.ts destructure only the four kept keys; resolveNullableArgumentStrategy/resolveComplexArgumentStrategy still run for their communicationStruct.add side effects.
I'm deferring rather than approving because (a) the bindgen surgery is broad enough that a maintainer glance at the byte-identical claim is cheap insurance, and (b) the five removed scripts have no in-repo references but a maintainer may know of local/undocumented usage that grep can't find.
Jarred-Sumner
added a commit
that referenced
this pull request
Aug 16, 2026
…egen, build scripts, and misc crates (#39249) Removes 2772 lines that nothing references (136 lines of signature, import and formatting adjustments added, plus a 214-line source lint pinning the removed symbols) across the WebCrypto bindings, the node:http internal binding and the C++/Rust behind it, the class code generator, the build scripts, and four Rust crates. No behavior change. ### Problem **WebCrypto (`src/jsc/bindings/webcrypto/`, about 560 lines)** - `SubtleCrypto.cpp` has had `isRSAESPKCSWebCryptoDeprecated()` returning `true` unconditionally since 2023, so every RSAES-PKCS1-v1_5 operation is rejected with `NotSupportedError` before the algorithm class is reached. Its `encrypt`/`decrypt`/`generateKey`/`importKey`/`exportKey` overrides, the whole of `CryptoAlgorithmRSAES_PKCS1_v1_5OpenSSL.cpp`, and `JSRsaKeyGenParams.{cpp,h}` (whose only caller was the unreachable generateKey branch) were dead. `isSafeCurvesEnabled()` is the same shape: always `true`, one never-taken branch. - `CryptoAlgorithmEcdsaParams::encoding` (DER signatures) and the `padding` fields of `RsaPssParams`/`RsaOaepParams` were only ever set by the old `src/bun.js/bindings/KeyObject.cpp`, deleted in April 2025. Since then the DER branches in `CryptoAlgorithmECDSAOpenSSL.cpp`, the padding branches in the PSS/OAEP OpenSSL files, and `platformEncryptWithHash`/`platformDecryptWithHash` ran on constant inputs. The same file left behind the `ignoreExtAndKeyOps` parameter of `convertDictionaryToJS(JsonWebKey)` (no caller passes `true`) and include blocks copied verbatim into `node_crypto_binding.cpp` and `AsymmetricKeyValue.cpp` (neither file uses any of them). - `CryptoAlgorithmMlDsaParams::isolatedCopy()` has no `crossThreadCopy` instantiation, `CryptoAlgorithm::VoidCallback` has no user, `SubtleCrypto::addAuthenticatedEncryptionWarningIfNecessary` is an empty body with two calls, and `CryptoKey.cpp`/`CryptoAlgorithmX25519.cpp` carried includes nothing in the file uses. **node:http binding (`src/js/internal/http.ts`, `NodeHTTP.cpp`, and the Rust it reached, about 880 lines)** - `internal/http.ts` destructures the object returned by `createNodeHTTPInternalBinding` and re-exports the members, but no module imports `getHeader`, `setHeader`, `Headers`, `assignHeaders`, `setRequestTimeout`, `headersTuple`, `webRequestOrResponseHasBodyValue` or `getCompleteWebRequestOrResponseBodyValueAsArrayBuffer` (the last users left with the http client rewrite); `assignEventCallback`, `Request`, `Response` and `Blob` were registered but not even destructured. A few symbols (`kDeprecatedReplySymbol`, `controllerSymbol`, `runSymbol`, `deferredSymbol`, `firstWriteSymbol`, `kEmptyObject`) and `isAbortError`/`get|setIsNextIncomingMessageHTTPS` lost their last reader in earlier sweeps. - That binding object is the only way into `NodeHTTP.cpp`'s `jsHTTPAssignHeaders`, `jsHTTPAssignEventCallback`, `jsHTTPSetTimeout`, `jsHTTPGetHeader` and `jsHTTPSetHeader`, which in turn were the only callers of `assignHeadersFromFetchHeaders`, `assignHeadersFromUWebSockets` (the `...ForCall` variant used by the request path stays) and the `RequestHeaderKind` helpers: about 600 lines of C++. - Those host functions were the only callers of the Rust exports `jsFunctionRequestOrResponseHasBodyValue`, `jsFunctionGetCompleteRequestOrResponseBodyValueAsArrayBuffer` (`Response.rs`), `Request__getUWSRequest`, `Request__setInternalEventCallback`, `Request__setTimeout` (`Request.rs`) and `NodeHTTPResponse__setTimeout` (`NodeHTTPResponse.rs`). `Request__setInternalEventCallback` was the only writer of `Request::internal_event_callback`, and the JS side stopped calling it in 2024, so `InternalJSEventCallback`, the `RequestContext::on_timeout` handler, `set_timeout_handler`, the `HAS_TIMEOUT_HANDLER` flag and its three clear sites, and `AnyRequestContext::enable_timeout_events` have not been reachable since then; `Body::Value::is_definitely_empty` was only used by the removed host function. **Code generators and build scripts (about 1000 lines)** - `generate-classes.ts`/`class-definitions.ts` still emitted code for `.classes.ts` keys no class file sets: `own` (and the `ZigGeneratedClasses.lut.txt` -> `.lut.h` build step in `scripts/build/codegen.ts`, whose output was an empty header), `callbacks`, the `accessor` field variant, `supportsObjectCreate`, `custom`, `zigOnly`, `defaultValue`, plus the `ONLY_ZIG` and `BUN_SILENT` environment switches nothing sets (`git grep` over all 30 `*.classes.ts` files, and `git log -G` for the keys that ever existed). `bundle-functions.ts` handled `$nakedConstructor`, `$sloppy` and `$intrinsic` directives no file in `src/js/builtins` uses, and tracked write-only fields; `generate-jssink.ts`, `replacements.ts` (`OutOfMemoryError` rewrite, no builtin throws it) and `cppbind.ts` had smaller leftovers. `src/runtime/bake/bake.bind.ts` was 100% comments and produced an empty `GeneratedBake.h` nothing includes. - `scripts/utils.mjs` (379 lines: `downloadTarget` and its helpers, `getBuildArtifacts` and its helpers, `getChangedFiles`, `isDocumentation`, `getPullRequestRepository`, `getRepositoryOwner`, `escapeYaml`, `escapeGitHubAction`, `parseNumber`, `getUser`, `isArm64`) and `scripts/runner.node.mjs` (`listArtifactsFromBuildKite`, a local `escapeGitHubAction`) exported functions none of the 11 importers import. `glob-sources.ts` globbed two patterns that have never matched a file; `flags.ts` defined `LIBUS_USE_BORINGSSL`, which nothing in `src/`, `packages/` or `vendor/` reads; `config.ts` carried three resolved fields nothing reads (the `PartialConfig` inputs stay); `depVersionsHeader.ts` and five `deps/*.ts` emitted seven version macros with no consumer in `BunProcess.cpp`, the header's only includer. **Rust crates (about 140 lines)** - `CssModuleReference::Local`/`Global` are never constructed (only `Dependency` is) and `eql` has no caller; `bun_sys` duplicated `NT_UNC_OBJECT_PREFIX`/`_U8` (the live copies are in `bun_paths`) and two Windows send-flag constants the wrappers do not use; `cares_sys::AddrInfo::name` has no caller; `libuv_sys` kept `Loop::{ref_, unref, unref_count, run, tick_with_timeout, wakeup}`, `uv_write_t::write_raw`, `uv_async_t::send`, `Process::get_pid`, `uv_stat_t::{atime, ctime}`, `ReturnCode::from_raw`, `ReturnCodeI64::init`, five type aliases and the two externs only those methods used. These come from the workspace reachability analysis in `tools/hawk/`, run for linux-gnu, darwin and windows-msvc and then re-checked by hand; the items it reported that are used only from `debug_assertions` code (which the release-profile analysis does not see) or only on FreeBSD were kept. ### Fix - Deletes the items above. The only non-deletion edits are the ones the deletions force: the three RSAES rejections in `SubtleCrypto.cpp` become unconditional (same error strings, pinned by `test/js/web/crypto/web-crypto.test.ts`), `isSupportedExportKey` loses an unused parameter, `platformEncrypt`/`platformDecrypt` in RSA-OAEP absorb their one-line `WithHash` wrappers, `RequestContext::set_timeout` keeps its `clear_timeout()` for `0`, one `const` destructure in `internal/http.ts` is reflowed, and `generate-classes.ts` conditions that became constant are folded. - Generator changes were verified by re-running `generate-classes.ts`, `generate-jssink.ts` and `bundle-modules.ts` into a scratch directory and comparing with the output generated before the change: byte-identical except for the dropped `#include "ZigGeneratedClasses.lut.h"` line and the two deleted webcrypto files disappearing from the `NativeFilenameCPP` union. `bun scripts/build.ts --configure-only` confirms `build.ninja` no longer references `GeneratedBake.h`, the `.lut` pair, or `LIBUS_USE_BORINGSSL`, and that the C++ source list changed only by the three deleted files. - Verified: `bun bd` builds; `bun run rust:check-all` passes on all 12 target triples; `cargo fmt --check`, prettier and clang-format are clean. `bun bd test` passes on `test/js/web/crypto/web-crypto.test.ts` (94), `test/js/node/crypto/{crypto.key-objects,crypto-rsa,sign-jwk-ieee-p1363,crypto-pqc,x509}` (162), `test/js/node/http/{node-http,node-http-server-timeouts,node-http-server-abort-events,node-http-res-settimeout-unref}` (151; the one failure, the http proxy test, fails identically with the released binary in this container), `test/js/bun/http/serve.test.ts` (283; the 4 failures, requestIP v6, root-range port, #6583 and /bun:info, fail identically with the released binary here), `test/js/node/http2/node-http2.test.js`, `test/js/node/fs/fs-leak.test.js`, `test/js/bun/udp/dgram.test.ts`, `test/js/node/readline/readline.node.test.ts`, `test/bundler/css/css-modules.test.ts`, `test/js/node/process/process.test.js` (dependency version assertions pass; the `USER` env assertion fails on both binaries here), the nine `test/internal/build-*`/`bindgen`/`macos-cross-config`/`rust-*` tests, `test/js/bun/perf/linker-order.test.ts`, and all of `test/internal/source-lints/` (166). The new `dead-symbols-webcrypto-nodehttp-codegen.test.ts` fails on main and passes here. - Cross-checked against the 19 open dead-code PRs at the line level: no deleted line here is deleted by any of them, except boilerplate lines (`auto& vm = ...`) that #35437 also deletes from a different function in `NodeHTTP.cpp`. Hunks adjacent to open PRs, which will need a trivial rebase on whichever side lands second: the `internal/http.ts` destructure and `NodeHTTP.cpp` registration block (#35437 removes `setServerIdleTimeout` there), the `CryptoKey.cpp` include block (#38005 removes the neighbouring line), `CryptoAlgorithmRSA_PSSOpenSSL.cpp` (#38005's hunk still applies), `class-definitions.ts` (#37208 removes `isEventEmitter`), `generate-classes.ts` (#37149 removes four DOMJIT includes), and `c_ares.rs` (#38703 removes a method 30 lines below). ### Background - `createNodeHTTPInternalBinding` is the C++ function behind `$cpp("NodeHTTP.cpp", ...)` in `internal/http.ts`; it builds a plain object whose properties are JS functions wrapping C++ host functions. A host function registered there is reachable only if some builtin reads the property, which is why the JS import graph decides what is dead in the C++ file. - Rust functions marked `export_name`/`uws_callback(export = ...)` exist for C++ callers; `cppbind`/js2native codegen emits the glue from the sources, so once the last C++ caller is gone they are unreachable, and the workspace lints (`dead_code`/`unused_imports` are `deny`) then flag whatever only they called, which is how the `Request.rs`/`RequestContext.rs` cascade was found and bounded. - `Request::internal_event_callback` was the hook node:http used to be told about per-request timeouts and aborts; `RequestContext::set_timeout_handler` registered the uWS timeout callback only when that hook was set. With no setter, the flag was never set and the handler never registered, so removing them changes nothing at runtime; `server.timeout()` still arms the socket timeout and the abort path is untouched. - `generate-classes.ts` reads every `src/**/*.classes.ts` and emits `ZigGeneratedClasses.{h,cpp}` plus `generated_classes.rs`; a feature of the class definition format that no class uses is dead code in the generator, and its removal is checkable by diffing the generated files. - `tools/hawk/README.md` describes the workspace-wide reachability analysis used for the Rust items: rustc's per-crate `dead_code` lint treats every `pub` item of a library crate as live, so cross-crate dead `pub` items need this separate pass. <details> <summary>Found dead but deliberately left alone</summary> - `bun_shim_impl::read_without_launch` and `FromBunShellContext` (`src/install/windows-shim/bun_shim_impl.rs`) have no caller, but the `LauncherMode::ReadWithoutLaunch` mode is threaded through the launcher, so removing it cleanly means de-generifying `launcher()`; better as its own change. - The write-only `CssModule::references` map itself (this PR removes only the never-constructed variants); the `#[cfg(target_arch = "wasm32")]` branches scattered through the parser/output code (no wasm32 target is built, but earlier commits describe it as "not built yet", so that is a product call); `windows_errno::posix::{mode_t, E}` and `bun_sys::File::write` (unused, but intentional API parity with the POSIX side); `WindowsLoop::{wait, unref}` and the Windows `us_socket_t::write_fd` stub (8 and 7 lines in files three open PRs are editing). - `AnyResponse::on_timeout` / `uws_res_on_timeout` in `bun_uws_sys` lose their last caller with this PR (`set_timeout_handler` was it); left for the next pass since those files are in open PRs. - DOMJIT support in `generate-classes.ts` is disabled on purpose (`define()` strips it), not dead; the split CI build modes in `scripts/build/profiles.ts` were explicitly kept by #37733 four days ago; the `*.idl` files in webcrypto are kept as documentation as in the previous sweeps; the AES-GCM `> UINT64_MAX` checks are upstream-identical. - Every item carrying `#[allow(dead_code)]` in the tree was re-verified and is live on some platform or under `debug_assertions` (the inventory in `dead-code-escape-limits.json` is accurate), so nothing was taken from there. - `CryptoAlgorithmAKPShared.h` does not include `CryptoAlgorithmParameters.h`, so `CryptoAlgorithmMLDSA.cpp` only compiles inside its unified bundle; pre-existing and unrelated to this change. </details> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Jarred-Sumner
pushed a commit
that referenced
this pull request
Aug 18, 2026
… and misc crates (#39574) ### Problem - `src/jsc/bindings/libuv/` is only on the include path for non-Windows builds (`scripts/build/flags.ts`, "libuv stubs for unix"). `uv/win.h` (703 lines) and `uv/tree.h` (512 lines, included only by `win.h`) are never reached. - `uv/sunos.h`, `uv/os390.h`, `uv/aix.h` and `uv/posix.h` are selected by `uv/unix.h` only on Solaris, z/OS, AIX, IBM i, Cygwin, Haiku, QNX and Hurd. Bun builds for linux, macOS and FreeBSD. - `packages/bun-error` is embedded in the dev error page (`src/runtime/server/dev-error-page.html`). The page calls the function behind `Symbol.for("Bun__renderFallbackError")` and nothing else. `renderRuntimeError`, the abort state `dismissError` kept for it, and the two modules only it imported (`sourcemap.ts`, `stack-trace-parser.ts`) have no callers. #37081 lists this path as a follow-up. - `bun_zlib_sys::posix` and `bun_zlib_sys::win32` declare zlib functions that nothing calls. `bun_zlib` declares its own. The only use of the two modules was as re-exports of the types in `shared.rs`. - A set of `pub` items in other crates has no user in any crate. rustc cannot report them because `pub` items count as used. ### Fix - Delete the six libuv headers. `uv.h` now includes `uv/unix.h` directly. `uv/unix.h` keeps the linux, darwin and BSD branches. `uv-posix-polyfills.c` drops the commented-out copies of the removed branches. - Delete `renderRuntimeError`, `sourcemap.ts` and `stack-trace-parser.ts`. `dismissError` keeps the part that removes the overlay. `runtime-error.ts` stays (it has a test). - Delete `bun_zlib_sys/posix.rs` and `win32.rs`. `bun_zlib` imports the types from `bun_zlib_sys::shared`, which is where the removed modules took them from. - Delete the unused Rust items listed below, plus the trait implementations and imports that only they needed. Verification: - Every Rust item was found by making the unexported items crate-private and compiling the workspace. An item is deleted only if rustc reports it dead on x86_64 linux (dev, release, and with the `bun_debug` and `bun_asan` cfgs), aarch64 linux, x86_64 musl, x86_64 Windows and aarch64 macOS. - Each removed name was also searched in `src/codegen/`, the `*.classes.ts` files, `src/js/` and the C++ bindings. Items that a codegen template can emit were kept. - `bun run rust:check-all`: 12 of 12 targets pass. `cargo check --workspace --all-targets` passes (benches and unit tests still compile). `cargo check -p bun_shim_impl --features shim_standalone` for the Windows target passes. - `bun bd` builds. The build recompiles `uv-posix-stubs.c` and `uv-posix-polyfills.c` against the trimmed `uv.h`, and rebuilds the bun-error bundle, which no longer exports `renderRuntimeError`. - New test in `test/js/bun/http/serve.test.ts`: it takes the bun-error bundle out of a real 500 page, evaluates it outside a browser, and checks that the bundle registers the renderer and that `dismissError` is a no-op when nothing is rendered. This is the surface the `packages/bun-error` change touches. - `bun bd test` passes for `test/js/bun/http/serve.test.ts -t "dev error page"` (including the new test), `test/js/bun/runtime-error.test.ts`, `test/js/bun/util/{zstd,arraybuffersink,filesink}.test.ts`, `test/js/node/zlib/deflate-streaming.test.ts`, `test/js/web/encoding/text-{encoder,decoder}.test.*`, `test/js/workerd/html-rewriter.test.js`, `test/js/bun/css/nth-anplusb-ident.test.ts`, `test/js/web/fetch/blob.test.ts` and `test/internal/source-lints/dead-code-escapes.test.ts`. - `cargo fmt --check`, clang-format on the touched C file and prettier on the touched TypeScript files pass. <details> <summary>Removed Rust items</summary> - `bun_zlib_sys`: modules `posix` and `win32` (`struct_gz_header_s`, `gz_header`, `gz_headerp`, `in_func`, `out_func`, and the `deflate*`, `inflate*`, `compress*`, `uncompress`, `adler32`, `crc32`, `zlibVersion` declarations), `shared::voidpf`. - `bun_zlib`: declarations `compress`, `compressBound`, `uncompress`, and the `internal` module that selected between the two removed modules. - `bun_zstd`: `decompress` (every caller uses `decompress_append`). - `bun_libdeflate_sys`: `libdeflate_deflate_decompress` (the `_ex` variant is the one in use). - `bun_mimalloc_sys`: `mi_strdup`, `mi_heap_collect`, `mi_thread_set_in_threadpool`. - `bun_cares_sys`: `ares_strerror`. - `bun_windows_sys`: `SetHandleInformation`, `closesocket`. - `bun_alloc`: `default_alloc::calloc`. - `bun_core`: `GenericIndexInt::from_usize` and its macro-generated implementations. - `bun_css`: the four deprecated `to_css` methods on `GenericSelectorList`, `GenericSelector`, `GenericComponent` and `Combinator`. Their bodies were `unreachable!()`; the serializer functions replaced them. - `bun_runtime`: `JsSinkType::done` and its six overrides, `FileCloser::update` and its implementations, `ReadableStream::to_js`, `node_fs::Null::to_js`. </details> <details> <summary>Overlap with open pull requests</summary> The deletions here were checked against the open dead-code pull requests (#35437, #35775, #35880, #36115, #36237, #37012, #37149, #37181, #37208, #37301, #37454, #37659, #37788, #38005, #38900, #39319, #39561) and against #38958 and #35075. Nothing deleted here is deleted by any of them. Candidates they already cover were left out: `src/jsc/bindgen.rs` (#37149), the dead `pub use` re-exports (#39319), the simdutf big-endian and UTF-32 wrappers (#38958), and the items named in the skip lists of the others. Some files here (`bun_alloc/lib.rs`, `bun_core/util.rs`, `libdeflate.rs`, `mimalloc.rs`, `node_fs.rs`, `Blob.rs`, `FileSink.rs`, `ReadableStream.rs`, `streams.rs`, `windows_sys/externs.rs`) are also touched by open pull requests in different hunks. #36437 edits `packages/bun-error` from a base that predates #37081; it changes one import line in `stack-trace-parser.ts` and keeps `renderRuntimeError`, so it does not overlap with this deletion but will need a rebase. </details> <details> <summary>Found but not deleted (judgment calls for a maintainer)</summary> - `packages/bun-inspector-protocol/src/protocol/v8/` (about 32,600 lines): not exported by the package index since 2023 and regenerated only with the opt-in `--v8` flag of `scripts/generate-protocol.ts`. #39110 kept the flag, so this needs a decision. - `packages/h3blast` (1,468 lines) and `packages/bun-build-mdx-rs` (558 lines): nothing in the repository references them. They may be kept on purpose as a load generator and a proof of concept. - `packages/bun-error/runtime-error.ts` is unused by the page but covered by `test/js/bun/runtime-error.test.ts`. The four images in `packages/bun-error/img/` are referenced only by the source glob in `scripts/glob-sources.ts`. - `HotReloadTaskView` in `src/jsc/hot_reloader.rs`: both `reload` implementations ignore the task, and `VirtualMachine::reload` ignores its `Option<HotReloadTask>` argument. Removing the plumbing is a small refactor rather than a deletion. - `react_compiler/compile_result.rs` has constructors and fields with no users, but the file says the types are waiting to be wired up. - The streams-era private globals in `BunBuiltinNames.h` (`makeGetterTypeError`, `makeDOMException`, `addAbortAlgorithmToSignal`, `removeAbortAlgorithmFromSignal`, `isAbortSignal`, `createUninitializedArrayBuffer`, about 100 lines of `ZigGlobalObject.cpp`) have no JS callers. Both files are being edited by several open dead-code pull requests, so they were left for a later run. </details> ### Background - rustc's `dead_code` lint treats every `pub` item in a library crate as used, because another crate could import it. In this workspace every crate is an implementation detail of one binary, so a `pub` item with no importer in any crate is dead in the same sense as a private one. Making such items crate-private for one compile lets rustc report the ones with no users at all. The visibility changes themselves are not part of this pull request. - On POSIX, bun does not link libuv. Node-API addons that reference libuv symbols get `uv-posix-stubs.c` and `uv-posix-polyfills*.c`, which are compiled against the copied headers in `src/jsc/bindings/libuv/`. On Windows the real libuv is linked and that directory is not used. - `JsSinkType` is the Rust trait behind the native sink classes (`FileSink`, `ArrayBufferSink`, the HTTP response sinks). Its methods are called from the shared sink glue in `Sink.rs`; `done` was declared there but the glue never called it. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts <!-- robobun:evidence:end -->
Collaborator
|
Closing as stale: this has merge conflicts with main. If the dead code is still present, please open a fresh PR against current main. |
Jarred-Sumner
pushed a commit
that referenced
this pull request
Aug 18, 2026
…cy edges (#37301) Scheduled dead-code sweep. This run scanned areas not claimed by the open dead-code PRs (#37208, #37272, #37229, #37181, #37149, #37089, #37062, #37012, #36237, #36115, #35880, #35775, #35437): src/runtime/crypto, src/runtime/node, src/runtime/webcore, src/runtime/api, src/runtime/socket, src/runtime/image, src/runtime/webview, src/http, src/dns, src/dotenv, src/which, src/glob, src/s3_signing, src/patch, src/router, src/semver, src/paths, src/threading, src/event_loop, src/io, src/transpiler, src/sourcemap, src/ast, src/parsers, src/options_types, src/platform, src/analytics, src/base64, src/standalone_graph, src/shell_parser, src/md, src/output, src/ptr, src/safety, src/perf, src/dispatch, src/uws, all 697 headers and 572 cpp files under src/jsc/bindings, src/js, scripts/, and commented-out blocks repo wide. The yield is small because the workspace denies `dead_code` and `unreachable_pub`, and prior sweeps plus the 13 open PRs hold everything else. Everything that survived verification: ## Removed - `ToJSError::MacroError` (src/ast/nodes.rs): never constructed anywhere; its only mention was a defensive match arm in src/runtime/api/YAMLObject.rs, now folded into the remaining defensive arm. (The `MacroError` enum in src/js_parser_jsc/Macro.rs is a different, live type.) - `pub use crate::webview::chrome_process as ChromeProcess` (src/runtime/api.rs): zero references to the alias; the module stays reachable via `crate::webview::chrome_process` and its `host_fn` exports. - `strum::IntoStaticStr`, `PartialEq`, `Eq` derives on `Algorithm` (src/runtime/crypto/PasswordObject.rs): the generated `From<Algorithm> for &'static str` and equality impls have zero call sites. Name parsing goes through `algorithm_from_zig_string` and hash-string formatting through `pwhash`. - Commented-out `globalThis.ASSERT` draft (src/runtime/bake/debug.ts lines 1-11): superseded verbatim by the live `globalThis.DEBUG.ASSERT` directly below it. - Unused Cargo dependency edges: `bun_runtime` does not reference `bun_css` (only `bun_css_jsc`) or `bun_transpiler`; `bun_spawn` does not reference `bstr`. Verified by zero textual references including cfg-gated code; note `rust-argon2` looks unreferenced by the same textual check but is live via `use ::argon2 as vendor` (lib name differs from package name), so it stays. ## Verification - rg zero references for each symbol across src/, scripts/, packages/, and build/debug/codegen output - `bun bd` builds clean - `bun scripts/rust-check-all.ts`: 10 ok, 0 failed, 0 skipped - `bun bd test` on password.test.ts (71 pass), yaml.test.ts (642 pass), bake/deinitialization.test.ts (1 pass) ## Not removed (follow-up notes) - src/runtime/webcore/TextEncoder.rs defines `unsafe extern "C" fn c`, a byte-identical duplicate of the adjacent `TextEncoder__encode16`, exported under the global symbol name `c`. No C++ declaration or caller was found, but extern "C" items are excluded from this sweep on principle. Looks like a rename accident worth a human look. - The h2 send-side cluster in src/runtime/api/bun/h2/connection.rs (~200 LOC, test-only callers) is scaffolding for the node:http2 rewrite, as already noted in #35880. - scripts/find-dead-exports.ts and scripts/trace.sh have no in-repo referencers but look like manual dev tools; left alone. - The boilerplate dep edges in src/{collections,css,glob,io,paths,router}/Cargo.toml are already removed by #35437 and #37229, so they are deliberately not duplicated here. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 9 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 3 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts bun test v1.4.0 (3cadf9f48) test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts: 33 | // hash-string formatting goes through pwhash); PartialEq/Eq were likewise 34 | // never used. 35 | ["src/runtime/crypto/PasswordObject.rs", /\bstrum\b/], 36 | ]; 37 | const resurrected = checks.filter(([file, re]) => re.test(src(file))).map(([file, re]) => `${file}: ${re.source}`); 38 | expect(resurrected).toEqual([]); ^ error: expect(received).toEqual(expected) - [] + [ + "src/ast/nodes.rs: \bMacroError\b", + "src/runtime/api/YAMLObject.rs: Up::MacroError", + "src/runtime/api.rs: chrome_process as ChromeProcess", + "src/runtime/crypto/PasswordObject.rs: \bstrum\b", + ] - Expected - 1 + Received + 6 at <anonymous> (/workspace/bun/test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts:38:23) (fail) dead Rust symbols (ast, runtime) do not reappear [38.66ms] 48 | }); 49 | 50 | test("dead built-in JS co ... (truncated) release without fix: 3 FAILED bun test v1.4.0-canary.1 (9008ae7) test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts: 33 | // hash-string formatting goes through pwhash); PartialEq/Eq were likewise 34 | // never used. 35 | ["src/runtime/crypto/PasswordObject.rs", /\bstrum\b/], 36 | ]; 37 | const resurrected = checks.filter(([file, re]) => re.test(src(file))).map(([file, re]) => `${file}: ${re.source}`); 38 | expect(resurrected).toEqual([]); ^ error: expect(received).toEqual(expected) - [] + [ + "src/ast/nodes.rs: \bMacroError\b", + "src/runtime/api/YAMLObject.rs: Up::MacroError", + "src/runtime/api.rs: chrome_process as ChromeProcess", + "src/runtime/crypto/PasswordObject.rs: \bstrum\b", + ] - Expected - 1 + Received + 6 at <anonymous> (/workspace/bun/test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts:38:23) (fail) dead Rust symbols (ast, runtime) do not reappear [0.59ms] 48 | }); 49 | 50 | test("dead built-in JS code does not reappear", () => { 51 | // bake/debug.ts: commented-out globalThis.ASSERT draft superseded verbatim 52 | // by the live globalThis.DEBUG.ASSERT directly below it. 53 | expe ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts bun test v1.4.0 (3cadf9f48) test/internal/source-lints/dead-symbols-runtime-ast-deps.test.ts: (pass) dead Rust symbols (ast, runtime) do not reappear [33.71ms] (pass) dead built-in JS code does not reappear [2.28ms] (pass) unused Cargo dependency edges do not reappear [5.20ms] 3 pass 0 fail 7 expect() calls Ran 3 tests across 1 file. [2.35s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 669ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/119] gen bake.{client,server,error}.js -> bake.client.js, bake.server.js, bake.error.js [2/119] gen generated_host_exports.rs generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited [2/119] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) ^[[1m^[[92m Compiling^[[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) ^[[1m^[[92m Compiling^[[0m bun_errno v0.0.0 (/workspace/bun/src/errno) ^[[1m^[[92m Compiling^[[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) ^[[1m^[[92m Compiling^[[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) ^[[1m^[[92m Compiling^[[0m bun_safety v0.0.0 (/workspace/bun/src/safety) ^[[1m^[[92m Compiling^[[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) ^[[1m^[[92m Compiling^[[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) ^[[1m^[[92m Compiling^[[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) ^[[1m^[[92m Compili ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` Cargo.lock | 3 -- src/ast/nodes.rs | 1 - src/runtime/Cargo.toml | 2 - src/runtime/api.rs | 2 - src/runtime/api/YAMLObject.rs | 9 ++-- src/runtime/bake/debug.ts | 11 ---- src/runtime/crypto/PasswordObject.rs | 6 +-- src/spawn/Cargo.toml | 1 - .../dead-symbols-runtime-ast-deps.test.ts | 63 ++++++++++++++++++++++ 9 files changed, 68 insertions(+), 30 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests Cargo.lock 0 0 0 src/ast/nodes.rs 1 1 0 src/runtime/Cargo.toml 1 1 0 src/runtime/api.rs 1 1 0 src/runtime/api/YAMLObject.rs 2 3 0 src/runtime/bake/debug.ts 1 1 0 src/runtime/crypto/PasswordObject.rs 1 1 0 src/spawn/Cargo.toml 1 1 0 …rnal/source-lints/dead-symbols-runtime-ast-deps.test.ts 0 2 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: Alistair Smith <hi@alistair.sh>
Jarred-Sumner
pushed a commit
that referenced
this pull request
Aug 21, 2026
…uilt-in JS, bindgen, uSockets, and 66 Cargo manifests (#39732) ### Problem - The tree carries code that nothing references: FFI shims with no caller on either side, enum variants that are never constructed, write-only fields, exports that no module imports, and Cargo dependency edges that no source file uses. - 16 dead-code PRs that removed much of this were closed as stale after merge conflicts. Their deletions were never re-applied (list in the notes). ### Fix - Re-apply the deletions that still apply to current main and add new ones in the same areas: `simdutf_sys`, `ncrypto` and the WebCore bindings, `js_parser`, `bundler`, `js_printer`, `bun_install`, built-in JS, bindgen, uSockets, and 66 Cargo manifests. 176 files, +95 / -3129 (`Cargo.lock` is -436 of that). - Every deletion was checked again with `rg` over `src/`, `packages/`, `src/codegen/` and the generated code. Items that became live again since the old PRs stay (examples in the notes). - No deletion duplicates an open dead-code PR. Deletions that need #39618 or #39697 to land first are listed as follow-ups in the notes. - Verified: `bun bd`, `bun run rust:check-all` (12 targets pass), byte-identical bindgen output, `test/internal/source-lints`, and the test files of the touched areas (list in the notes). New tests in `transpiler.test.js` and `bundler_edgecase.test.ts` pin the two diagnostics that `mark_strict_mode_feature` still emits and the class body printing that `visit_class` still does. ### Background - A Rust `extern "C"` declaration with no call site creates no link reference, so a dead FFI chain is removed on both sides at once. Three C++ definitions whose Rust side #39618 removes stay until that PR lands. - `cargo check` covers the host target only. `rust:check-all` repeats it for every shipped target, which proves the `#[cfg]`-gated deletions and the Windows-only dependency removal (`bun_sys` -> `bun_output`). - The dependency removals are manifest-only. `Cargo.lock` loses the matching entries and nothing else: no version changes. <details><summary>Notes</summary> **Closed PRs re-applied**: #35437, #35559, #35775, #35880, #36115, #36237, #37012, #37062, #37089, #37208, #37272, #37454, #37788, #38005, #38439, #38703. **Kept because they are live again on main**: `GenericIndexOptional::{get, is_some, is_none}`, the WebSocket deflate `OutOfMemory` variants, `V8Local::reinterpret`, `SystemErrno::MAX`, `MarkPopErrorOnReturn::peekError`, `ResourceTiming::populateServerTiming`, `UvHandle` in `test/parallel/Channel.rs`, the `h2::FrameType` entries that `hawk.toml` marks as a code table. **Tests run**: node-http, node-http2, fetch headers, streams, readable-stream-blob-consumed, filesink, transpiler, buffer, url, FormData, TextEncoder, MessageChannel, serve-direct-readable-stream, serve-body-leak, crypto key objects, crypto-rsa, scrypt, pbkdf2, sqlite-sql, local-sql, postgres-simple-query-pipeline, sql-helpers-validation, bun-outdated, websocket-server, test/internal (bindgen, codegen outputs, source lints). The `localhost` proxy test in node-http and the concurrent WebSocket send tests fail the same way on unmodified main in this environment (the first is a `localhost` resolution issue, the second is the 300k-message benchmark starving its concurrent neighbours in a debug build). **By kind**: C/C++ -1327, Rust -702 (+75, mostly signature updates at call sites), built-in JS/TS -130, codegen TS -75 (+13), Cargo manifests -459, `Cargo.lock` -436. **simdutf_sys** (`simdutf.rs`, `bun-simdutf.cpp`, `parsers/benches/support/simdutf_shim.cpp`): 16 shim chains with no caller, each removed as Rust wrapper + `extern` declaration + C++ definition: `simdutf__convert_utf8_to_utf16le`, `_utf16be`, `_utf16be_with_errors`, `convert_utf8_to_utf32_with_errors`, `convert_valid_utf8_to_utf32`, `convert_utf16be_to_utf8_with_errors`, `convert_valid_utf16be_to_utf8`, `convert_utf32_to_utf8_with_errors`, `convert_valid_utf32_to_utf8`, `convert_utf32_to_utf16be_with_errors`, `convert_valid_utf32_to_utf16be`, `convert_utf16be_to_utf32_with_errors`, `convert_valid_utf16be_to_utf32`, `utf8_length_from_utf16be`, `utf32_length_from_utf16be`, `utf32_length_from_utf8`, plus the now empty `utf32` modules and the `be` wrappers. **ncrypto** (`ncrypto.h/.cpp`): `BignumPointer::isOne`, `X509View::ifRsa`, `X509View::ifEc`, `BIOPointer::NewFp`, `checkScryptParams`, `scrypt`, `pbkdf2`, `EVPKeyCtxPointer::setRsaMgf1Md`, `Rsa::encrypt`, `Rsa::decrypt` and the `RSA_Cipher` template, `Cipher::ForEach` with `CipherCallbackContext`/`array_push_back`, `NCRYPTO_REQUIRE`, `NCRYPTO_VERSION` and the version enum. **JSC / WebCore bindings**: `ZigGlobalObject`: `functionFulfillModuleSync` (and the `fulfillModuleSync` builtin name plus `$fulfillModuleSync` stub), `JSDOMFileConstructor_getter/_setter`, `navigatorObject`, `functionLazyNavigatorGetter`, `GlobalObject_getPerformanceObject`, `hasNapiFinalizers`, `jsFunctionNotImplemented`, `jsFunctionCreateFunctionThatMasqueradesAsUndefined`, `Zig__GlobalObject__getModuleRegistryMap`/`resetModuleRegistryMap`, `NodeVM*ModulePrototype()` accessors, `ZIG_GLOBAL_OBJECT_DEFINED`. `BunString.cpp`: `Bun__WTFStringImpl__ref`/`deref` definitions (Rust inlines these; #39618 removes the declarations). `JSBuffer.cpp`: the `JSValue`-name `validateOffset` overload and the three unused `jsBufferConstructorAlloc*WithoutTypeChecks` JIT operations. `NodeValidator`: `validateString(JSValue name)` and `validateOneOf(span<ASCIILiteral>)`. `ScriptExecutionContext`: `ensureOnMainThread`, `executionContext`. `napi.h`: `hasFinalizers`, `currentFinalizer`, `isVMTerminating`. `IDLTypes.h`: `IDLDate`, the `NullableTypeWithLessPadding` helpers and two includes. `BunProcess.cpp`: three unused `*CodeGenerator` aliases. `c-bindings.cpp`: `HNS_PER_SEC`, `NS_PER_HNS`, `HNS_PER_US`. `BunCommonStrings.h`: `ConnectionWasClosed`, `ec`, `ed25519`, `rsa`, `rsaPss`, `jwkDsa`, `jwkG`, `systemError`, `x25519`. `BakeAdditionsToGlobalObject.h`: the never-read `m_bakeGetAsyncLocalStorage` lazy property (the function is still installed directly) and the `LazyPropertyOfGlobalObject` alias. `JSBundlerPlugin.cpp`: the `JSBundlerPlugin__onVirtualModulePlugin` declaration, which has no definition. WebCore: `DeferredPromise::whenSettled` and the `PromiseFunction`/`BindingPromiseFunction` adapters, `JSEventListener::sourceURL/sourcePosition`, `Event::receivedTarget`, `toJS(PerformanceObserverCallback)` and `callbackData()`, `jsFetchHeaders_getRawKeys` (its only caller in `internal/http.ts` is removed too), stale forward declarations in `Performance.h`/`ResourceTiming.h`, and the commented-out `BINDING_INTEGRITY` blocks in 8 generated-style files. `node/crypto`: `JSPrivateKeyObjectConstructor` and `JSPublicKeyObjectConstructor` (4 files, superseded by `JSKeyObjectConstructor`). Bake: `BakeRegisterProductionChunk`, `BakeProdSourceMap`, `BakeProduction.h`, the `IncrementalGraph` log scope. **uSockets**: `us_poll_ext`, `us_loop_iteration_number`, `us_socket_is_tls`, `us_connecting_socket_get_loop`, `us_udp_packet_buffer_local_ip` / `bsd_udp_packet_buffer_local_ip`. **Rust**: `js_parser`: the six `StrictModeFeature` variants that are never constructed (and the `can_be_transformed` branch), `FnOnlyDataVisit::{class_name_ref, should_replace_this_with_class_name_ref, is_inside_async_arrow_fn}` with the `this` substitution path that was gated on the always-false flag (the `shadow_ref` arena cell becomes a plain `Ref`). `bundler`: `Linker::{resolver, hashed_filenames}`, `IS_CACHE_ENABLED`, `InputFileFlags::IS_PLUGIN_FILE`, `parse_task::Step::ReadFile`. `js_printer`: the write-only `Options::transform_only`. `bun_install`: `CacheBehavior`/`ManifestLoad` (every caller passed `LoadFromMemoryFallbackToDisk`, so the parameter and the memory-only branch are gone from `by_name`, `by_name_hash` and `by_name_hash_allow_expired`), `pub use patch_install as patch`. `webcore`: `ReadableStream::detach_if_possible` (empty) and the `global` parameter of `done()`, `BlobExt::{on_structured_clone_transfer, get_mime_type}`, six `StartTag` variants that no sink uses. `server`: `AnyRoute::ref_`, the write-only `OPENED_BIT`. `bun_core`: `concat`, `ExternalShared::as_ptr`, `QuoteEscapeFormatFlags::ascii_only`. `bun_io`: stale `Waker`/`Closer` re-exports. `bun_sys`: `UTIME_OMIT`. `cli`: the never-read `IS_MAIN_THREAD` thread local. `css`: `DeclarationContext::Keyframes`, the empty `generated_color_conversions` module. `html_rewriter`: the `EndTag.replace` host function that `html_rewriter.classes.ts` does not expose. **Built-in JS/TS**: `internal/http.ts`: 29 unused symbol constants, `filterEnvForProxies`, `getRawKeys`, `emitCloseNTAndComplete`, `ClientRequestEmitState`. `node/http2.ts`: `kSettingNames`, three unused primordials. `internal/sql/query.ts` and `internal/repl/node-shims.js`: export entries nothing imports, and the `BuiltinModule` shim methods nothing calls. `builtins.d.ts`: 8 stubs for builtin names that no longer exist. **bindgen** (`src/codegen/bindgen*.ts`): `allFunctions`, `ArgStrategyChildItem`, `Variant.argStruct`, `Struct.namespace`/`toString`, `FuncMetadata`/`exposedOn`/`ExposedOn`, `FuncWithoutOverloads`, the dead `debug` binding, two shadowed duplicate `case` labels and an unreachable `return`. Generated output is byte-identical. **Cargo**: 459 dependency lines across 66 manifests (mostly the `strum`/`bstr`/`scopeguard`/`const_format`/`enum-map`/`enumset`/`libc`/`bitflags` boilerplate block, plus 94 `bun_*` edges such as `bun_jsc -> bun_simdutf_sys` and `bun_bundler_jsc -> 8 crates`). One dev-dependency (`bun_router -> bun_js_parser`, checked with `cargo check -p bun_router --tests`). The manifests that #39618 and #39697 already edit (`collections`, `io`, `paths`, `css`, `shell_parser`, `sql`, `sql_jsc`) were left alone. **Rebase note**: main restructured the private builtin function registration in `ZigGlobalObject::addBuiltinGlobals` into a table (#39770). The conflict was resolved by dropping the `k_fulfillModuleSync` row from the new table, which is the same registration the first version of this PR removed. #39770 also touched the two `JS*KeyObjectConstructor.h` files before this PR deletes them; they are still unreferenced on main, so the deletion stands. Re-verified after the rebase: `bun bd`, `rust:check-all` 12/12, and the test files listed above. **Follow-ups once open PRs land** (not done here to avoid duplicating them): after #39618: the C++ definitions of `URL__fromJS` (BunString.cpp) and `Bun__allocUint8ArrayForCopy` (ZigGlobalObject.cpp), the seven `<Sink>()` constructor accessors in `ZigGlobalObject.h` that only the generated `__getter` functions use, the Rust `extern` declarations of `Bun__WTFStringImpl__ref/deref`, and `BufferWriter::append_null_byte` (no writer ever sets it to true). After #39697: the root `[workspace.dependencies] typed-arena` entry. Independently of those: the `DeferredPromise::{promise, resolve(), reject(...)}` overloads and `DOMPromise::whenPromiseIsSettled` have no callers but sit next to code #39618 edits. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_edgecase.test.ts <!-- robobun:evidence:end -->
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.
Removes 1,279 lines of verified-dead code across the codegen scripts, orphaned developer scripts, and several Rust crates and built-in JS modules. Net -1,162 lines.
What was removed
src/codegen (bindgen) - the dead Zig emission machinery (~450 lines)
Commit d451445 ("Remove the .zig porting-reference sources") stopped writing
GeneratedBindings.zig, but left all the code that populated thezig/zigInternalbuffers. Everything feeding those never-flushed buffers is now gone:bindgen.ts: thezig/zigInternalCodeWriters and preambles,emitZigStruct,zigTypeName/zigTypeNameInner,returnStrategyZigType,emitNullableZigDecoder/emitComplexZigDecoder, the whole per-variant Zig dispatch emission loop, and the Zig wrapper/extern emissionbindgen-lib-internal.ts:Struct.emitZig, the write-onlyallFunctionslist, thezigPrefix/zigMappedNamefields, the unusedsnakehelperbindgen-lib.ts:t.zigEnum(zero callers; no.zigfiles exist in src/ for it to resolve against), the never-readexposedOn/ExposedOnandimplNamespaceoptions, plus thezigEnumbranches across all three filesfmt_jsc.bind.ts: theimplNamespaceoption (only ever fed the deletedzigPrefix), with the matching sweep ofdocs/project/bindgen.mdxclass-definitions.ts:isEventEmitter, read by nothing in generate-classes.ts and set by no.classes.tsVerified by running
bun src/codegen/bindgen.tsbefore and after: the generatedGeneratedBindings.cppand everyGenerated*.hare byte-identical.Orphaned scripts (~800 lines)
Zero references from package.json, .buildkite/, .github/, docs, or other scripts; last touched ~a year ago:
scripts/gamble.ts(manual flaky-test retry harness)scripts/github-metrics.ts(release metrics via gh CLI)scripts/debug-coredump.ts(Buildkite coredump helper)scripts/lldb-inline.sh+scripts/lldb-inline-tool.cpp(self-contained LLDB tool pair), plus their.gitignoreentryC++
src/jsc/bindings/v8/v8config.h: a 3-line include shim nothing includes; its stated purpose no longer holds since the node headers dir is on the include path directlyRust
src/url/lib.rs: theQueryStringMap::name_countremnant - a write-only field, an unusedNAME_COUNT_BUFthread-local (self-described as unused), and the commented-out Zig port body that referenced them, plus the then-unusedRefCellimportsrc/bun_core/string/immutable.rs:QuoteEscapeFormatFlags.ascii_only, write-only; theDisplayimpl hardcodesfalse(all four construction sites use..Default::default())src/runtime/dns_jsc: theInternalDNSRequestalias forinternal::Request(zero users; the type stays reachable via theinternalre-export)src/runtime/napi/napi_body.rs: a stale commented-out Rust stub fornapi_get_property_names, which is implemented in C++ (napi.cpp) and exported via symbols.txtBuilt-in JS
src/js/internal/sql/query.ts: fivesymbolsexport entries (_resolve,_reject,_queryStatus,_handler,_flags) no requirer destructures; the backing Symbols stay as private class keysVerification
build/debug/codegen/output before removalbun src/codegen/bindgen.tsoutput is byte-identical before/afterbun bddebug build passesbun run rust:check-all: 10/10 target triples passtest/js/sql/sqlite-sql.test.ts,test/js/node/quic/quic-endpoint.test.ts,test/js/bun/util/filesystem_router.test.ts, and the vendoredtest/js/node/test/parallel/test-quic-internal-endpoint-*.mjspair (the one failure, "properly finalizes prepared statements", times out identically on a debug build of main and passes in release, so it is unrelated)test/internal/source-lints/dead-symbols-bindgen-scripts-misc.test.tspinning the removals, following the existing dead-symbols lint pattern; its checks on modified files read the working tree, so the lint fails while the symbols are present and passes once they are goneFour candidates flagged by the initial scan turned out to be live and were kept:
bun_semver::String::EMPTY(used byCatalogMap.rs),Progress::Unit::Files/Bytes(assigned bypack_command.rs/upgrade_command.rs), the dns/napi re-export lists (load-bearing for-D dead-code/-D unreachable-pubon FFI-mirror items even with zero external callers), and theinternal/quic/quicstate getters (reached by vendored Node tests via--expose-internals, caught by CI and restored). Only the provably-dead subset shipped.[review] gate passed · iteration 3 · 20 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 3 rejected · iteration 3
evidence per changed file