Repository navigation
Conversation
…torHTTPServerAgent.cpp, and JSBuffer.cpp Delete five declarations that nothing references: - ExceptionOr.h: the free function isolatedCopy(ExceptionOr<void>&&) - Exception.h: the free function isolatedCopy(Exception&&), whose one caller was the function above - ErrorCode.h: the alias ErrorCodeCache::Field - InspectorHTTPServerAgent.cpp: the typedefs RouteId and RequestId - JSBuffer.cpp: the member JSBuffer::typeRange
Collaborator
Author
|
Updated 9:09 AM PT - Oct 17th, 2026
✅ @robobun, your commit 34b42e02e6eb6f1ea24bccd8113868d1b3512424 passed in 🧪 To try this PR locally: bunx bun-pr 45725That installs a local version of the PR into your bun-45725 --bun |
Collaborator
Author
|
Status: ready for review. How each deletion was verified:
This PR deletes code only. It has no test because no caller can observe a difference. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThis change removes exception-copy overloads and several type declarations and constants from JavaScriptCore bindings. The ChangesBinding declaration cleanup
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly identifies the primary change: removing dead code from the listed binding files. |
| Description check | |
The description explains the changes, confirms no behavior change, documents verification steps and test results, and provides relevant background. It does not use the exact template headings, but it … |
Full details: Description check
-
Explanation
The description explains the changes, confirms no behavior change, documents verification steps and test results, and provides relevant background. It does not use the exact template headings, but it includes the required information.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
LGTM — five pure deletions of unreferenced declarations, each confirmed to have no consumer.
What was reviewed:
- The deleted free
isolatedCopy(Exception&&)/isolatedCopy(ExceptionOr<void>&&): every cross-thread copy of anExceptionin the tree (the WebCryptocrossThreadCopy(WTF::move(result))path and theCrossThreadCopierBase<..., ExceptionOr<T>>specializations inExceptionOr.h) passes aconst Exception&, which WTF routes to the memberException::isolatedCopy() constthat is kept; no unqualified rvalue call exists insrc/orpackages/. JSBuffer::typeRange: the localJSBufferclass is used only forJSBuffer::info()/s_infoand is never ajsCast/inheritstarget; JSC reads type ranges fromInheritsTraits<T>specializations, not a class member.RouteId/RequestIdtypedefs: no remaining C++ use, theRoutestruct passed over the C ABI usesint route_iddirectly, so no ABI effect;ErrorCodeCache::Fieldhas no references.
Extended reasoning...
The change is a single commit removing 21 lines across five files in src/jsc/bindings: two free rvalue isolatedCopy overloads, a type alias in ErrorCodeCache, two unused extern "C" typedefs, and a static constexpr member on a ClassInfo-only class. It touches no security-sensitive surface and no CODEOWNERS entry covers these paths. I confirmed by grep that each symbol has no caller, and traced the one non-obvious path (WTF crossThreadCopy on ExceptionOr results from the WebCrypto work queue) to the retained member isolatedCopy() rather than the deleted free functions. The bug hunt exited on dry_streak with no findings, and the deletions cannot change runtime behavior if they compile, which CI will confirm.
alii
pushed a commit
that referenced
this pull request
Oct 18, 2026
…ent.cpp (#45764) ### Problem - 29 lines in 6 files have no user. - `source_file`, `compare_types` and `symbol_to_string` in the `queries!` list of `src/lint/types/mod.rs` have no caller. The list generates the only call of the `Services` functions of the same names in `bun_sema`. The trait is `pub`, so rustc does not check it. - `BunProcess.cpp` declares five `using X = JSC::X;` aliases that it never names. `DirEntType` in `NodeDirent.cpp` has three enumerators that nothing names. ### Fix - Delete the three list lines with their `Services` functions (21 lines), the five aliases and the three enumerators. - Correct because a word search over the sources, the tests and the generated code finds no user, and the workspace compiles for 12 targets. - Verified: `bun bd`, `bun run rust:check-all`, `cargo check --workspace --all-targets`, 8 test files (Notes). - Self-reviewed: 41 concerns raised, 33 addressed. Rejected: a `pub(crate)` narrowing of `Queries` (`resolve_name` needs a caller first), and a hold until earlier sweeps merge. ### Background - `queries!` lists each question that a lint handle (`Type`, `TsSymbol`) can ask the type checker. One line generates a `Queries` method and a forwarding call to `Services` in `bun_sema`. - `BunProcess.cpp` compiles as its own unit with `using namespace JSC;`, so the aliases change no name lookup. - A deletion of unreferenced code has one possible place, so no other design was weighed. ### Downsides - A later lint handle that needs one of the three queries must restore it. - None found for users. Checked: the heads of all 5,559 open pull requests, 12 targets, the tests in Notes. Behaviour change: none <details><summary>Notes</summary> **Removed** - `src/lint/types/mod.rs`: the `queries!` lines `source_file`, `compare_types` and `symbol_to_string`. No call of these names has a `Queries` receiver. The calls that exist go to other functions: `Checker::compare_types` and `Checker::symbol_to_string` in `bun_sema`, a private `compare_types` in `prefer_optional_chain.rs`, and the parser's `source_file`. With `Queries` made crate-private in a scratch edit, rustc reports `resolve_name` as the only query left that is never used. - `src/sema/check/services.rs`, `services/types.rs`, `services/symbols.rs`: `Services::source_file`, `Services::compare_types`, `Services::symbol_to_string`. The generated forwarding call was the only user. What they call (`Files::by_path`, `Checker::compare_types`, `Checker::symbol_to_string`, `Checker::prop_to_string`) keeps other callers. - `src/jsc/bindings/BunProcess.cpp`: the aliases `Exception`, `JSModuleLoader`, `JSModuleRecord`, `SourceOrigin`, `JSNonFinalObject`. None of the names appears in the rest of the file. The same block in `Path.cpp` and `ZigGlobalObject.cpp` is left to #43378 and #42431. - `src/jsc/bindings/NodeDirent.cpp`: the enumerators `Whiteout`, `Door` and `EventPort` of `DirEntType`, all equal to `Unknown`. No C or C++ file names them. **Open pull requests** - Compared at 2026-10-17 16:40 UTC on main 017b243 with the heads of all 5,559 open pull requests, drafts and every author included. - 156 of them touch a file of this diff (151 touch `BunProcess.cpp`). A trial merge of each with main and with this branch (`git merge-tree`) shows that the branch adds a conflict to none. - None deletes one of these lines. None adds a use of a removed name: the added lines were searched for the three query names, the five alias names in `BunProcess.cpp`, and the three enumerators. **Found dead in this run, and left alone** - Already gone on main: three lines in `JSSQLStatement.cpp` and `NodeSqlite.cpp` (#45304). - Another open pull request deletes or rewrites them: 13 commented-out lines in uSockets, uWS, `JSEventTargetCustom.cpp` and `src/node-fallbacks/url.js` (#45709, and #43730 for `url.js`), two typedefs in `InspectorHTTPServerAgent.cpp` (#45725), the `BUN_WEBKIT_VERSION` fallback in `BunProcess.cpp` (#41729), a commented-out line in `src/js/internal/html.ts` (#45682). - An open pull request uses them: `Event::currentTargetIsInShadowTree()` and the `MediaTime` overload of `JSConverter<IDLUnrestrictedDouble>` (#45415). - Held by an earlier sweep for a reason that still holds: the QUIC setters `scid_len` and `delay_onclose` (#43976, and #40220 changes the line above them). - `resolve_name`, the fourth query with no caller. tsgolint's `prefer-readonly` calls `ResolveName` and Bun's port does not, which gives a wrong autofix for a global enum. The fix is handed off and needs the query. After it, `Queries` can become `pub(crate)`, and rustc then guards the list. - Mirrors of typescript-eslint and tsgolint: the write-only fields `ThenCall::on_fulfilled`, `FinallyCall::on_finally`, `BaseTypeOfClassMember::base_type` and `heritage_token`, `TargetElement::is_shorthand`, with `TsNode::token` (as #45402 held them). Also 33 functions from macro lists in `src/lint/types/` that nothing calls, and `File::expr_tags_in_tree`. - Debugging aids: 25 hand-written `Debug` impls in `bun_lint`, 5 in `bun_format`, and `Display` for `Difference`, `FilePoll` and `Part`. - Handed off: the plugin `target` value that nothing reads (`BundlerPlugin::target`, three more members, a parameter of `JSBundlerPlugin__create`, and the Rust enum). Its removal changes an FFI signature. - Needs a decision: `PerformanceResourceTiming` cannot be constructed, so `ResourceTiming.cpp`, `NetworkLoadMetrics.cpp` and `ServerTiming.h` are reachable only from its getters. **How the candidates were found** - Ten text searches, one for each area: `src/js`, the C++ bindings (three parts), uSockets and uWS, the build scripts, the side packages, the three new crates, and commented-out code. - A relink of the debug build with `-Wl,--gc-sections`, with each dropped function mapped to its source line by `llvm-nm -l`: 517 C++ functions and 607 Rust function lines. Almost all are Windows or macOS code, release-only code, or are deleted by an open pull request. - For `bun_sema`: `pub` narrowed to `pub(crate)` on each item that no other crate names, in a scratch tree. rustc reported nothing new. - For 39 trait impls that the linker drops: delete all, compile for linux, windows and darwin and with `--all-targets`, put back each impl that an error names. What stayed deleted is a debugging aid. **Tests, debug build of this branch** (with `--timeout 600000`, because the lint and check tests need more than the default with a debug build) - `test/cli/lint/conformance.test.ts`: 3 pass. - `test/cli/lint/rules.test.ts`: 16 pass. - `test/cli/lint/native-rules.test.ts`: 166 pass. - `test/cli/lint/bun-rules.test.ts`: 100 pass. - `test/cli/check/check.test.ts`: 599 pass, 42 skip. - `test/js/node/process/process.test.js`: 188 pass, 5 skip. - `test/js/node/fs/dir.test.ts`: 23 pass. - `test/regression/issue/24129.test.ts`: 2 pass. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check <!-- robobun:evidence:end -->
This was referenced Nov 4, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Behaviour change: none
Problem
src/jsc/bindingshave no reference. No tool reports them. A name search and a read of the code found them.isolatedCopy(Exception&&)andisolatedCopy(ExceptionOr<void>&&). WebKit deleted the same two in 2022 (WebKit/WebKit@5b8ad34f804c).Fix
isolatedCopyfunctions together.isolatedCopy(Exception&&)goes because its one caller goes in the same change.ErrorCodeCache::Field, the typedefsRouteIdandRequestId, andJSBuffer::typeRange.bun bdlinks.bun bd testpasses fortest/js/node/buffer.test.js(718 tests),test/cli/inspect/HTTPServerAgent.test.ts(1) andtest/js/web/crypto/web-crypto.test.ts(95).Background
ExceptionandExceptionOrare Bun's copies of WebCore's error types.WTF::CrossThreadCopiercopies a value for another thread through the memberisolatedCopy(), which stays.ErrorCodeCachecaches one error structure per Node error code in its internal fields.ErrorCode.cppindexes them withstatic_cast<unsigned>(code), not throughField.CREATE_METHOD_TABLEreadsJSCastingHelpers::InheritsTraits<JSBuffer>::typeRange, not a member of the class. SoJSBuffer::typeRangehas no effect.Downsides
Exception.h. It addsexceptionWithMessage()directly after the deleted function. Keep its new function and keep the deletion.Notes
Removed, with the check for each (all run on this branch):
src/jsc/bindings/ExceptionOr.h:isolatedCopy(ExceptionOr<void>&&).git grep -P '(?<![.>\w~])isolatedCopy\s*\(\s*[^)\s]' -- src packagesfinds no call of a freeisolatedCopy.src/jsc/bindings/Exception.h:isolatedCopy(Exception&&). Its one caller was the function above. Keep both deletions or neither: with only theExceptionOr.hfunction restored, its call binds to itself through theExceptionOr(Exception&&)constructor.src/jsc/bindings/ErrorCode.h:using Field = ErrorCode;inErrorCodeCache.git grep 'ErrorCodeCache::Field' -- srcfinds nothing.src/jsc/bindings/InspectorHTTPServerAgent.cpp:typedef int RouteId;andtypedef int RequestId;.git grep -w -E 'RouteId|RequestId' -- 'src/*.cpp' 'src/*.h'finds nothing else. They belong to HTTP server request events that the agent never sends. Remove dead code from the inspector agent, the WebKit IDL copies, and an orphaned package #41064 has the five uncalled methods above them (requestWillBeSenttorequestHandlerException). This change adds no conflict to it.src/jsc/bindings/JSBuffer.cpp:JSBuffer::typeRange.git grep -w typeRange -- srcfinds only theInheritsTraitsspecializations inJSEventCustom.handBindgen/JSWrapper.h.Open pull requests, checked on 2026-10-17 09:47 UTC, directly before the push:
git merge-treeof each of the 46 against main and against this branch shows one new conflicted file:Exception.hin Headers: throw instead of aborting when a value passes the string length limit #42218, which already conflicts with main in two other files.Not deleted, because another pull request already has it or uses it:
src/js/bun/sql.tsdefault export: sql: consolidate the open Bun.sql, bun:sqlite and node:sqlite pull requests #45304 already merged the same change.pathOutinsrc/jsc/modules/BunJSCModule.h: Write the sampling profiler report at exit from Bun, not JSC's atexit hook #41137 already has the same hunk.bun_sys::posix::s_isdir: dead on main, but fs.watch: watch symlink entries of a recursive tree on Linux #43090 adds a caller.src/jsc/bindings/BunAnalyzeTranspiledModule.h, which only repeats two declarations ofheaders-handwritten.h: Build every runtime ES module's record from the transpiler output instead of a second JSC parse #35605 adds a declaration and an includer to it.extern "C" bool isBunTest;inModuleLoader.cpp: a redundant redeclaration, and Share one value between import and require() of a data file, per loader #35973 inserts a function directly under it.NestedCmakeBuildfields inscripts/build/source.ts: Remove dead code from the code generators, the build scripts, and ten crate roots #45709 has them.Probably dead, left for a maintainer to decide:
Bun__signpost_emitand its#if OS(DARWIN)block inc-bindings.cpp: no Rust declaration and no caller. A comment insrc/bun_core/util.rssays that the macOS signpost path is not wired yet.myersDiffinsrc/js/internal/assert/myers_diff.tsand its native chain: nothing reads it since assert: render Myers diffs natively #39924. It mirrors Node'sinternal/assert/myers_diff, which a vendored Node test can load with--expose-internals.AsyncContextFrame.exchangeinsrc/js/internal/async_context_frame.ts: no caller. The file header says that it ports the Node surface.src/runtime/ffi/FFI.h. A removal changes the text thatviewSource()prints.<Class>_getterfunctions thatWEBCORE_GENERATED_CONSTRUCTOR_GETTERdefines inZigGlobalObject.cpp. A removal needs a second macro.Self-review concerns that this PR does not take:
Question for a maintainer: 57 dead-code sweep pull requests from this account are open, and none has merged since #43976 on 2026-09-26. Should the sweep continue to open standalone pull requests?
no test proof · iteration 0 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check