Skip to content

Remove dead code from bun_bundler, bun_install, bun_runtime, bun_parsers, and the JSC private host functions - #41088

Open
robobun wants to merge 2 commits into
mainfrom
robobun/14b4b44c/dead-code-sweep
Open

robobun wants to merge 2 commits into
mainfrom
robobun/14b4b44c/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Sep 1, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • These items have no caller, no reader, or no constructor anywhere in the tree. The workspace denies dead_code, so what remains is cross-crate pub items and private host functions that the lints cannot see. A relink of the debug binary with --gc-sections --print-gc-sections and a rebuild of the C++ with -Wunused-function -Wunused-template confirm the same set.
  • Each deletion was checked with a repo-wide search (src/, packages/, scripts/, build/debug/codegen/, .classes.ts, string literals) and against the deletions in every open dead-code PR.

Fix

  • Rust: drop never-constructed enum variants and never-read payloads. DependencyToEnqueue::Pending (and the EnqueueResult::Pending / DependencyToResolve::Pending arms that only it fed), DNSRequestOwner::Prefetch, OptionsData::Saved(usize), Value::Saved(SavedFile) with the empty SavedFile struct, AdditionalFile::SourceIndex(u32), AnyRoute::FrameworkRouter(TypeIndex), AsyncState::Done(ExitCode), PromptBehaviour::Once { removed_count }, BuiltinInput::ArrayBuf.i, IntoArray.value, yaml NodeTag::{Verbatim, Unknown}(StringRange).
  • Rust: drop unused re-exports and consts. bun_core::timespec_mode, ResolvedSourceTag::INTERNAL_MODULE_REGISTRY_FLAG, ffi::ffi_object, macho_types::{cpu_subtype_t, load_command, LoadCommand, RawSlice}, ResultSet::Header, DataCell::Raw.
  • C++: drop six private host functions that no builtin calls any more: $makeGetterTypeError, $makeDOMException, $addAbortAlgorithmToSignal, $removeAbortAlgorithmFromSignal, $isAbortSignal, $createUninitializedArrayBuffer in ZigGlobalObject.cpp, with their BunBuiltinNames.h entries. The stream builtins that used the abort helpers moved to C++ and call AbortSignal::addAbortAlgorithmToSignal directly.
  • Verified: bun bd builds, cargo check --workspace, cargo clippy --workspace --no-deps, bun run rust:check-all (12 targets), and bun bd test on the shell, streams, yaml, bundler, Bun.build, bunx, and auto-install suites. Self-reviewed: 8 concerns raised, 2 folded in ($createUninitializedArrayBuffer, SavedFile), the rest are series-level follow-ups listed in the notes.
  • No test is added on purpose. Every line this PR removes is unreferenced, so there is no behavior a test can observe before and after the change. The code that stays is covered by the suites above.

Background

  • dead_code = "deny" covers private items only. A pub item reachable from the crate root is an "external API root" and exempt. To find cross-crate dead code, the sweep rewrote every pub item whose name appears nowhere outside its crate to pub(crate) and ran cargo check with --cap-lints warn. rustc then reported what nothing in the crate uses either. The rewrite is not part of this PR.
  • Most "never read" fields that rustc reported are false positives: multi_array_columns! reads them through items_<field>() accessors by name, RAII fields own memory a sibling borrows, and some structs are layout-punned. Those stay.
  • enqueue_dependency_to_root resolves synchronously (it sleeps until the task queue drains), so it never returned Pending. The Zig original had the same shape. The Pending arms downstream were unreachable since before the port.
Notes

Overlap with open PRs:

Related:

Scanned and found clean: src/js/** (module exports, $builtin functions, whole files), unused extern declarations in the *_sys crates, orphan .rs files outside any mod tree, unused Cargo dependencies, #if ENABLE(...) branches in the bindings, and commented-out code blocks older than six months.

Flagged by the tools but kept on purpose:

  • EmptyCopyFileState, WriteKind, Handle::init_close_on_complete, WIN_SOCKET_INFO_KEY, ListenerType::NamedPipe, TaskXmlError::*, CalendarError::InvalidCron, the kernel32 re-export module: live under #[cfg(windows)] or #[cfg(target_os = "macos")].
  • inflate_embedded, inflate_embedded_nul: live under cfg(bun_codegen_embed) through the embed_compressed! macro.
  • SSRKind::Regular, ErrorKind::Js* (bake serialized_failure.rs): part of a tag space shared with C++ or TypeScript.
  • PerformanceResourceTiming, PerformanceServerTiming and their ResourceTiming / NetworkLoadMetrics helpers: never instantiated, but the JS wrapper classes are exposed as globals and their prototype getters reference the implementation. Removing the implementation changes the prototype shape, so it needs a design decision.
  • CowSliceZ::init_dupe: only its own unit test uses it, and that test is the only in-tree Z = true instantiation.

Open dead-code PRs already hold the rest of what the tools flagged (ExportRenamer, BindgenTrivial, has_termination_request, rsplit_once, Pink, AllStmts, FontFeatureValues, the compile_result.rs types, and the formatStackTraceToJSValueWithoutPrepareStackTrace helper among others). Those were left out of this PR to avoid duplicate deletions.

Series-level follow-ups the self-review suggested, not done here: a source lint that fails when a BunBuiltinNames.h entry has no $name caller, a pub to pub(crate) narrowing pass over src/runtime, and the hawk gate in #37328.

@github-actions github-actions Bot added the claude label Sep 1, 2026
@coderabbitai

coderabbitai Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: b826a61e-fb42-4d5d-a21b-68a9e8bb2747

📥 Commits

Reviewing files that changed from the base of the PR and between 2b8bcbd and 6215216.

📒 Files selected for processing (5)
  • src/bundler/OutputFile.rs
  • src/bundler/linker_context/writeOutputFilesToDisk.rs
  • src/js/builtins/BunBuiltinNames.h
  • src/jsc/bindings/ZigGlobalObject.cpp
  • src/runtime/api/output_file_jsc.rs
💤 Files with no reviewable changes (2)
  • src/jsc/bindings/ZigGlobalObject.cpp
  • src/js/builtins/BunBuiltinNames.h

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


Walkthrough

This pull request removes unused payloads, obsolete enum variants, public re-exports, builtin identifiers, and legacy helper registrations across bundling, dependency resolution, parsing, runtime, and SQL modules.

Changes

Bun cleanup

Layer / File(s) Summary
Bundler output and source markers
src/bundler/..., src/runtime/api/output_file_jsc.rs
AdditionalFile::SourceIndex and saved output variants no longer carry unused index or size values.
Dependency resolution outcomes
src/install/..., src/install_types/..., src/resolver/resolver.rs
Pending dependency outcomes were removed. Version dependencies are borrowed during lookup and enqueueing.
YAML tag representation
src/parsers/yaml.rs
Verbatim and unknown tags remain validated but no longer store source ranges.
Runtime state and input payloads
src/runtime/server/..., src/runtime/shell/..., src/runtime/dns_jsc/dns.rs
Unused payloads were removed from route, array-buffer input, prompt, async completion, and DNS ownership variants.
Byte stream result data
src/runtime/webcore/...
Stream and file-reader results now retain copied lengths and completion state without JavaScript array values.
Public exports and obsolete builtins
src/bun_core/util.rs, src/exe_format/..., src/js/..., src/jsc/..., src/runtime/ffi/mod.rs, src/sql_jsc/...
Obsolete public exports, builtin identifiers, abort helpers, and module constants were removed.

Suggested reviewers: jarred-sumner, dylan-conway

Merge Risk: ⚪ Minimal · up to 62152

This PR removes verified dead code without a reported behavior change, and no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: removing dead code across the listed Rust components and JSC private host functions. It is specific and related to the changeset, although it lists sev…
Description check ✅ Passed The description explains the problem, lists the code removed, documents related scope and retained items, and provides detailed build, lint, and test verification. It covers the required purpose and v…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title clearly identifies the primary change: removing dead code across the listed Rust components and JSC private host functions. It is specific and related to the changeset, although it lists several affected areas.

Full details: Description check

Explanation

The description explains the problem, lists the code removed, documents related scope and retained items, and provides detailed build, lint, and test verification. It covers the required purpose and verification information despite using different section headings from the template.


Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Sep 1, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

How each deletion was verified:

Checks run locally: bun bd, cargo check --workspace, cargo clippy --workspace --no-deps, bun run rust:check-all (12 targets), and bun bd test on the shell, streams, yaml, bundler, Bun.build, bunx, and auto-install suites.

The second commit folds in two items the self-review found in files the PR already edits: the $createUninitializedArrayBuffer host function (no $ caller) and the empty SavedFile payload of Value::Saved.

No test is added on purpose. Every removed line is unreferenced, so there is no behavior a test can observe before and after the change.

The mordant job reports one generic_body_not_generic finding in src/runtime/server/RequestContext.rs, a file this PR does not touch. That job is advisory (continue-on-error).

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this dead-code sweep and didn't find any bugs. Because it's a broad automated sweep touching 28 files across many subsystems (bundler, resolver/auto-install, streams, JSC private host functions), a maintainer sanity check on the deletion set is still worthwhile.

What was reviewed:

  • Removed builtin private names ($makeGetterTypeError, $makeDOMException, $addAbortAlgorithmToSignal, $removeAbortAlgorithmFromSignal, $isAbortSignal): grepped src/js/** and src/jsc/bindings/** — no remaining $name/@ name call sites; only stale builtins.d.ts declarations remain (types-only, slated for #40690). The AbortSignal::addAbortAlgorithmToSignal C++ static and its JSStreamPipeToOperation callers are untouched.
  • IntoArray::value: JSValue removal: confirmed StreamResult::to_js and release() only ever read .len; ByteStream still calls self.value() for its pending_value.clear_without_deallocation() side effect, and ByteBlobLoader::on_pull still keeps array alive via EnsureStillAlive — no GC keep-alive was dropped.
  • Resolver Pending removal: enqueue_dependency_to_root has no remaining path that produces Pending; MatchStatus::Pending / PendingResolutionTag::Download remain live for the download path. The version: Dependency::Version → &Dependency::Version signature change only drops a clone that fed the deleted PendingResolution { dependency: version } construction.
  • AnyRoute::FrameworkRouter payload drop: init_ctx.framework_router_list is still pushed and consumed wholesale via mem::take in ServerConfig.rs; nothing indexed it by the removed TypeIndex.
Extended reasoning...

Overview

This PR is a cross-crate dead-code sweep: enum tuple/struct payloads that were never read are collapsed to unit variants (AdditionalFile::SourceIndex, OptionsData::Saved, YAML NodeTag::{Verbatim,Unknown}, AnyRoute::FrameworkRouter, shell AsyncState::Done, PromptBehaviour::Once, BuiltinInput::ArrayBuf, IntoArray), never-constructed variants are deleted (DependencyToEnqueue::Pending, EnqueueResult::Pending, DependencyToResolve::Pending, DNSRequestOwner::Prefetch), unused pub use re-exports are dropped (bun_core::timespec_mode, several bun_sys::macho items, ffi_object, MySQL Header, postgres Raw, INTERNAL_MODULE_REGISTRY_FLAG), and five unused JSC private host functions are removed from ZigGlobalObject.cpp along with their BunBuiltinNames.h macro entries and privateFunctions[] registrations. Net -176 lines across 28 files.

Security risks

None identified. This is pure deletion — no new code paths, no changed validation, no widened input handling. The one non-deletion change is enqueue_dependency's version parameter going from by-value to &, which only avoids a clone whose sole consumer (the deleted PendingResolution construction) is gone; the three remaining uses already borrowed it. The removed DNSRequestOwner::Prefetch arm called freeaddrinfo — but the variant was never constructed, so no leak is introduced.

Level of scrutiny

Moderate. Rust's exhaustive match and the workspace dead_code = "deny" mean any deleted variant still referenced would fail cargo check, and the PR claims bun run rust:check-all passed (covering #[cfg]-gated code like the edited #[cfg(not(windows))] defuse_array_buf_pins). For the C++ side, removing a BunBuiltinNames.h macro(name) entry would break the generated k_name/namePrivateName() accessor if anything still used it, so the build itself gates most of this. I focused manual verification on the two places the compiler cannot check: (1) whether any src/js/** builtin still calls the removed $name intrinsics at runtime — none do; (2) whether the dropped IntoArray::value: JSValue was a load-bearing GC root — it was write-only (to_js reads only .len), and the one side-effectful producer (ByteStream::value(), which clears pending_value) is still invoked.

Other factors

The change is mechanical and each deletion is individually simple, but the breadth (bundler, install/resolver auto-install path, Bun.serve route parsing, shell interpreter, ByteStream/FileReader, YAML parser, DNS, SQL, JSC bindings) plus bot authorship argues for a maintainer glance rather than auto-approval — particularly the ZigGlobalObject.cpp private-function removals and the resolver Pending chain, where the PR description's own notes flag adjacent follow-up work (PendingResolutionTag::Resolve, on_package_manifest_error) intentionally left behind. No CODEOWNERS-covered paths are touched (no .d.ts, no test/expectations.txt, no packages/bun-types/). No outstanding third-party reviews on the timeline.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR (including the follow-up commit) and didn't find any bugs. Given the breadth — 29 files across the bundler, resolver/auto-install, JSC C++ bindings, streams, shell, and YAML — a human skim would still be worthwhile, particularly on the claim that enqueue_dependency_to_root never returns Pending.

What was reviewed:

  • Removed $-builtins: no remaining $name/@ name call sites in src/js/** or codegen; only the .d.ts declarations remain (deferred to #40690 per the description).
  • IntoArray.value drop: the sole consumer in streams.rs reads only .len; ByteStream keeps the self.value() call for its clear_without_deallocation side effect.
  • Pending chain: MatchStatus::Pending still has its other producer (the download path at resolver.rs:3097), so only the enqueue-fed arm is removed; enqueue_dependency moving to &Dependency::Version is sound now that nothing moves version out.
  • Second commit: $createUninitializedArrayBuffer has no callers and its putDirectNativeFunction registration is removed alongside; SavedFile was empty and all match arms updated.
Extended reasoning...

Overview

This PR is a cross-crate dead-code sweep removing never-constructed enum variants, never-read variant payloads, unused struct fields, unused re-exports, and six orphaned JSC private host functions. It spans 29 files across bun_bundler, bun_install/bun_resolver, bun_runtime (server, shell, webcore streams, DNS, FFI), bun_parsers (YAML), bun_core, exe_format, the SQL crates, and the C++ JSC bindings (ZigGlobalObject.cpp, BunBuiltinNames.h). Net −197 lines. A second commit pushed after the earlier review added two more removals of the same shape ($createUninitializedArrayBuffer and the empty SavedFile payload on output_file::Value::Saved).

Security risks

None identified. No auth, crypto, TLS, permission, or input-validation code is touched. The removed C++ host functions were unreferenced by any builtin JS, so no user-reachable behavior changes. The resolver change removes an unreachable branch rather than weakening a check.

Level of scrutiny

Moderate. Each individual hunk is a trivial, mechanical deletion (drop a payload, drop a re-export, drop a match arm), and REVIEW.md explicitly requires deleting dead code in the PR that makes it dead. However, the sweep is broad, bot-authored, and touches subsystems where a wrong "dead" call has non-local consequences: JSC private builtins (a missed $name caller would fail at runtime, not compile time), the auto-install resolver path (removing Pending relies on enqueue_dependency_to_root being synchronous — asserted in the description but not independently re-derived here), and stream internals (a dropped JSValue field could have been a GC keep-alive). I verified the riskiest of these: no $/@ call sites remain for the six removed builtins; IntoArray.value is read nowhere and the ByteStream side-effecting self.value() call is preserved; MatchStatus::Pending retains its other constructor so the enum itself is untouched; the &Dependency::Version borrow is sound now that the only consumer that moved it is gone.

Other factors

No CODEOWNERS entries cover the changed paths. The bug hunt exited on dry_streak with zero findings. The PR intentionally ships no new tests (pure deletion of unreferenced code — nothing observable to assert), and claims bun bd, cargo check --workspace, cargo clippy, and bun run rust:check-all across all 12 targets pass, plus the shell/streams/yaml/bundler/bunx/auto-install suites. An earlier COMMENTED review under this app's identity was followed by a commit folding in two of its points; nothing in the timeline reads as an outstanding objection. Given the file-count breadth and the one behavioral assumption I did not independently prove (synchronous enqueue_dependency_to_root), deferring for a quick maintainer skim is the safer call over auto-approval.

@robobun

robobun commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

On the one assumption the review did not re-derive, here is the evidence for enqueue_dependency_to_root never returning Pending.

The function is src/install/PackageManager/PackageManagerEnqueue.rs:523-670. It has five return sites: Failure at lines 556, 585 and 654, NotFound at line 664, and Resolution at line 667. When the dependency is not resolved yet, it does not return. It calls PackageManager::sleep_until (line 646) with a closure that runs run_tasks until pending_task_count() == 0, and only then reads the resolution. So the Pending(DependencyID) variant had a declaration and no construction site. The Zig version of this function (PackageManagerEnqueue.zig before d451445) had the same five returns and the same sleepUntil loop, so the arms downstream (EnqueueResult::Pending, DependencyToResolve::Pending) were unreachable before the port as well.

Jarred-Sumner pushed a commit that referenced this pull request Sep 15, 2026
…ter, NodeHTTPResponse, and three JS binding objects (#42682)

### Problem
- Some native members exist only for built-in JS to call, and no
built-in JS calls them. The largest is `H2FrameParser.setStreamPriority`
(`src/runtime/api/bun/h2_frame_parser.rs`). `Http2Stream#priority()` is
a no-op since RFC 9113.
- Three binding objects (`node:vm`, `node:crypto`, `bun:sql`) carry
properties that their only JS consumer never reads.
- `packages/bun-usockets` keeps a commented-out
`bsd_udp_packet_buffer_ecn` from 2024-04. Nothing reads
`completions/spec.yaml`, a 2021 appspec file.

### Fix
- Delete each item: 15 files, 535 lines removed. The Notes list every
symbol.
- Correct because every removed name has zero references in `src/`,
`packages/`, `scripts/`, `test/` and `build/debug/codegen/` outside its
own definition. No removed member is documented or typed API. User code
reaches the h2 parser handle only through the undocumented
`Symbol.for("::bunhttp2native::")` key.
- The `-D dead-code` lint then reported three more items in
`h2_frame_parser.rs`, and two `Stream` fields became write-only. They
are removed too.
- Verified: `bun bd` and `bun run rust:check-all` (12 targets) pass. The
http2, shell, vm, crypto, `node:http`, sql, udp and streams test files
pass (list in the Notes).

### Background
- A `*.classes.ts` file declares the methods of a native class. The
generator emits the C++ wrapper and the Rust glue from it. A `proto`
entry that no JS reads is dead, and so is its Rust function.
- A binding object is a plain object that native code fills and one
built-in module destructures once. A property that the module does not
destructure is never read.
- 28 other dead-code pull requests are open. This one deletes nothing
that they delete. #40240 and #37588 edit the body of
`set_stream_priority`. Each conflict resolves by taking the deletion.

<details><summary>Notes</summary>

Removed, one line each:

- `H2FrameParser.setStreamPriority` / `set_stream_priority` (109 lines).
No `.setStreamPriority(` in `src/js` or `test/`.
- `H2FrameParser.isStreamAborted` / `is_stream_aborted`. Same check.
- `H2FrameParser.hasNativeRead` / `has_native_read`. Same check.
- `FrameType::HTTP_FRAME_PRIORITY`, `ErrorCode::PROTOCOL_ERROR`,
`SignalRef::is_aborted`. Reported by the dead-code lint once the three
functions above were gone. Both enums already list only the wire values
that the file uses.
- `Stream::stream_dependency`, `Stream::exclusive`. Their only reads
were in `set_stream_priority`. The declarations, the initializers and
the two stores in `request()` go. The locals that `request()` writes to
the wire stay, and so does `Stream::weight` (read by `getStreamState`).
- `ShellInterpreter.isRunning` / `is_running` and `.started` /
`get_started`. `src/js/builtins/shell.ts` only calls `interp.run()`.
- `Interpreter::started`. The field was only read by `get_started`. The
two stores and the `AtomicBool` import go with it.
- `NodeHTTPResponse.dumpRequestBody` / `dump_request_body`. Added in
#17093, never called from JS at any commit.
- `createNodeVMBinding`: `kUnlinked`, `kLinking`, `kEvaluating`,
`kSourceText`, `kSynthetic`. `src/js/node/vm.ts` destructures `kLinked`,
`kEvaluated`, `kErrored` and never touches the binding object again.
- `createNodeCryptoBinding`: `SecretKeyObject`, `PublicKeyObject`,
`PrivateKeyObject`. Not destructured in `src/js/node/crypto.ts`. The
classes stay, only the unread properties go.
- `bun_sql_jsc::mysql::create_binding`: `MySQLConnection`.
`bun_sql_jsc::postgres::create_binding`: `PostgresSQLConnection`.
`src/js/internal/sql/{mysql,postgres}.ts` destructure
`createConnection`, `createQuery`, `init` only.
- `packages/bun-usockets`: the commented-out `bsd_udp_packet_buffer_ecn`
(`bsd.c`), its commented-out wrapper `us_udp_packet_buffer_ecn`
(`udp.c`), and the two commented-out declarations (`libusockets.h`,
`internal/networking/bsd.h`). `git blame`: 589f941, 2024-04-26. The
`libusockets.h` lines are context lines of a hunk in #40294.
- `completions/spec.yaml`. No hit for `spec.yaml` or `appspec` in the
repo. It still lists the removed `bun dev` subcommand. The shell
completions are hand-maintained and embedded with `include_bytes!`.

Tests run with the debug build: `test/js/node/http2/` (7 files,
`node-http2.test.js` 387 pass, `h2-conformance.test.ts` 70 pass), the
four `test-http2-*priority*` Node tests, `test/js/bun/shell/` (4 files),
`test/js/node/vm/vm.test.ts`, `crypto.key-objects.test.ts`, five
`test/js/node/http/` files, five `test/js/sql/` files,
`udp_socket.test.ts`, `streams.test.js`.

How the candidates were found:

- A repo-wide identifier index (definitions with zero other mentions,
with and without comments).
- A relink of the debug binary with `--gc-sections --print-gc-sections`.
Almost every real hit from that pass is already deleted by one of the
open pull requests. The rest were inlined functions, `const fn`s used
only at compile time, and Windows or macOS paths.
- A per-class comparison of every `*.classes.ts` member against
`src/js`, `packages/bun-types` and `test/`. The generated thunk for a
`proto` entry is `#[no_mangle]` and `#[allow(dead_code)]`, so neither
rustc nor the linker can flag these members.
- Checks for `.rs` files outside every `mod` tree, headers that nothing
includes, C/C++ files outside the build, Cargo features that nothing
enables, and long commented-out blocks. All clean apart from the
usockets block.

Found, not deleted here:

- `TCPSocket`/`TLSSocket` `endBuffered` (`$end`): no JS caller, but its
removal leaves `write_or_end_buffered::<IS_END>` with one instantiation.
That is a refactor, not a deletion.
- `*InternalReadableStreamSource.isClosed` getter: no reader, but its
removal leaves `NewSource::is_closed` write-only, and the stores are in
files that #41088 touches.
- `NodeHTTPResponse.onwritable`: no reader in `src/js` today, but #41822
starts to use it.
- The HEADERS+PRIORITY emitter and the native `SignalRef` abort path in
`H2FrameParser::request()`. `http2.ts` warns with DEP0194 for the
priority options and handles `options.signal` itself. Whether these
native paths still run needs a separate look.
- The `IPV6_RECVTCLASS` / `IP_RECVTOS` `setsockopt` calls in
`packages/bun-usockets/src/bsd.c` ("used for getting the ECN"). Nothing
reads the ECN now, but their removal changes socket options, so it is
not a pure deletion.
- `macro(mockedFunction)` and `macro(writer)` in
`src/js/builtins/BunBuiltinNames.h` (the builtin-name entries, not the
live `mockedFunction` string in `BunCommonStrings.h`): no
`mockedFunctionPrivateName` or `writerPrivateName` user, but two open
pull requests edit adjacent lines.
- `scripts/packer/build-image.pkr.hcl`, `scripts/lldb-inline.sh`,
`scripts/lldb-inline-tool.cpp`, `scripts/github-metrics.ts`,
`scripts/gamble.ts`: nothing references them, but they are standalone
tools that a person can run by hand.

Self-reviewed: 12 concerns raised, 10 addressed (the two `Stream`
fields, the header comment lines, and the body corrections above).
Rejected: a split into three pull requests, because every deletion is
verified the same way and a split triples the CI and review rounds.
Deferred: a caller lint for `*.classes.ts` `proto` entries, as a
follow-up that does not gate this change.

</details>
usrbinkat pushed a commit to usrbinkat/bun that referenced this pull request Sep 15, 2026
…ter, NodeHTTPResponse, and three JS binding objects (oven-sh#42682)

### Problem
- Some native members exist only for built-in JS to call, and no
built-in JS calls them. The largest is `H2FrameParser.setStreamPriority`
(`src/runtime/api/bun/h2_frame_parser.rs`). `Http2Stream#priority()` is
a no-op since RFC 9113.
- Three binding objects (`node:vm`, `node:crypto`, `bun:sql`) carry
properties that their only JS consumer never reads.
- `packages/bun-usockets` keeps a commented-out
`bsd_udp_packet_buffer_ecn` from 2024-04. Nothing reads
`completions/spec.yaml`, a 2021 appspec file.

### Fix
- Delete each item: 15 files, 535 lines removed. The Notes list every
symbol.
- Correct because every removed name has zero references in `src/`,
`packages/`, `scripts/`, `test/` and `build/debug/codegen/` outside its
own definition. No removed member is documented or typed API. User code
reaches the h2 parser handle only through the undocumented
`Symbol.for("::bunhttp2native::")` key.
- The `-D dead-code` lint then reported three more items in
`h2_frame_parser.rs`, and two `Stream` fields became write-only. They
are removed too.
- Verified: `bun bd` and `bun run rust:check-all` (12 targets) pass. The
http2, shell, vm, crypto, `node:http`, sql, udp and streams test files
pass (list in the Notes).

### Background
- A `*.classes.ts` file declares the methods of a native class. The
generator emits the C++ wrapper and the Rust glue from it. A `proto`
entry that no JS reads is dead, and so is its Rust function.
- A binding object is a plain object that native code fills and one
built-in module destructures once. A property that the module does not
destructure is never read.
- 28 other dead-code pull requests are open. This one deletes nothing
that they delete. oven-sh#40240 and oven-sh#37588 edit the body of
`set_stream_priority`. Each conflict resolves by taking the deletion.

<details><summary>Notes</summary>

Removed, one line each:

- `H2FrameParser.setStreamPriority` / `set_stream_priority` (109 lines).
No `.setStreamPriority(` in `src/js` or `test/`.
- `H2FrameParser.isStreamAborted` / `is_stream_aborted`. Same check.
- `H2FrameParser.hasNativeRead` / `has_native_read`. Same check.
- `FrameType::HTTP_FRAME_PRIORITY`, `ErrorCode::PROTOCOL_ERROR`,
`SignalRef::is_aborted`. Reported by the dead-code lint once the three
functions above were gone. Both enums already list only the wire values
that the file uses.
- `Stream::stream_dependency`, `Stream::exclusive`. Their only reads
were in `set_stream_priority`. The declarations, the initializers and
the two stores in `request()` go. The locals that `request()` writes to
the wire stay, and so does `Stream::weight` (read by `getStreamState`).
- `ShellInterpreter.isRunning` / `is_running` and `.started` /
`get_started`. `src/js/builtins/shell.ts` only calls `interp.run()`.
- `Interpreter::started`. The field was only read by `get_started`. The
two stores and the `AtomicBool` import go with it.
- `NodeHTTPResponse.dumpRequestBody` / `dump_request_body`. Added in
oven-sh#17093, never called from JS at any commit.
- `createNodeVMBinding`: `kUnlinked`, `kLinking`, `kEvaluating`,
`kSourceText`, `kSynthetic`. `src/js/node/vm.ts` destructures `kLinked`,
`kEvaluated`, `kErrored` and never touches the binding object again.
- `createNodeCryptoBinding`: `SecretKeyObject`, `PublicKeyObject`,
`PrivateKeyObject`. Not destructured in `src/js/node/crypto.ts`. The
classes stay, only the unread properties go.
- `bun_sql_jsc::mysql::create_binding`: `MySQLConnection`.
`bun_sql_jsc::postgres::create_binding`: `PostgresSQLConnection`.
`src/js/internal/sql/{mysql,postgres}.ts` destructure
`createConnection`, `createQuery`, `init` only.
- `packages/bun-usockets`: the commented-out `bsd_udp_packet_buffer_ecn`
(`bsd.c`), its commented-out wrapper `us_udp_packet_buffer_ecn`
(`udp.c`), and the two commented-out declarations (`libusockets.h`,
`internal/networking/bsd.h`). `git blame`: 589f941, 2024-04-26. The
`libusockets.h` lines are context lines of a hunk in oven-sh#40294.
- `completions/spec.yaml`. No hit for `spec.yaml` or `appspec` in the
repo. It still lists the removed `bun dev` subcommand. The shell
completions are hand-maintained and embedded with `include_bytes!`.

Tests run with the debug build: `test/js/node/http2/` (7 files,
`node-http2.test.js` 387 pass, `h2-conformance.test.ts` 70 pass), the
four `test-http2-*priority*` Node tests, `test/js/bun/shell/` (4 files),
`test/js/node/vm/vm.test.ts`, `crypto.key-objects.test.ts`, five
`test/js/node/http/` files, five `test/js/sql/` files,
`udp_socket.test.ts`, `streams.test.js`.

How the candidates were found:

- A repo-wide identifier index (definitions with zero other mentions,
with and without comments).
- A relink of the debug binary with `--gc-sections --print-gc-sections`.
Almost every real hit from that pass is already deleted by one of the
open pull requests. The rest were inlined functions, `const fn`s used
only at compile time, and Windows or macOS paths.
- A per-class comparison of every `*.classes.ts` member against
`src/js`, `packages/bun-types` and `test/`. The generated thunk for a
`proto` entry is `#[no_mangle]` and `#[allow(dead_code)]`, so neither
rustc nor the linker can flag these members.
- Checks for `.rs` files outside every `mod` tree, headers that nothing
includes, C/C++ files outside the build, Cargo features that nothing
enables, and long commented-out blocks. All clean apart from the
usockets block.

Found, not deleted here:

- `TCPSocket`/`TLSSocket` `endBuffered` (`$end`): no JS caller, but its
removal leaves `write_or_end_buffered::<IS_END>` with one instantiation.
That is a refactor, not a deletion.
- `*InternalReadableStreamSource.isClosed` getter: no reader, but its
removal leaves `NewSource::is_closed` write-only, and the stores are in
files that oven-sh#41088 touches.
- `NodeHTTPResponse.onwritable`: no reader in `src/js` today, but oven-sh#41822
starts to use it.
- The HEADERS+PRIORITY emitter and the native `SignalRef` abort path in
`H2FrameParser::request()`. `http2.ts` warns with DEP0194 for the
priority options and handles `options.signal` itself. Whether these
native paths still run needs a separate look.
- The `IPV6_RECVTCLASS` / `IP_RECVTOS` `setsockopt` calls in
`packages/bun-usockets/src/bsd.c` ("used for getting the ECN"). Nothing
reads the ECN now, but their removal changes socket options, so it is
not a pure deletion.
- `macro(mockedFunction)` and `macro(writer)` in
`src/js/builtins/BunBuiltinNames.h` (the builtin-name entries, not the
live `mockedFunction` string in `BunCommonStrings.h`): no
`mockedFunctionPrivateName` or `writerPrivateName` user, but two open
pull requests edit adjacent lines.
- `scripts/packer/build-image.pkr.hcl`, `scripts/lldb-inline.sh`,
`scripts/lldb-inline-tool.cpp`, `scripts/github-metrics.ts`,
`scripts/gamble.ts`: nothing references them, but they are standalone
tools that a person can run by hand.

Self-reviewed: 12 concerns raised, 10 addressed (the two `Stream`
fields, the header comment lines, and the body corrections above).
Rejected: a split into three pull requests, because every deletion is
verified the same way and a split triples the CI and review rounds.
Deferred: a caller lint for `*.classes.ts` `proto` entries, as a
follow-up that does not gate this change.

</details>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant