Skip to content

Remove dead code from the zsh completions, the SQLite lazy loader, and the C++ bindings - #47439

Open
robobun wants to merge 11 commits into
mainfrom
robobun/a2b0d6d7/dead-code-sweep
Open

robobun wants to merge 11 commits into
mainfrom
robobun/a2b0d6d7/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • 83 lines in 18 files have no user on main.
  • No check that CI runs reports them: shell functions, header-only C++, repeated #include lines, comments, and one Rust line that only the disabled unused_extern_crates lint reports.

Fix

Behaviour change: none

Background

  • lazy_sqlite3.h compiles on macOS only. There Bun loads the system SQLite with dlopen. It calls each function through a pointer that this header declares and resolves.
  • completions/bun.zsh is embedded in the binary. bun completions writes it to disk.
  • A second #include of a guarded header compiles to nothing.
  • No other design was weighed: a deletion has one place.

Downsides

  • The installed zsh completion script loses two functions that nothing calls.
  • A later merge can add a use of a removed name. This happened once: since AOT: the next base #48120, node:net reads emitEventWithArgs from internal/trace_events, and that hunk is out. The Notes hold a check of every open head.
Notes

Removed

  • completions/bun.zsh (-21). _set_remove and __bun_dynamic_comp. Each name has one hit in the tree, its definition. The script dispatches by name nowhere: its only compdef is compdef _bun bun.
  • src/jsc/bindings/sqlite/lazy_sqlite3.h (-12). The typedef, the pointer, the #define and the dlsym line of sqlite3_bind_blob, sqlite3_bind_text and sqlite3_bind_text16. A word search over src outside the vendored amalgamation finds these 12 lines only. The two includers call the 64 variants. The macOS link is the check for a missed caller: no libsqlite3 is linked there. Remove dead code from the JSC, WebCore, and node bindings, uWS/uSockets, SecretsLinux, and seven Rust crates #42431 found the same three entries and left them out because the header compiles on macOS only.
  • src/jsc/bindings/ZigGlobalObject.h (-5). Bun::vm(Zig::GlobalObject*). The twelve calls pass a JSC::VM& (eight) or a JSC::JSGlobalObject* (four).
  • src/jsc/bindings/JSFFIFunction.{h,cpp} (-2). The member symbolFromDynamicLibrary and its one write. Remove dead code from Bun.Terminal, FileSink, and URLSearchParams: four write-only fields #46268 held it for bun:ffi: make cc() symbols' .ptr the address of the compiled C function #38031. At its head, bun:ffi: make cc() symbols' .ptr the address of the compiled C function #38031 reads the function parameter of the same name, not the member.
  • 19 repeated #include lines in 9 files.
    • 13 are in the same preprocessor scope as the first include, or in a scope inside it: bun-spawn.cpp (<fcntl.h>, <signal.h>), BunProcess.cpp (<sys/stat.h>), c-bindings.cpp (<cstdlib>), node/crypto/JSHmac.cpp and JSHash.cpp (<JavaScriptCore/Error.h>), node/crypto/DiffieHellmanFunctions.h, webcore/JSWebSocket.cpp (four), webcore/JSAbortSignal.cpp, webcore/JSDOMURL.cpp.
    • 2 follow an #if/#else whose two arms both include the header: <fcntl.h> in BunProcess.cpp, <atomic> in c-bindings.cpp.
    • 4 are in a sibling block that the platform implies: <sys/resource.h> and <sys/types.h> in BunProcess.cpp (__linux__ or __FreeBSD__ implies !WIN32), <signal.h> and <pthread.h> in c-bindings.cpp (OS(LINUX) || OS(DARWIN) || OS(FREEBSD) implies !OS(WINDOWS)).
  • webcore/EventNames.h (-2), webcore/JSEventTargetCustom.cpp (-4). Commented-out code. git blame dates the lines 2022-03-28 in the first file, and 2022-05-08 to 2026-04-25 in the second.
  • src/install/lib.rs (-1). extern crate bun_sha_hmac as bun_sha;. The crate names bun_sha_hmac directly. rustc reports the line with -W unused_extern_crates.
  • src/jsc/bindings/Bindgen/IDLConvert.h (-14). Bun::Bindgen::ArgumentInFrame, which Compile TypeScript to machine code ahead of time: bun build --compile --aot #45631 added. Nothing names the class. The generated host functions hand a cppInlineBody the class ArgumentsInFrame, and a cppSymbol function takes its arguments as a Rust thunk does.
  • src/jsc/bindings/c-bindings.cpp (-3). An #include <wtf/WTFConfig.h> under ENABLE(AOT_GUARDED_PAGES), which Compile TypeScript to machine code ahead of time: bun build --compile --aot #45631 added. The same #if !OS(WINDOWS) block includes the header 25 lines above.

Taken out of this pull request since it opened (66 lines)

Open pull requests

Found dead in this run, and left alone (list of 2026-11-01, not checked again)

How the candidates were found

  • Eleven text searches, one for each area: src/js (two parts), the C++ bindings (five parts), uSockets and uWS, the build scripts, the side packages, and commented-out code.
  • A relink of the debug build with --gc-sections --print-gc-sections, with each dropped function mapped to its source line. The linker drops 379 functions of the hand-written C and C++. Almost all are Windows or macOS code, or are deleted by an open pull request.
  • cargo check --workspace with -W unused_crate_dependencies -W unused_macro_rules -W unused_extern_crates: two hits, and no unused Cargo dependency.
  • A scope-aware scan for repeated #include lines.

Tests, debug build of this branch

  • Head of 2026-11-10, with main 98cda0d: bun bd builds. A word search of the merged tree finds no user of a removed name. 16 files pass with --timeout 60000: test/aot/shell-completions-list-flags.test.ts, source-lints/no-iostream-include.test.ts, source-lints/shim-stdint-includes.test.ts, test/internal/bindgen.test.ts, bindgen-declared-results.test.ts, crypto.hmac.test.ts, crypto-hmac-algorithm.test.ts, crypto-lazyhash.test.ts, ecdh.test.ts, url.test.ts, abort.test.ts, websocket-close-code.test.ts, sqlite.test.js, ffi.test.js, trace-events.test.ts and bun/spawn/spawn.test.ts. One earlier run of spawn.test.ts did not end in 40 minutes.
  • Merge of main 89452d2 (AOT: bun compile, build output, module mocks, fixes and new names #48436), head 9d4dc5a: no conflict, and that commit changes no file of this diff. bun bd builds. The word search finds no user of a removed name. 13 of the 16 files above pass again with --timeout 60000. The other 3 did not run again: bindgen-declared-results.test.ts, crypto-hmac-algorithm.test.ts and spawn.test.ts.
  • rust:check-all did not run on this head. The one Rust line of the diff is the bun_sha alias, and no file of src/install names bun_sha for any target.
  • Earlier heads: bun bd, bun run rust:check-all (12 triples) and up to 26 test files, on 2026-10-30, 2026-10-31 and 2026-11-01. The one failure then was "should be the node version on the host that we expect" in process.test.js: the sandbox has node v26.3.0 and the test expects v26.12.1.
  • Not run here: macOS and Windows. CI builds them.

no test proof · iteration 15 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check

…d the C++ bindings

Delete code that nothing references:

- _set_remove and __bun_dynamic_comp in completions/bun.zsh.
- The lazy-loader entries of sqlite3_bind_blob, sqlite3_bind_text and
  sqlite3_bind_text16. The bindings call the 64 variants only.
- The Bun::vm(Zig::GlobalObject*) overload. Each call passes a JSC::VM&
  or a JSC::JSGlobalObject*.
- JSFFIFunction::symbolFromDynamicLibrary, which is written and never
  read.
- 19 #include lines that repeat an earlier #include of the same header.
- Commented-out code in EventNames.h and JSEventTargetCustom.cpp.
- The bun_sha alias of bun_sha_hmac in bun_install.
…and bindgen

#45631 added these without a reader, or removed their last reader:

- signatureOfBindgen, parseBufferAccessors, jsonSerializationUses and
  unplacedPath in src/codegen/builtins.
- Bun::Bindgen::ArgumentInFrame.
- $Node.PreallocatedEvents and the emitEventWithArgs re-export of
  internal/trace_events.
- The notImplementedIssueFn build-time macro.
- A second include of wtf/WTFConfig.h in c-bindings.cpp.
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The pull request removes helper code and native declarations, including selected SQLite lazy bindings. It also removes duplicate includes and commented-out code, and adjusts platform-specific includes in BunProcess.cpp.

Changes

Code and include cleanup

Layer / File(s) Summary
Helper and interface removals
completions/bun.zsh, src/install/lib.rs, src/jsc/bindings/Bindgen/IDLConvert.h, src/jsc/bindings/JSFFIFunction.cpp, src/jsc/bindings/JSFFIFunction.h, src/jsc/bindings/ZigGlobalObject.h
Completion helpers, the bun_sha alias, ArgumentInFrame, and the Bun::vm(Zig::GlobalObject*) overload were removed. JSFFIFunction no longer stores symbolFromDynamicLibrary; the read-only JavaScript ptr property remains.
SQLite lazy binding removals
src/jsc/bindings/sqlite/lazy_sqlite3.h
Lazy declarations, pointers, aliases, and symbol lookups for sqlite3_bind_blob, sqlite3_bind_text, and sqlite3_bind_text16 were removed. The corresponding 64-bit bindings remain.
Native include and inactive-code cleanup
src/jsc/bindings/BunProcess.cpp, src/jsc/bindings/bun-spawn.cpp, src/jsc/bindings/c-bindings.cpp, src/jsc/bindings/node/crypto/*, src/jsc/bindings/webcore/*
Duplicate includes and commented-out code were removed. BunProcess.cpp adds <sys/time.h> to the Linux and FreeBSD include block and removes repeated FreeBSD includes.
Merge Risk: Minimal · up to 7d1f6

This cleanup is not expected to change runtime behavior. The author reports successful builds and tests, and the remaining checks are ordinary CI and platform validation.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly summarizes the main changes: removing unused code from zsh completions, the SQLite lazy loader, and C++ bindings.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description explains the problem, the removals, and the intended lack of behavior change. It also reports build and test verification, including tests not run and platform limits. It covers both r…
Full details: Description check

Explanation

The description explains the problem, the removals, and the intended lack of behavior change. It also reports build and test verification, including tests not run and platform limits. It covers both required template topics, though it uses different section headings.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 0a562a7 and 7d1f6ec.

Files selected for processing: 18

  • completions/bun.zsh
  • src/install/lib.rs
  • src/jsc/bindings/Bindgen/IDLConvert.h
  • src/jsc/bindings/BunProcess.cpp
  • src/jsc/bindings/JSFFIFunction.cpp
  • src/jsc/bindings/JSFFIFunction.h
  • src/jsc/bindings/ZigGlobalObject.h
  • src/jsc/bindings/bun-spawn.cpp
  • src/jsc/bindings/c-bindings.cpp
  • src/jsc/bindings/node/crypto/DiffieHellmanFunctions.h
  • src/jsc/bindings/node/crypto/JSHash.cpp
  • src/jsc/bindings/node/crypto/JSHmac.cpp
  • src/jsc/bindings/sqlite/lazy_sqlite3.h
  • src/jsc/bindings/webcore/EventNames.h
  • src/jsc/bindings/webcore/JSAbortSignal.cpp
  • src/jsc/bindings/webcore/JSDOMURL.cpp
  • src/jsc/bindings/webcore/JSEventTargetCustom.cpp
  • src/jsc/bindings/webcore/JSWebSocket.cpp
Files with no reviewable changes: 18

  • src/jsc/bindings/node/crypto/JSHmac.cpp
  • src/jsc/bindings/node/crypto/DiffieHellmanFunctions.h
  • src/jsc/bindings/bun-spawn.cpp
  • src/jsc/bindings/webcore/JSAbortSignal.cpp
  • src/jsc/bindings/webcore/EventNames.h
  • src/jsc/bindings/webcore/JSEventTargetCustom.cpp
  • src/jsc/bindings/JSFFIFunction.h
  • src/jsc/bindings/node/crypto/JSHash.cpp
  • src/jsc/bindings/Bindgen/IDLConvert.h
  • src/install/lib.rs
  • src/jsc/bindings/webcore/JSWebSocket.cpp
  • src/jsc/bindings/JSFFIFunction.cpp
  • completions/bun.zsh
  • src/jsc/bindings/BunProcess.cpp
  • src/jsc/bindings/webcore/JSDOMURL.cpp
  • src/jsc/bindings/ZigGlobalObject.h
  • src/jsc/bindings/sqlite/lazy_sqlite3.h
  • src/jsc/bindings/c-bindings.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@robobun

robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • The head is 9d4dc5a. It contains main at 89452d2, its tip on 2026-11-10 at 16:56 UTC. The diff against main is 83 deleted lines in 18 files, and no insertion.
  • On 2026-11-10 the branch conflicted with main in 4 files, and main had overtaken several hunks. Those hunks are out of the diff. The section "Taken out" in the description names each one.
  • The most important one: since AOT: the next base #48120, node:net reads emitEventWithArgs from internal/trace_events. The old hunk removed that export and still merged with main with no conflict. It is out.
  • The notImplementedIssueFn macro stays, because open AOT: a loop over a Map or a Set reads the facts of its table once, in front of the loop #48356 calls it at its head. The two $Node constructor types of the ahead-of-time node:http texts stay with their owners.
  • The comparison with the open pull requests ran on 2026-11-10 on all 6,205 heads, and again at 16:13 UTC on the 104 heads updated since. The branch adds a conflict to no pull request that merges with main. No head adds a use of a removed name. The description has the numbers.
  • Local checks on this head: bun bd builds, a word search finds no user of a removed name, and 13 test files pass. The Notes of the description list them.
  • CI on this head, build #139823, is complete: 188 of 189 jobs pass. All 14 build-bun jobs, the binary-size check and every macOS test job are among them. One debian 13 x64-asan test job fails outside the tests. The test runner crashed with TypeError: Invalid URL in getFileUrl (scripts/buildkite.ts:374) when it reported a first attempt of vendor/elysia/test/response/stream.test.ts that failed. The second attempt of that test passed. Since Check vendored test suites out by commit #48428 a vendored test suite is checked out with no origin remote, and getFileUrl needs one. This diff changes no file under scripts/. No test is red. The build of main at 89452d2 (#139772) is green.
  • On the head before the merge of AOT: bun compile, build output, module mocks, fixes and new names #48436, build #139743 had 187 of 188 jobs pass. Its one failure was a peak-memory check of the ahead-of-time compiler on ubuntu 25.04 aarch64 (140 MB against a limit of 139 MB). That check passes in the build of this head.

@robobun

robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:09 PM PT - Nov 10th, 2026

❌ @robobun, your commit 9d4dc5a has 1 failures in Build #139823 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 47439

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

bun-47439 --bun

@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 and didn't find any bugs. Because it spans 24 files including platform-gated includes and a .d.ts covered by CODEOWNERS, a human look would still be worthwhile.

What was reviewed:

  • Grepped every removed symbol (signatureOfBindgen, parseBufferAccessors, notImplementedIssueFn, ArgumentInFrame, symbolFromDynamicLibrary, $Node.PreallocatedEvents, the bun_sha alias, the two zsh functions, sqlite3_bind_text/blob/text16) across src, test and completions — no remaining reader; emitEventWithArgs is still exported by internal/trace_event_recorder, which is what node:net requires.
  • The Bun::vm(Zig::GlobalObject*) removal: all seven callers already pass a JSC::VM& or a JSC::JSGlobalObject* (bindings.cpp and InspectorTestReporterAgent.cpp declare globalObject as JSC::JSGlobalObject*), so the deleted overload was never selected and dispatch is unchanged.
  • Each removed #include is still included earlier in the same file in an enclosing or implied preprocessor scope (e.g. BunProcess.cpp's #ifndef WIN32 block covers the __linux__ || __FreeBSD__ removals; c-bindings.cpp's <atomic>/<signal.h>/<pthread.h> are in both arms or an implied !OS(WINDOWS) scope).
Extended reasoning...

The change is a 127-line pure deletion across zsh completions, the builtins codegen, built-in JS, C++ bindings headers/includes, the macOS-only lazy SQLite loader, and one Rust extern-crate alias, with no additions. It touches no security-sensitive surface; the crypto files only lose duplicate include lines. Every deletion was verified to have no remaining consumer and every include removal to be covered in the same file, and the sole overload removal provably changes no call resolution. Deferring rather than approving because src/js/builtins.d.ts falls under the *.d.ts CODEOWNERS entry, the diff is wide, and the lazy_sqlite3.h and Windows/FreeBSD include paths cannot be compiled from this Linux checkout.

One conflict, in src/jsc/bindings/ZigGlobalObject.h. On main, #47498
moved the declaration of `Bun::putDirectNamed` to root.h. This branch
removes the unused `Bun::vm(Zig::GlobalObject*)`, the next lines of the
file. Both are removed.
@robobun

robobun commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

Merged main (204d701) into this branch. The old head was 7b5eb29, the new head is 4d5484b.

The search for users, run again on the merged tree:

  • No name that this PR removes has a user. symbolFromDynamicLibrary has two hits, and both are the function parameter of that name.
  • Bun::vm() still has seven calls: four pass a JSC::VM& and three pass a global object. The tree builds without the removed overload.
  • Each of the 20 removed #include lines still has one include of that header in its file (two where an #if and its #else both include it).
  • node:net takes emitEventWithArgs from internal/trace_event_recorder, as before.
  • Not run again: the comparison with the heads of the open pull requests.

Checked on the merged tree (Linux x64, debug build with ASAN):

  • bun bd builds. No Rust file changed on main since the base of this PR, so rust:check-all was not run again.
  • check-types-of-modules.ts and tsc --noEmit -p src/tsconfig.json pass. clang-format and rustfmt report nothing on the touched files.
  • The 7 internal files pass: builtins-manifest.test.ts (23), build-codegen-declared-outputs.test.ts (2), second-texts-of-modules.test.ts, internal-module-blob.test.ts, the two source lints and shell-completions-list-flags.test.ts. The first two need AOT_ENGINE_DIRECTORY here, because this container has no WebKit checkout.
  • test/internal/builtins-table/: 124 pass and 9 fail on the debug build. The 9 run the table generator under the debug build and go over their own 120 s limit. With a release Bun as the runner, their five files pass.
  • The 15 other files of the description: 1617 pass and 1 fails. The failure is "should be the node version on the host that we expect" in process.test.js, as before.
  • Not run: macOS and Windows. lazy_sqlite3.h compiles on macOS only.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

…f-time node:http texts

- `bun_aot::diagnostics::log_internal_error` and its re-export. Nothing
  calls it.
- `$Node.OutgoingMessageConstructor` and `$Node.ClientRequestConstructor`
  in `src/js/ahead-of-time/node`. Nothing names these type declarations.
@robobun

robobun commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

A later run of the dead-code sweep added commit 3cbe3a5 here instead of opening another pull request. It removes 22 more lines that #45631 left unused: log_internal_error in bun_aot and two $Node type declarations in src/js/ahead-of-time/node. The description lists them under "Removed, third commit", with the check against every open head and the items that run found and left alone.

@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 the latest push (including the third commit) and found no bugs; since src/js/builtins.d.ts is a CODEOWNERS-covered path, a human look is still the right final step.

What was reviewed:

  • Grepped the tree for every deleted symbol: the only surviving emitEventWithArgs caller (src/js/node/net.ts) imports from internal/trace_event_recorder, which keeps its export; the three Bun::vm(globalObject) sites (bindings.cpp x2, InspectorTestReporterAgent.cpp) all take JSC::JSGlobalObject*, so the remaining overload still binds; no non-64 sqlite3_bind_blob/text/text16 call exists outside the vendored amalgamation headers.
  • Checked each platform-gated #include removal against an earlier include in an enclosing or implied scope (e.g. BunProcess.cpp <sys/stat.h> is unconditional at line 49; c-bindings.cpp <atomic>, <signal.h>, <pthread.h> are in both arms of !OS(WINDOWS)).
  • Confirmed write_internal_errors, listAccessors, createField/createReceiver keep callers after their wrappers were removed, and that the non-AOT _http_* twins never declared the removed $Node types.
Extended reasoning...

The PR is a pure deletion sweep (+1/-149 across 28 files): unused zsh helpers, a Rust re-export, codegen helpers, TypeScript type-only declarations, a C++ class and member, a Bun::vm overload, three lazy-sqlite entries, commented-out code, and duplicate includes. It touches no security-sensitive surface; the crypto and sqlite files only lose redundant includes and never-called function-pointer slots. Every removed symbol was grepped for remaining references and none was found, and each removed include was traced to an equivalent include in an enclosing or implied preprocessor scope. Approval is withheld only because *.d.ts is covered by CODEOWNERS and the cfg-gated include removals cannot be compiled here; no third-party objections are outstanding in the timeline.

No conflict. The branch was three commits behind main (#47490, #47512,
#47513). Those commits make the binary smaller, and the binary-size check
compares a pull request build with the latest build of main.
No conflict. The pipeline builds a branch only if it contains
3f3a9ff (#47625).
Open pull request #47635 merges with main and changes a line in the body
of `signatureOfBindgen`. The removal of that function made the two
conflict. Put the function back, with `unplacedPath` and the two type
imports that only it uses. The function still has no caller.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

Four files conflict. main (#47942, #48120) already removed
`log_internal_error` from src/aot/diagnostics and the
`PreallocatedEvents` declaration from internal/streams/readable.ts, so
those files take main's text. In ahead-of-time/node/_http_outgoing.ts
the unused `OutgoingMessageConstructor` interface is removed again, on
main's text.

`internal/trace_events` keeps its `emitEventWithArgs` export. Since
#48120, node:net reads the function from that module.
Open pull request #48356 calls `notImplementedIssueFn` twice in
src/js/builtins/BundlerPlugin.ts at its head. Put the rule of
src/codegen/replacements.ts and the declaration of src/js/builtins.d.ts
back. No source on main calls the macro.
…exts

#48120 changed the body of `$Node.OutgoingMessageConstructor` and kept
the interface. So the owners of these texts maintain the declarations,
and a sweep does not remove them. `$Node.ClientRequestConstructor` is
the same kind of declaration. Both still have no user.
@robobun robobun changed the title Remove dead code from the zsh completions, the SQLite lazy loader, the C++ bindings, and the builtins codegen Remove dead code from the zsh completions, the SQLite lazy loader, and the C++ bindings Nov 10, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — pure deletion, and each removed line checks out as dead on the rebased tree.

What was reviewed:

  • Removed symbols: no remaining uses of symbolFromDynamicLibrary as a member (only the function parameter in Bun__CreateFFIFunctionValue), ArgumentInFrame, bun_sha::, the zsh helpers, or the non-64 lazy sqlite binders outside the vendored sqlite3_local.h declarations.
  • Bun::vm(Zig::GlobalObject*) removal: the four globalObject callers pass JSC::JSGlobalObject* (and any Zig::GlobalObject* converts to the surviving overload); clientData->bunVM and m_bunVM are the same per-VM pointer, as the neighboring Bun::bunVM() helper already assumes.
  • Removed includes: every one is still included in the same TU either unconditionally or in the enclosing/sibling arm of the same platform guard (<sys/stat.h> line 53 of BunProcess.cpp; <atomic> in both arms of c-bindings.cpp; <wtf/WTFConfig.h>, <signal.h>, <pthread.h> in the !OS(WINDOWS) block), and the webcore/crypto ones are exact duplicates of an earlier line.
  • Not done: no build or test run on any platform; cross-platform compilation was checked by reading the guards only.
Extended reasoning...

The change deletes 83 lines across 18 files with nothing added: two unused zsh functions, an unused Rust extern-crate alias, an unused bindgen helper class, an unread FFI member, a redundant Bun::vm overload, three non-64 lazy sqlite binder entries, duplicate or redundant #include lines in nine C++ files, and stale commented-out code. It touches no security-sensitive surface (the crypto files only lose a duplicate include, the FFI change removes a write to a field nothing reads). Every removed symbol was grepped for remaining users and every removed include was confirmed still present in the same translation unit under the same platform conditions, so the only unverifiable claim is a compile on each platform, which reading the guards makes very unlikely to fail. No CODEOWNERS entry covers the changed files, no CHANGES_REQUESTED review is outstanding, and the three commits since the last review rebased onto current main and dropped the hunks that had been overtaken, which narrowed rather than widened the scope.

No conflict. main is one commit ahead (#48436), and that commit changes
no file of this diff.

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.

2 participants