Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Updated 6:50 PM PT - Aug 22nd, 2026
❌ @robobun, your commit d9d4acf has 2 failures in
🧪 To try this PR locally: bunx bun-pr 39618That installs a local version of the PR into your bun-39618 --bun |
|
Review follow-up, in two small commits on top of the sweep:
Debug build, rustfmt, clang-format, and the webcrypto tests pass on the new head. |
|
b5d7c78 follows up on the second review note: CI on the previous head (build 101099) had one red test, |
There was a problem hiding this comment.
I reviewed this PR across three passes and the two incomplete-removal issues I flagged earlier (orphaned us_nq_spec_peer_ctx/Bun__NodeUtil__jsParseArgs/root(CryptoKey*) counterparts, and the IDLNull/hasNullType dispatch) are now addressed in 4576d79 and b5d7c78. No further issues found on the current head. Given the breadth — 230 files across JSC FFI glue, WebCore converter templates, GC-adjacent changes (JSCommonJSExtensions base-class swap), the QUIC connect signature, codegen output, and the [bundle].packages bunfig behavior drop — a human pass is still warranted.
What was reviewed: the Bun__deepEquals<false,false,false,false> explicit instantiation replacing the removed implicit one; JSCommonJSExtensions → JSNonFinalObject (no remaining C++ fields, visitChildren/dtor correctly removed); the us_quic_socket_context_connect signature change and its Rust caller in h3_client/ClientContext.rs; TaskSource renumbering (only consumer ignores the value); jest.classes.ts call: true removal against the retained Rust call methods.
Extended reasoning...
Overview
This is a ~7,800-line dead-code sweep across 230 files: 74 JSC__*/WebCore__* C-ABI shims and their Rust callers in bindings.cpp/headers.h, unused WebCore IDL converter templates and ENABLE(BINDING_INTEGRITY) blocks, usockets/QUIC helpers with signature changes propagated to Rust callers, the 2,300-line mime_type_list.txt data file, the write-only [bundle].packages bunfig parser, orphaned scripts, and codegen output nothing installs. Two follow-up commits addressed my earlier incomplete-removal findings.
Security risks
None identified. The removals in CryptoUtil.cpp (keyFromString, passphraseFromBufferSource, parseKeyFormat/parseKeyType) and JSVerify.cpp (keyFromPublicString) are unreferenced helpers — the live crypto paths use different entry points. us_listen_socket_find_server_name_userdata and the SNI user field are removed but userdata lookup still works via SSL_CTX_set_ex_data (the comment is updated to reflect this is now the only path). No auth, permission, or input-validation logic is weakened.
Level of scrutiny
High. Although each hunk is a deletion, the surface area spans the JSC FFI boundary (where a missed caller is a link error at best and a silent behavior change at worst), a GC base-class change (JSDestructibleObject → JSNonFinalObject), a C signature change with a coordinated Rust caller update, codegen script edits that affect emitted C++, and one intentional user-visible behavior change (bunfig [bundle].packages no longer validates). The PR description's zero-reference verification and rust:check-all on 12 targets are good signals, but the JSCommonJSExtensions subspace/heap-cell-type interaction and the template-instantiation replacement in bindings.cpp are the kind of thing a maintainer should eyeball.
Other factors
Both rounds of my prior feedback were addressed promptly with dedicated commits and re-verification. The bug-hunting system found nothing on b5d7c78. CI on the previous head was green except for a pre-existing Windows shutdown flake the author cross-referenced against main. The PR carefully avoids overlap with four open dead-code PRs and documents the six shared files. Test coverage is exercised via the listed suites rather than new tests (appropriate for pure deletions). The scale alone puts this outside auto-approval bounds.
|
Thanks. One note on the Current state for review: head b5d7c78, zero open review threads, build 101109 has 178 of 179 jobs green with the last one still running. The one failing test it has shown so far, |
There was a problem hiding this comment.
Both earlier findings (the three orphaned counterparts and the IDLNull/hasNullType pair) are addressed in 4576d79 and b5d7c78, and this pass found nothing new. Given the size (230 files, −7.7k lines) and the handful of hunks that are structural rather than pure deletion — the JSCommonJSExtensions base-class change, the us_quic_socket_context_connect signature change, TaskSource/EventListener::Type renumbering, and the [bundle].packages bunfig removal — a human pass is still worthwhile.
What was reviewed:
JSCommonJSExtensionsJSDestructibleObject→JSNonFinalObject: header and .cpp are consistent (no leftover destructor,visitChildren, orm_registeredFunctions);subspaceForImplpicks the cell type from the base.ArrayBuffer::allocUint8Arrayarm removal: all three callers passJSType::ArrayBuffer.Bun__deepEquals<false,false,false,false>explicit instantiation replaces the oneJSC__JSValue__deepEqualsprovided as a side effect.IDLNull+ unionhasNullTypebranch removed together in b5d7c78;ConditionalConverterretained for the remaining branches.
Extended reasoning...
Overview
Dead-code sweep across ~230 files: JSC C-ABI shims in bindings.cpp/headers.h, unused WebCore IDL converters and ENABLE(BINDING_INTEGRITY) blocks, usockets/QUIC accessors and the write-only user fields, orphaned scripts, the 2.3k-line mime_type_list.txt, commented-out builtin JS blocks, and assorted Rust-side wrappers, re-exports, and Cargo dep edges. Two rounds of prior review found orphaned counterparts (fixed in 4576d79) and the IDLNull half-removal (fixed in b5d7c78); this run found nothing further.
Security risks
None identified. Pure removals of unreferenced code plus a few mechanical signature/enum adjustments. No auth, crypto behavior, input validation, or trust-boundary changes; the CryptoUtil deletions are unreferenced helpers.
Level of scrutiny
High, on volume alone. Most hunks are mechanical, but several are not pure deletions: JSCommonJSExtensions changes GC subspace via a base-class swap, us_quic_socket_context_connect drops a parameter with the h3 client caller updated in lockstep, TaskSource and EventListener::Type enumerator values shift, CastedThisErrorBehavior::RejectPromise loses its IDLAttribute::get handling, and [bundle].packages in bunfig stops validating. The description covers each of these, and I spot-checked the ones above, but the breadth argues for a maintainer skim.
Other factors
CI on the latest heads is green modulo pre-existing flakes the author called out and cross-referenced against main. The description enumerates every removed symbol, tracks overlap with four open PRs, and lists follow-ups deliberately left for those PRs. The vendored llhttp/api.c edits remove the wasm block and getter shims Bun never calls; touching vendored code is a policy call for a maintainer.
|
Two of the structural items listed above, checked against the tree for the human pass:
On llhttp: the copy under Retrigger build 101124 is at 166/179 green with no failures so far. |
91e0c76 to
b682107
Compare
|
Rebased onto current main (24c0063), force-pushed as b682107.
On the rebased tree: debug build, |
|
910417c follows up on the post-rebase review note: the Build 101157 on the rebased head was 178 of 179 jobs green. Its one red test is |
There was a problem hiding this comment.
I reviewed this PR again after 910417c and the bug-hunting pass found no new issues — the three earlier orphaned-counterpart findings are all addressed. Given the scale (231 files, −7.7k lines) and the handful of structural changes mixed in with the deletions, a human pass is still warranted.
What was reviewed:
- The
JSCommonJSExtensionsbase-class change (JSDestructibleObject→JSNonFinalObject) —m_registeredFunctionsand its visitor were the only C++ state, andsubspaceForImplpicks the heap cell type from the base, so no separate subspace edit is needed. - The
us_quic_socket_context_connectsignature change andsni_node_t.userremoval — the h3 client caller and the SNI userdata path (now viaSSL_CTX_set_ex_dataonly) are updated in lockstep. IDLAttribute::get'sRejectPromisebranch andCastedThisErrorBehavior::ReturnEarly— no attribute getter in the tree or codegen output passes either, so the removedif constexprarms could not be instantiated.- The
ErrorCode.tsrow removals — spot-checked that codes on both sides of the removed rows still resolve by name after regeneration.
Extended reasoning...
Overview
Dead-code sweep across 231 files (net −7712 lines): 74 orphaned JSC__*/ZigString__*/WebCore__* C-ABI shims in bindings.cpp/headers.h and their Rust-side declarations; unused WebCore IDL converters, ENABLE(BINDING_INTEGRITY) blocks, and whole headers (JSDOMConvertPromise.h, JSDOMConvertNull.h, JSErrorHandler.{h,cpp}, JSCInlines.h, v8config.h, sizegen.cpp); unused usockets/QUIC entry points and struct fields with a signature change to us_quic_socket_context_connect; the 2.3k-line mime_type_list.txt data file; a JSCommonJSExtensions base-class change dropping the unused m_registeredFunctions vector; the [bundle].packages bunfig parser; 14 llhttp API entry points; commented-out blocks in built-in JS; five orphaned scripts; and assorted Rust helpers, re-exports, and Cargo dep edges. Three follow-up commits (4576d79, b5d7c78/e257a125, 910417c) addressed orphaned counterparts my earlier passes flagged.
Security risks
None identified. This is deletion of unreachable code. The one security-adjacent edit — dropping the user field from sni_node_t in openssl.c — leaves the SNI userdata lookup path intact via SSL_CTX_set_ex_data/SSL_CTX_get_ex_data, which was already the mechanism per-socket lookup used. No auth, crypto verification, or permission checks are weakened.
Level of scrutiny
High. Although each individual deletion is mechanically simple, the PR is very large and touches several critical areas: JSC GC-visited C++ (JSCommonJSExtensions changes base class and drops visitChildren), C ABI in usockets/QUIC (a struct field removal and a function signature change with the sole caller updated), WebCore IDL converter templates whose only correctness proof is that no instantiation reaches them, and the ErrorCode.ts codegen input where row removal renumbers every subsequent generated constant. The description's verification (whole-tree token scan, rust:check-all on 12 targets, targeted test runs, regenerated codegen) is thorough, and the three rounds of orphaned-counterpart feedback were each addressed with a rebuild and re-verification. But at this scale a human should confirm the higher-risk structural items (particularly the GC base-class change and the QUIC connect signature) and spot-check a sample of the FFI-shim removals against main.
Other factors
- Three prior automated review rounds each found one incomplete-removal class; all were fixed and re-verified. This run found nothing.
- Two acknowledged behavior changes:
[bundle].packagesin bunfig is no longer validated (was write-only, undocumented), andTaskSourceenumerators are renumbered (only consumer ignores the value). - CI on the previous head was green except for pre-existing flakes the author called out; build 101173 is running on the current head.
- The PR explicitly avoids overlap with four open dead-code PRs and lists the seven shared files where hunks are non-overlapping.
|
Noted, thanks. One precision for the human pass: State for review: head 910417c, zero open threads. Build 101173 finished with 178 of 179 jobs green. The one red test is |
910417c to
1205aec
Compare
|
Rebased again onto current main (f8d486a), force-pushed as 1205aec. One conflict: #39633 deleted |
) ### Problem - 92 hand-written trait impls (`Default`, `From`, `Clone`, `PartialEq`) across 29 crates have no caller. rustc's `dead_code` lint exempts trait impls, so the workspace lints never report them. - A few items existed only for those impls: two tables in `src/resolver/options.rs`, `Flags::DEFAULTS` in `src/md/types.rs`, and five error variants that only a removed `From` impl constructed. ### Fix - Delete the impls and the items that became unused. 80 files, 1064 lines removed. The one added line is a doc comment with its last sentence cut. - The candidates come from the linker: a relink of the debug build with `--gc-sections --print-gc-sections` lists the trait methods dropped from every codegen unit. Impls that a bound or a supertrait still needs (`Deref` under `DerefMut`, `PartialEq` under `Ord`, `From<RGBA> for SRGB`) do not compile without the impl, so they stay. - Verified: `bun run rust:check-all` (11 targets), `bun run rust:clippy`, `cargo build --tests` for the 21 touched crates with unit tests, `cargo miri test` for the touched miri crates, `bun bd`, and 2112 tests across css, md, shell, env, resolve, node:fs, console.table, archive, http2, build errors, install, and compile sourcemaps. ### Background - To rustc a trait impl is always reachable: generic code could call it through the trait. Only a whole-program view shows which impls nothing calls. - The debug build uses `-ffunction-sections` at opt-level 0, so a function that nothing references is its own section and is not inlined away. `--gc-sections` drops exactly those sections. A method that survives in any codegen unit (an `#[inline]` copy, for example) counts as live. - The removed impls are listed below. <details><summary>Removed impls by crate</summary> - `src/runtime` (19): `Default for Chmod`; `Default for ExecCfg`; `Default for FChmod`; `Default for FetchOptions`; `Default for FrameHeader`; `Default for Framework`; `Default for FromJSOptions`; `Default for GzipOptions`; `Default for ID`; `Default for InitOptions<'a>`; `Default for IntoArray`; `Default for JSS3Error`; `Default for MkdirTemp`; `Default for Open`; `Default for ReactFastRefresh`; `Default for RmDir`; `Default for Stat`; `From<CreatePtyError> for crate::Error`; `From<InitError> for crate::Error` - `src/react_compiler` (10): `Clone for GlobalRegistry`; `Clone for ShapeRegistry`; `Default for Environment`; `Default for ExhaustiveEffectDepsMode`; `Default for ScopeBlockTraversal`; `From<&str> for JsString`; `From<String> for JsString`; `From<f64> for FloatValue`; `PartialEq<&str> for JsString`; `PartialEq<str> for JsString` - `src/ast` (8): `Default for ClassStaticBlock`; `Default for CommonJSNamedExport`; `Default for Dependency`; `Default for MetadataResolve`; `Default for NamedImport`; `Default for Query`; `From<SetError> for crate::Error`; `PartialEq for Data` - `src/jsc` (6): `Default for BuildMessage`; `Default for CPUProfilerConfig`; `Default for Column`; `Default for ResolveMessage`; `Default for RuntimeTranspilerCache`; `Default for Store` - `src/resolver` (6): `Clone for DependencyMap`; `Default for BundleOptions`; `Default for EntryCache`; `Default for ExtensionOrder`; `Default for ExtensionOrderGroup`; `Default for Package<'_>` - `src/bun_core` (4): `Clone for SmolStr`; `Default for CtorOptions`; `Default for DeserOpts`; `From<crate::Error> for JsError` - `src/install` (4): `Default for Entry`; `Default for MoreInstructions<'_>`; `Default for Scratch`; `PartialEq<crate::Error> for ForManifestError` - `src/css` (3): `PartialEq for CustomPropertyName`; `PartialEq for KeyframesName`; `PartialEq for LayerName` - `src/md` (3): `Default for BlockHeader`; `Default for Flags`; `Default for Theme<'a>` - `src/parsers` (3): `Default for JSONOptions`; `From<AddToLogError> for crate::Error`; `From<bun_alloc::AllocError> for PErr` - `src/uws_sys` (3): `Default for ListenConfig`; `Default for VTable`; `Default for us_socket_stream_buffer_t` - `src/bun_alloc` (2): `Default for MaxHeapAllocator`; `Default for Mutex` - `src/bundler` (2): `Clone for OutputFile`; `Default for BunLogOptions` - `src/install_types` (2): `Default for TarballInfo`; `From<CreateMatcherError> for FromExprError` - `src/js_parser` (2): `Default for ReactFastRefresh`; `Default for ThenCatchChain` - `src/sourcemap` (2): `Default for Chunk`; `Default for Mapping` - `src/base64` (1): `Default for VLQ` - `src/collections` (1): `Default for StringMap` - `src/dns` (1): `Default for Backend` - `src/dotenv` (1): `Default for Map` - `src/io` (1): `Default for FilePoll` - `src/paths` (1): `From<Error> for crate::Error` - `src/picohttp` (1): `Default for Response<'a>` - `src/ptr` (1): `Default for WeakPtrData` - `src/resolve_builtins` (1): `Default for Alias` - `src/s3_signing` (1): `Default for SignQueryOptions` - `src/sql_jsc` (1): `From<FlushQueueError> for crate::Error` - `src/sys` (1): `Default for Address` - `src/threading` (1): `Default for ThreadPool` Follow-up removals that became unused with them: - `src/resolver/options.rs`: the `EXTENSION_ORDER` / `MODULE_EXTENSION_ORDER` copies and the `TargetMainFields` / `DEFAULT_MAIN_FIELDS_*` tables (only `Default for BundleOptions` read them; the bundler projects these options itself). - `src/md/types.rs`: `Flags::DEFAULTS` (only `Default for Flags` read it; `root.rs` builds `Flags` field by field). - `src/parsers/xml.rs`: the `PErr::Oom` variant and its match arm (only the removed `From<AllocError>` produced it). - `src/css/rules/keyframes.rs`, `src/css/rules/layer.rs`: the `Hash` and `Eq` impls that went with the removed `PartialEq` impls, and the two comments that described them. The `ShapeRegistry` and `GlobalRegistry` docs in `src/react_compiler/hir/` lose the sentence about cloning for the same reason. Neither type is used as a map key; `LayerName` comparisons go through the inherent `eql`. - `src/ast/stmt.rs`: `impl Eq for Data`. - Error variants whose only constructor was a removed `From` impl, with their `name()` arms: `bun_runtime::Error::TerminalInit`, `bun_ast::Error::Clobber`, `bun_paths::Error::MaxPathExceeded`, `bun_sql_jsc::Error::AuthenticationFailed`. The inner error types (`InitError`, `SetError`, `path_options::Error`, `FlushQueueError`) are still live on their own. </details> <details><summary>Notes</summary> - `bun_core::strings::rsplit_once` has no caller, but `clippy.toml` names it as the replacement for `str::rsplit_once`, `str::rsplitn` and `bstr::ByteSlice::rsplit_once_str`, so it stays. Policy files are one kind of reference the linker does not see. No other removed item is named in `clippy.toml`, `hawk.toml`, or the docs. - Three impls came back during verification and are not in the diff: `Semaphore: Default` (used by the macOS `fs_events.rs`, found by `rust:check-all`), `clap::Help: Default` and `braces::Token: PartialEq` (used only by unit tests, found by `cargo build --tests`). The cross-target and test builds are needed for this kind of sweep: the linux binary alone does not see them. Scope of this sweep and what came back clean, so the next run can skip it: - Seven dead-code PRs are open right now (#37181, #37659, #39581, #39582, #39618, #39697, #39732). This PR deletes no line that any of them deletes (checked by diffing the removed lines). `src/jsc/headergen/sizegen.cpp` is dead too, but #39618 already removes it. - Flipping every `pub` to `pub(crate)` in `bun_runtime` (349k lines) and letting rustc report leaves 53 items, nearly all Windows-only or FFI enum tags. The same two-round flip over the other 95 crates found only macro-generated tables and items that are used from other platforms or from macro bodies. - The gc-sections list for C++ is almost entirely covered by the open PRs. What is left is small: 15 `JSFoo::toWrapped` stubs, a few `ncrypto` helpers, a handful of unused overloads. - `src/js/internal/**` (111 files) and the remaining `src/js/node`, `builtins`, `bun`, `thirdparty` files (86 files) were scanned export by export: nothing dead beyond a few single lines (`WASI.prototype.getState/setState`, three unused WASI constants). - No orphan files: every `.rs` is mounted, every `.cpp` is compiled, every `.h` is included. No stale `cfg` names (the build emits no `unexpected_cfgs` warnings). - Left alone on purpose: `src/runtime/api/bun/h2/` (in-progress rewrite, `#![allow(dead_code)]` by design), `src/react_compiler/compile_result.rs` (marked as not yet wired), the MySQL `CharacterSet` collation table (222 unused constants, a code table like the ones `hawk.toml` keeps), `Sys`/`Alloc` error variants that follow the crate `Error` convention, and the `JsPoster` clone vtable slot. - Possible larger follow-up for a maintainer decision: `PerformanceResourceTiming::create` and the `ResourceTiming` / `NetworkLoadMetrics` / `PerformanceServerTiming` constructors are unreferenced, so no such entry can ever exist. The JS classes stay reachable as globals, which is why about 1.8k lines of `src/jsc/bindings/webcore/*Timing*` survive. Removing them changes the global surface, so it is not in this PR. </details> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1205aec to
3383db8
Compare
|
Rebased a third time, onto 56c4e3d, force-pushed as 3383db8. Two things changed:
Nothing else main added references anything removed here (checked by scanning main's added lines for every removed name, and the deleted files' names). One caveat for CI: main at 56c4e3d does not build by itself ( |
3383db8 to
c91d2ac
Compare
|
Main's build break is fixed (#39839), so this is rebased onto 18bf288 and force-pushed as c91d2ac. No conflicts this time, nothing main added references anything removed here, and the build plus the lint, require-extensions, and the new serve abort test pass locally. Diff unchanged at -5401 across 228 files. CI is running on a buildable main again. |
|
The description note about CI on this head (build 102044, the first on a buildable main): 177 of 179 jobs green. The two failed jobs are the asan test lane, whose artifact download timed out before any test ran, and ubuntu aarch64, where |
…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 -->
#39618 removes the rest of the DOMPromise class. Deleting this last member here would leave the two pull requests fighting over JSDOMPromise.h, and the header and JSDOMPromise.cpp can be deleted together once both have landed.
|
Heads-up from #39925: it adds |
…uilt-in JS, and orphaned scripts Delete code that nothing references any more: - 74 JSC__*, ZigString__*, Bun__* C-ABI glue functions in bindings.cpp whose Rust callers are gone, plus 30 stale prototypes in headers.h. - http_types/mime_type_list.txt, a data file no build step reads. - Dead WebCore binding helpers: the JSDOMConvert* converters nothing instantiates, ExceptionOr/ExceptionCode helpers, HTTPHeaderNames tables, the BINDING_INTEGRITY vtable blocks (the macro is never defined), JSErrorHandler, JSDOMBindingInternalsBuiltins, and unused overloads in CryptoUtil, JSMIMEType, InspectorHTTPServerAgent, PerformanceResourceTiming and others. - Unused llhttp API entry points, usockets and uws_sys functions, and the write-only user pointer of the QUIC connect path. - Rust wrappers, exports and link-interface members with no callers, the unused [bundle].packages bunfig cluster, and eleven unused Cargo dependency edges. - Commented-out blocks in CommonJS.ts, child_process.ts, url.ts and shell/subproc.rs, and the unused setBigUint64 Buffer builtin. - src/jsc/headergen/sizegen.cpp (a Zig-era generator nothing runs) and five developer scripts with no references.
us_nq_spec_peer_ctx lost its C definition, Bun__NodeUtil__jsParseArgs lost its exported symbol, and root(CryptoKey*) lost its declaration and its only caller. Remove the Rust extern, the C++ declaration, and the definition that each left behind.
Only the unused extern declarations go. The comment still describes why the Rust port exists.
Converter<IDLNull> went away with JSDOMConvertNull.h. IDLNull existed only as a union member for that converter, and the hasNullType branch in the union converter was its only user. No union in the tree lists IDLNull.
#39544 made the RESP null parser return InvalidNull, so it is no longer dead. The other four unconstructed variants stay removed.
Four of them lost their last user when the unconstructed RedisError variants went. The other five had no user before this change.
c91d2ac to
3b504c3
Compare
|
Rebased onto 8eb5b6e (99 commits, including the merged dead-code PRs #39582 and #39732), force-pushed as 3b504c3. One conflict: On the rebased tree: nothing main added references anything still removed here (scan of main's added lines for every removed name), the debug build and |
…tion Their only users were the converter specializations removed earlier in this change.
|
d9d4acf follows up on the post-rebase review note: |
There was a problem hiding this comment.
I reviewed this PR again after d9d4acf and found no bugs — all four orphaned-counterpart issues from earlier passes are addressed. Given the scope (226 files, ~5100 lines removed, cross-language FFI signature changes, the JSCommonJSExtensions base-class change, and the [bundle].packages bunfig behavior change), a maintainer sign-off is still warranted.
What was reviewed:
- The
JSCommonJSExtensionsJSDestructibleObject→JSNonFinalObjectchange: no remaining C++ fields, subspace selection follows the base persubspaceForImpl, and~JSCommonJSExtensionsis no longer declared. - The
us_quic_socket_context_connectsignature change: C definition, header, Rust extern, and the sole h3 client caller updated together. ArrayBuffer::alloc_for_copyUint8Arrayarm removal and theappend_null_byte = falsedrops injsc_hooks.rs— both match existing defaults, no callers affected.- The
PostgresSQLConnection__createInstanceandBun__NodeUtil__jsParseArgsexport removals — zero references remain in the tree.
Extended reasoning...
Overview
This is a large dead-code sweep: 226 files, net −5099 lines, spanning JSC FFI glue (bindings.cpp, headers.h), WebCore/node C++ bindings, usockets C, Rust crates across ~15 workspace members, built-in JS, codegen scripts, and orphaned developer scripts. The vast majority is pure deletion of symbols with zero remaining references, but a handful of hunks are structural: the JSCommonJSExtensions base class changes from JSDestructibleObject to JSNonFinalObject (with visitChildren and the destructor removed), us_quic_socket_context_connect drops a parameter across C/Rust/caller, sni_node_t drops a struct field, TaskSource enumerators are renumbered, and the bunfig [bundle].packages parser is removed (user-visible: an invalid value there no longer errors).
Security risks
None identified. This is deletion of unreachable code; no new input parsing, no auth/crypto logic changes (the crypto hunks remove uncalled helpers only), no new external surface. The one user-facing change — silently accepting a previously-validated-but-ignored bunfig key — is benign.
Level of scrutiny
High, and beyond what an automated pass can fully cover. The change is mechanically simple per-hunk but the aggregate risk is in the cross-language coupling: 74 C-ABI functions removed from bindings.cpp where a stale Rust extern or a codegen consumer would only surface as a link error (which the build catches) or a silent behavior change (which it doesn't). The JSCommonJSExtensions base-class change affects GC subspace selection and destructor semantics — the author's timeline note explains why subspaceForImpl handles it, and the debug build with JSC assertions would trip on a mismatch, but this is exactly the class of change REVIEW.md flags for careful GC review. The five deleted developer scripts and the llhttp trimming are judgment calls a maintainer should confirm.
Other factors
- Four prior review passes on this PR each found one orphaned-counterpart issue (deleted specialization leaving its primary declaration behind); all four were fixed promptly, and the current head d9d4acf is the fix for the last one. This run's bug hunter and my own read found no further orphans.
- CI on recent heads has been green modulo pre-existing main flakes the author documented per-build in the timeline.
- The author has been thorough about rebase interactions with concurrent PRs (#39544, #39770, #40018/#40055, merged #39582/#39732) and re-verified the reference scan after each.
- Spot-checks confirmed:
PostgresSQLConnection__createInstancehas zero tree references;append_null_bytedefaults tofalseso the dropped assignments are no-ops;Bun__allocUint8ArrayForCopy's C++ body remains (listed as a follow-up in the description) and the removed Rust match arm is compile-time dead.
Deferring rather than approving because the size, the FFI/GC-adjacent structural changes, and the deleted maintainer scripts put this outside what should merge without a human look.
|
Thanks. State for the maintainer pass: head d9d4acf, rebased onto current main, zero open review threads, net -5099 across 226 files. Build 103845 on this head finished 179 of 181 jobs green. Both failed jobs are the two macOS x64 test-bun shards, and both are network timeouts on that host: one shard's The two structural items a human may want to eyeball are the |
…JS, the bake dev server assets, the shell, and the SQL crates (#40066) Scheduled dead-code sweep. Net -3275 lines (78 files, +112 / -3387; the source side is +32 / -3387, the rest is the test pin). No behavior change: every item below has zero references on `main` in `src/`, `packages/`, `scripts/`, `test/` and the regenerated `build/debug/codegen/`, and the build passes without it. About 2,350 of these lines carry over the removals from #37659, which was closed only for merge conflicts. Each of those was re-verified against current `main` (nothing gained a caller), and the diff applies to current `main` with no conflicts. The rest comes from fresh scans of the shell, SQL, bake and built-in JS areas, which no open dead-code PR covers. ### bun-uws (`packages/bun-uws/src`, -1168) Bun reaches uWS only through the C shim in `src/uws_sys/` and eight bindings files, so those includers are the complete set of callers. Checked against every one of them: * Whole headers nothing includes: `ClientApp.h`, `HttpError.h` (superseded by `HttpErrors.h`), `Multipart.h` and `MessageParser.h` (its only includer), `ProxyParser.h` (only reachable under `UWS_WITH_PROXY`, which no build defines; `grep UWS_ build/debug/compile_commands.json` is empty), plus every `#ifdef UWS_WITH_PROXY` block and the `reserved` / `proxyParser` parameters that only carried the proxy parser into `getHeaders()`. * `WebSocketBehavior::subscription` and `WebSocketContextData::subscriptionHandler`: the only place a `WebSocketBehavior` is built (`uws_ws()` in the shim) never sets it, so every dispatch site in `WebSocket::end/subscribe/unsubscribe` and `WebSocketContext::onClose` was unreachable. `TopicTree::unsubscribe` drops the `newCount` only those sites read. * `HttpRequest`: the per-route `std::map` of parameter offsets (`setParameterOffsets`, `getParameter(name)`; every caller uses the index overload), `getQuery()` with no key, `isShortRead`, `notFieldNameWord` / `hasMore` / `hasBetween`. * `HttpContext::HTTP_IDLE_TIMEOUT_S`, `HttpFlags::isAuthorized` (the live flag is the per-socket one in `AsyncSocketData`), `HttpResponse::getHttpResponseDataS`, `HttpResponse::endWithoutBody`, `HttpResponse::overrideWriteOffset` (the shim calls `setWriteOffset`). * `App::publish(unsigned char)`, two `App::listen` overloads, the `UWS_NO_ZLIB` block; two `H3App::listen` overloads; `Http3Request::isAncient` / `getCaseSensitiveMethod`; nine `Http3Response` members whose `uws_h3_res_*` shims are stubs; the write-only `Http3ResponseData::totalSize`; `Loop::integrate`, `Loop::setSilent` and `LoopData::noMark`; the `UWS_NO_ZLIB` / `UWS_MOCK_ZLIB` mock streams and `UWS_USE_LIBDEFLATE` guards in `PerMessageDeflate.h` (the macro is defined unconditionally three lines above its first use). * `HttpContext::layoutAssert()` and `WebSocketContext::layoutAssert()` were never called, and as static members of class templates their `static_assert`s never ran. The `HttpContext` one guarded nothing. The `WebSocketContext` one guards the layout `WebSocket::getContextData()` depends on (`group` first, `data` right after), so it becomes a `RELEASE_ASSERT` in `create()`: `offsetof` cannot express it because the struct is not standard-layout, and the layout can differ per ABI, so the check runs once per context in every build. * `misc/` (upstream README assets, demo `cert.pem` / `key.pem`), and the `.gitattributes`, `.dockerignore`, `.cursorignore` entries for a `fuzzing/` directory that no longer exists. ### Native `JSBufferList` (`src/jsc/bindings`, -641) `JSBufferList.cpp` / `.h`, the lazy class structure on `GlobalObject`, its three accessors, and the `DOMIsoSubspaces` / `DOMClientIsoSubspaces` slots. The JS streams implementation replaced it; the only remaining mention of "BufferList" in the tree is a comment in `internal/streams/readable.ts`. The sources are globbed, so no build-script change is needed. ### Built-in JS (`src/js`, -508) * `internal/util/inspect.d.ts` (ambient types for `node-inspect-extracted`, which nothing imports) and `src/js/.gitignore` (ignores `src/js/out`, which codegen has not written since the build moved under `build/`). * `private.d.ts`: the `BunFS` / `BunFSWatcher` types (`Bun.fs()` no longer exists), `Bun.TOML`, `Bun.tty`. * `QuicStream.prototype[kSendHeaders]` and its symbol, the unused `get [kVerifyPeer]` (the setter stays; `session[kVerifyPeer] = …` writes it), `colors.ts` `yellow` / `gray` / `clear` / `reset`, the runtime `SSLMode` export of `internal/sql/shared.ts` (only imported as a type), the `REPL_MODE_*` re-exports of `internal/repl/utils.js` (`repl.js` takes them from `internal/repl/mode`), `Dequeue#clear()`, the write-only `listeningId` in `net.ts`, and the `kGetNativeReadableProto` symbol in `internal/shared.ts` (its last reader went in 032713c). ### Bake (`src/runtime/bake`, -830) * `incremental_visualizer.html` and `memory_visualizer.html`: #36184 removed the `runtime_embed_file!` sites and the `/_bun/incremental_visualizer` route that served them, so the two pages are orphaned assets. The no-op `emit_*_visualizer_*` methods that #36184 left behind are still reachable and are not touched here. * `client/websocket.ts` `getMainWebSocket()` (exported, never imported) and a commented-out Zig `writeJsValue` in `serialized_failure.rs`. ### Shell (`src/runtime/shell`, `src/shell_parser`, -51) * `Base::end_scope()`, an empty function, and its 12 call sites in `states/*.rs`. `Script::deinit_from_interpreter` only called it and goes too. * `ShellErr::InvalidArguments`: never constructed (only matched), with its four match arms. * `bun_shell_parser` dependencies `scopeguard`, `const_format`, `enum-map`, `enumset`, `typed-arena`, `bun_collections`: no `use` or path reference in the crate. ### SQL (`src/sql`, `src/sql_jsc`, -22) and `hawk.toml` (-104) * `bun_sql` dependencies `const_format`, `enum-map`, `enumset`, `libc`; `bun_sql_jsc` dependencies `const_format`, `enum-map`, `enumset`, `libc`, `bun_uws_sys`, and a commented-out `bun_runtime` edge that would be a dependency cycle (`bun_runtime` depends on `bun_sql_jsc`). * `pub use` re-exports nothing imports: `ArrayBuffer`, `JSCell`, `host_fn` in `sql_jsc/jsc.rs` (the proc-macro is used by its absolute path), `mysql::{MySQLConnection, MySQLQuery, MySQLStatement}` (every consumer imports the leaf module), `postgres::SASL`. * `hawk.toml`: 13 overrides for `bun_sql::mysql::status_flags::StatusFlag::*` variants that #37229 removed; `StatusFlag` has one variant left. ### Rust (-18) `zig__ModuleInfo__destroy` and `zig_log` in `src/bundler/analyze_transpiled_module.rs`: `extern "C"` exports that no C++ declares or calls (`BunAnalyzeTranspiledModule.cpp` frees through `zig__ModuleInfoDeserialized__deinit`), with no hit in `vendor/WebKit` either. ### Verification * `rg` for every symbol across `src/`, `packages/`, `scripts/`, `test/`, `docs/` and `build/debug/codegen/`; `.classes.ts` files, `src/codegen/`, string-named lookups (`$getByIdDirectPrivate(this, "…")`, `$zig` / `$cpp`) and token-paste macros (`name##PublicName`) were checked by hand. Two candidates failed those checks during the run and stay: `BunBuiltinNames` `mockedFunction` (reached through `BUN_COMMON_STRINGS_EACH_NAME`) and `writer` (reached through `$getByIdDirectPrivate(this, "writer")` in `ConsoleObject.ts`). * `bun bd` builds. `cargo check --workspace` passes on all 12 target triples. `cargo fmt --check`, prettier, oxlint and clang-format are clean. `tsc -p src/js` reports the same errors before and after (only line numbers move). * `test/js/bun/websocket/websocket-server.test.ts` gains a pin for the pub/sub paths this PR edits: with two sockets on shared topics, `unsubscribe()` of a topic the socket never joined (or that does not exist) reports false and changes nothing, leaving the last topic frees the subscriber and a later `subscribe()` counts again, and `close()` drops the socket from every topic without touching the other socket. Because this PR only deletes unreachable code, the pin passes before and after by design. * `bun bd test`: `bun/websocket/websocket-server.test.ts`, `web/websocket/websocket-permessage-deflate.test.ts`, `bun/http/bun-serve-routes.test.ts`, `bun/http/serve-http3.test.ts`, `node/http/node-http.test.ts`, `node/quic/quic-stream.test.ts`, `node/net/node-net-server.test.ts`, `bun/util/inspect.test.js`, six `bun/shell/*.test.ts` files, `sql/sql-mysql*.test.ts`, `sql/sql-mariadb-json.test.ts`, `sql/postgres-binary-numeric.test.ts`, `sql/sql-connect-error-reporting.test.ts`, `node/stream/node-stream.test.js`, `node/console/console-table-iterators.test.ts`, `web/console/console-log.test.ts`, `internal/fifo.test.ts`, `bake/dev/esm.test.ts`, `bake/dev/hot.test.ts`, `bun/repl/repl.test.ts` (mode tests), `node/test/parallel/test-repl-definecommand.js`. Details on the local failures that also fail on the released bun are under Notes. ### Looked at and left alone * `src/runtime/api/bun/h2/`: the outbound half (`Connection::send_header_block` / `send_data` / `send_push_promise`, `hpack::Coder::encode`, `SendWindow::available`) is only exercised by its unit tests. The module doc describes it as the engine that will replace `h2_frame_parser.rs`, so it is in progress, not dead. * The bake visualizer protocol residue: `MessageId::Visualizer`, the `IncrementalVisualizer` topic, and the empty `emit_*_visualizer_*` methods and their timer. All reachable; removing them is a protocol change that also touches the generated `generated.ts` ids. * `ColumnDefinition41::{fixed_length_fields_length, decimals}` are decoded and never read, but they mirror the MySQL wire struct; left in place. * `bun_sql::mysql::StatusFlags`'s `Display` impl prints nothing, but a debug log in `MySQLConnection.rs` still formats it. <details><summary>Notes</summary> How the candidates were found: the dead-code PRs that were closed for merge conflicts were re-applied against current `main` and every removal re-verified (#37659 applied with no conflicts; its `assertion_error.ts` and `util.ts` hunks had already landed via #39924 and #39628 and are not in this PR). Read-only scans of `src/runtime/shell` + `src/shell_parser`, `src/sql` + `src/sql_jsc` + `src/runtime/node`, `src/runtime/bake` + `src/runtime/api` + `src/runtime/image`, and `src/js` + `src/jsc/modules` produced the fresh items. `src/runtime/node`, `src/runtime/api` (outside `h2/`), `src/runtime/image` and `src/jsc/modules` came back clean. Local test failures, all reproduced with `USE_SYSTEM_BUN=1` (the released bun) in the same container, so none is caused by this diff: * `websocket-server.test.ts`: eight `it.concurrent` cases that overlap the 300,000-message `(benchmark)` test time out under the debug ASAN build in this container (the whole file takes 64 s here against a 6.4 s CI budget); each passes when run with `-t`. * `node-http.test.ts` "request via http proxy": `ECONNREFUSED` from `localhost` resolution in this container. * `bunshell.test.ts` `-c`, `-f character device`, and the seven "stdin redirect from a zero-length buffer" cases: this container's `/dev/null` is a regular file, not a character device. * `sql-mysql-binary-null-indexed.test.ts`: `Access denied for user 'root'@'localhost'` against the local MariaDB. * `fifo.test.ts` "pushing and shifting a lot of items": a 10 s perf budget exceeded under ASAN; the only `fifo.ts` change is the removal of the unused `clear()` method. Files shared with open PRs, different hunks: `src/jsc/bindings/ZigGlobalObject.cpp` / `.h` (#39581 removes other accessors; its `HTMLRewriterSinkPrototype()` hunk sits one blank line above the `JSBufferList` accessors removed here), `Cargo.lock` (#39618 edits other package blocks). </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 4 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/websocket/websocket-server.test.ts <!-- robobun:evidence:end -->
|
Conflicts |
|
Rebased onto a25c009 and force-pushed as c95c83e. The conflict was in the shell error enum: #40066 (another sweep that merged in the meantime) already removed |
2 participants
Dead-code sweep. Net -5062 lines across 225 files (93 insertions, 5155 deletions). Every item below has zero references left in
src/,packages/,scripts/,cmake, and the regeneratedbuild/debug/codegen/output. Nothing here overlaps with the open dead-code PRs #39581, #37181, and #37659 (#39582, #39732 and #40066 merged and this PR is rebased past them). This PR does not touch their files, except where the hunks are in different places (listed in the notes).Closes #9765 (the
sizegen.cppdeletion).Removed
JSC FFI glue (
src/jsc/bindings/headers.h,bindings.cpp, about -820)JSC__*,ZigString__*,WebCore__*, andBun__*C-ABI functions whose Rust callers are gone. Examples: the wholeJSC__JSInternalPromise__*family,JSC__JSValue__deepEquals,JSC__JSValue__createRopeString,JSC__VM__deleteAllCode,Bun__REPL__formatValue.Bun__deepEquals<false, false, false, false>gets an explicit instantiation because the removedJSC__JSValue__deepEqualswas the only thing that instantiated it forBunObject.cpp.Reader__*__fastpath,FFI__ptr__fastpath,JSC__VM__create,ZigException__fromException,Zig__GlobalObject__fetch, and others.VM::has_termination_request,Zig__GlobalObject__reportUncaughtExceptionandreport_uncaught_exception,Bun__tickWhilePausedandtick_while_paused,Bun__Timer__getNextID, theBun__ConsoleObject__{profile,profileEnd,record,recordEnd,screenshot}no-op exports,StringJsc::to_range_error_instance, theJSCommonJSExtensions__*imports and their C++m_registeredFunctionsmechanism,ZigString__free,AbortSignal__Timeout__run,Resolver__propForRequireMainPaths,DeprecatedStrong::unref, two host exports with no C++ caller, the unusedBun__NodeUtil__jsParseArgsC-ABI export and its declaration, androot(CryptoKey*), which nothing could reach once the genericaddWebCoreOpaqueRootoverloads were gone.WebCore and node bindings (about -1500)
JSDOMConvertPromise.h,JSDOMConvertNull.h,JSErrorHandler.{h,cpp},JSDOMBindingInternalsBuiltins.cpp,JSCInlines.h(every include uses theJavaScriptCore/header),v8/v8config.h, andheadergen/sizegen.cpp(a Zig-era generator nothing runs).JSDOMConvert{Strings,Callbacks,Numbers,Any,Object,Interface}.h,ExceptionOr.h,ExceptionCode.h,HTTPHeaderNames.h,WebCoreOpaqueRoot*.h,JSDOMPromise.h,TaskSource.h(its values are never read),IDLNullwith the union-converter branch that was its only user (no union in the tree lists it, and its converter left withJSDOMConvertNull.h), and theIDLCallbackInterfacetype andVariadicConverterforward declaration whose only users were the removed converter specializations.ENABLE(BINDING_INTEGRITY)vtable blocks in 13JS*.cppfiles. The macro is defined nowhere, and the assertion inside the blocks is commented out.CryptoUtil,JSVerify,JSMIMEType,InspectorHTTPServerAgent,PerformanceResourceTiming,ErrorStackTrace,FormatStackTraceForJS,BunPlugin(browser and node targets),BunProcess(Process_defaultSetter, aqueueNextTickoverload, the empty POSIXsignalHandler),JSBigIntBinding,StringBuilderBinding,RegularExpression,highway_json,SQLClient,c-bindings,napi.cpp(two internal helpers, not Node-API exports),NodeHTTPParser,NodeFSStatFSBinding,DOMFormData::toURLEncodedString, and the 14 unused llhttp API entry points inllhttp/api.c.usockets, uws_sys, http (about -200)
us_nq_spec_peer_ctx,us_socket_open,us_listen_socket_{ext,get_fd,port},us_socket_group_{next,timestamp},us_listen_socket_find_server_name_userdataand the SNIuserfield, the QUICon_opencallback,us_quic_stream_{flush,has_unacked},us_quic_socket_close, and the write-onlyuserpointer ofus_quic_socket_context_connect(C, Rust declaration, and the h3 client caller all updated together).uws_sys(App.rs,ListenSocket.rs,us_socket_t.rs,quic/*),libdeflate_sys,bun_url,bun_http,picohttp, and the four valkey protocol error variants nothing constructs (InvalidNullwas in this list until valkey: fix null array, null CRLF, big number and blob error replies #39544 started constructing it, so the rebase keeps it), plus the nineERR_REDIS_*rows inErrorCode.tsthat nothing maps to (four lost their last user with those variants, five had none before).Rust crates (about -450)
[bundle].packagesbunfig cluster (BundlePackage,package_bundle_map, the parser). The key was undocumented and nothing read the map, so an invalid value no longer reports an error.ErrnoNames::{max_dense,win32_name}link-interface members and theirbun_errnoproviders, thePARSER_ERRORsentinel statics,ShellErr::Todo,bun_sys::windowshelpers (translate_ntstatus_to_errnoduplicate,GetEnvironmentVariableError,FdOptional),bun_core::utilandfmt::rawhelpers,Timer::lap,options_typesre-exports, the duplicateConcurrentGroup::sequences_const,impl Default for StepResult, and visibility narrowing innode_fs.rsandMySQLTypes.rs.collections,io, andpaths(Cargo.lockupdated).Built-in JS, codegen, scripts (about -1100)
CommonJS.ts(the oldloadEsmIntoCjs),child_process.ts,url.ts, andshell/subproc.rs. The unusedsetBigUint64Buffer builtin (JSBuffer.cppnever installs it).generate-jssink.tsno longer emits thefunction*__gettercustom getters nothing installs.call: trueis removed from the sevenExpect*matcher classes injest.classes.ts. WithnoConstructor, it only generated thunks nothing called. The Rustcallmethods stay,expect.any()and friends use them directly.scripts/{gamble.ts,github-metrics.ts,debug-coredump.ts,lldb-inline.sh,lldb-inline-tool.cpp}and their.gitignoreentry. No reference frompackage.json,.buildkite,.github, or other scripts.Verification
test/is the removal of theno-iostream-includelint allowlist entry for the deletedsizegen.cpp.bun bddebug build passes.bun run rust:check-allpasses on all 12 targets.cargo fmt,clang-format, and prettier are clean on the changed files.test/internal/source-lints, require-extensions, abort, FormData, mime-api, console-log, performance, perf_hooks, websocket, broadcast-channel, crypto.key-objects, crypto-sign, sign-jwk, setTimeout, url, bunfig-test-options, node-http, serve, buffer, process, fs, shell (bunshell-default, exec), fetch-http3-client, serve-http3. The only failures are tests that fail identically with the unmodified release binary in this sandbox (no IPv6, running as root, no public network) and the timing-sensitive worker and setTimeout leak tests under debug ASAN, which also fail without this change.Every removed symbol that now has zero references in the tree (244)
Notes
Where the candidates came from. A token-frequency scan of the whole tree (every Rust
pubitem, extern declaration, C++ function, and TS export with no reference outside its own declaration) found the headers.h / bindings.cpp glue. The rest are items from the dead-code PRs that were closed as stale (#37012, #38005, #37208, #38439, #37454, #37788, #36115, #35775, #36237, #35559, #35437, #37062, #37089, #37272). Only hunks that still apply to current main were taken, and every one was re-verified against the current tree. The build and the reference scan rejected several of them because the code is live again. Those were dropped: the WebSocketDeflateOutOfMemoryvariants,jsFetchHeaders_getRawKeys,v8::Local::reinterpret, thejs_parserand bundler clusters,fulfillModuleSync, and the REPL shim andsql/query.tsedits.Files shared with open PRs.
src/bun_core/lib.rs,src/jsc/lib.rs,src/jsc/VirtualMachine.rs,src/runtime/shell/Builtin.rs,src/lsquic_sys/lib.rs,src/jsc/bindings/ZigGlobalObject.cpp, andsrc/jsc/bindings/ErrorCode.tsare also touched by #39581, #39582, #37181, or #37659. The hunks are in different places (this PR removesErrnoNamesmembers,StringJsc::to_range_error_instance,report_uncaught_exception, twoShellErrmatch arms, theus_nq_spec_peer_ctxextern, oneBUN_DECLARE_HOST_FUNCTIONline, and theERR_REDIS_*rows, which #39582 does not touch). All other files touched by those PRs were left alone.Follow-ups found but not removed here, because their files belong to an open PR or the removal is a refactor:
URL__fromJS,Bun__WTFStringImpl__{ref,deref}(BunString.cpp),Bun__allocUint8ArrayForCopy,Zig__GlobalObject__{get,reset}ModuleRegistryMap,functionFulfillModuleSyncand its builtin name (ZigGlobalObject.cpp),uws_app_listenanduws_app_run(libuwsockets.cpp).ScriptExecutionContext::ensureOnMainThreadand the freeexecutionContext(),ResourceTiming::populateServerTiming, and theJSPerformanceObserverCallbackhelpers (headers are in Remove dead code from C++ bindings, bindgen glue, ast, and orphaned scripts #39581).BINDING_INTEGRITYblocks inJSDOMFormData,JSMessageChannel,JSTextEncoder,JSPerformanceObserverEntryList,JSDOMURL, andJSSubtleCrypto, plus the commented-outqueueTaskKeepingObjectAliveblocks inWebSocket.cpp.StrictModeFeaturevariants nothing constructs and the write-onlyFnOnlyDataVisit::is_inside_async_arrow_fn(p.rsis in Remove dead code from C++ bindings, bindgen glue, ast, and orphaned scripts #39581). The unusedLinker::initparameters,InputFileFlags::IS_PLUGIN_FILE, andparse_task::Step::ReadFile(bundle_v2.rsis in Remove dead code from the bun_runtime re-export hubs, bun_core, bun_css, bun_install, bun_bundler, the FFI crates, and the error-code table #39582). The never-enabled CSS dependency collection andPseudoClassesprinter option (about 330 lines,css_parser.rsis in Remove dead code from the bun_runtime re-export hubs, bun_core, bun_css, bun_install, bun_bundler, the FFI crates, and the error-code table #39582).headers.hstill declaresJSC__VM__throwErrortwice and repeats theFileSinkblock.Rebases. Onto 24c0063:
highway_json_index(#39616 touched it, still no caller) stays deleted, andRedisError::InvalidNullis restored because #39544 constructs it. Onto f8d486a: #39633 deletedJSC__JSValue__getErrorsPropertynext to theJSC__JSValue__jsTDZValuedeleted here, both deletions kept. Onto 56c4e3d: #39770 madesrc/http_types/mime_type_list.txtthe input of the newmime_type_list.generate.ts, so this PR no longer deletes it and leavessrc/http_types/untouched.Bun__REPL__formatValuewas rewritten mechanically there and still has no caller, so it stays deleted. Onto 18bf288: no conflicts. Onto 8eb5b6e (99 commits, including the merged dead-code PRs #39582 and #39732): #40018 and #40055 rewrotewebsocket_client.rsaroundThisPtr, andBun__WebSocketClient__writeBlobis now aHOST_EXPORTwith a generated C++ caller, so this PR no longer touches that file (main's version is taken as is). Items the merged PRs already removed dropped out of the diff on their own. Onto a25c009 (#40066, another merged sweep, removedShellErr::InvalidArgumentsitself and leftShellErr::Todo, which is still never constructed, so onlyTodoremains removed here). After each rebase the build,rust:check-all, and a scan of the lines main added for every removed name were rerun (nothing new references removed code).Behavior notes.
[bundle].packagesin bunfig is no longer validated (it was never read).TaskSourceenumerators are renumbered, but the only consumer ignores the value. Everything else is the removal of unreachable code.