Repository navigation
Conversation
|
Updated 2:44 PM PT - Oct 21st, 2026
❌ @robobun, your commit b1a896d has 1 failures in
🧪 To try this PR locally: bunx bun-pr 46140That installs a local version of the PR into your bun-46140 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughChangesArrayBuffer allocator
WebSocket constructor helper
Subprocess cached accessors
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the primary change: removing unused code from JSWebSocket, ArrayBuffer::alloc, and Subprocess. |
| 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. |
| Description check | |
The description clearly explains the problem, the removed code, the compile-time assertion, the lack of intended behavior change, and the verification performed. It does not use the template headings … |
Full details: Description check
-
Explanation
The description clearly explains the problem, the removed code, the compile-time assertion, the lack of intended behavior change, and the verification performed. It does not use the template headings exactly, but it provides the required information and includes detailed test results and known baseline failures.
✨ 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}
|
Status: ready for review. The diff is green. The one red CI job is not from this change.
|
- getWebSocketConstructor: no caller since the ws module moved to the native binding. - ArrayBuffer::alloc: no caller passes JSType::Uint8Array, so the arm and Bun__allocUint8ArrayForCopy have no user. A const assert makes any other kind fail the build. - Subprocess: nothing calls the stdout and stderr cached accessors.
ede1b9f to
b1a896d
Compare
Problem
getWebSocketConstructor(JSWebSocket.cpp) has no caller. WebSocket: real closing handshake, one failure path,wsrewritten on native transports (consolidates the open WebSocket PRs) #45223 removed the last one.ArrayBuffer::allocpassesJSType::Uint8Array. That arm is the only user ofBun__allocUint8ArrayForCopy. Thestdoutandstderrentries of theSubprocesscached accessor list have no caller.Fix
const { assert!(..) }toalloc. A kind other thanArrayBufferfails the build, as@compileErrordid in the Zig original.--gc-sectionsdebug link drops both C++ functions.bun bd,bun run rust:check-all(12 targets),cargo check --workspace --all-targets, and the tests in Notes. Self-reviewed: 23 concerns raised, 16 addressed (rejected ones in Notes).Background
ArrayBuffer::allocselects the kind of JS object with a const generic. Each kind is onematcharm.codegen_cached_accessors!declares Rust wrappers for the value slots of a generated JS class. The two entries declare only hand-written wrappers. The slots and the generated shims stay.alloc: keep both arms (Remove dead code from react_compiler, the WebCore/IDL bindings, install_jsc, node-fallbacks, and misc crates #39420), or drop the const generic (tried in Remove dead code from Progress, Tag::infer and bun_jsc::Error #45974, withdrawn). This shape adds no conflict inarray_buffer.rsto an open pull request.Downsides
alloc::<{ JSType::Uint8Array }>must restore the arm and the C++ function.cargo checkdoes not evaluate the assert. A full build does.Behaviour change: none
Notes
Removed
WebCore::getWebSocketConstructor(JSWebSocket.cpp,JSWebSocket.h, 7 lines). Its only caller was$cpp("JSWebSocket.cpp", "getWebSocketConstructor")insrc/js/thirdparty/ws.js:149. WebSocket: real closing handshake, one failure path,wsrewritten on native transports (consolidates the open WebSocket PRs) #45223 replaced that file withws.ts, which usescreateWebSocketBindingForWs.JSType::Uint8Arrayarm ofArrayBuffer::allocand theBun__allocUint8ArrayForCopyextern (src/jsc/array_buffer.rs, 8 lines), and the C++ function (src/jsc/bindings/ZigGlobalObject.cpp, 12 lines). The three calls ofalloc(src/runtime/node/node_crypto_binding.rs:618,:1244,:1266) passJSType::ArrayBuffer.stdoutandstderrincodegen_cached_accessors!("Subprocess"; ...)(src/runtime/api/bun/subprocess.rs, 2 lines). Nostdout_get_cached,stdout_set_cached,stdout_take_cachedorGc::stdoutuse exists, and the same holds forstderr.generate-classes.tsstill emits theSubprocessPrototype__{stdout,stderr}{Get,Set}CachedValueshims and thejs_Subprocesswrappers. This PR does not change them.Added
const { assert!(KIND.0 == JSType::ArrayBuffer.0, "ArrayBuffer::alloc: KIND not implemented") }inalloc, the pattern ofsrc/uws_sys/socket.rs:718. On the pinned toolchain aUint8Arrayinstantiation passes a metadata-only compile (cargo check) and fails a full build witherror[E0080]: evaluation panicked: ArrayBuffer::alloc: KIND not implemented. Thematchand itspanic!arm stay as they are.createhas the same run-timepanic!arm for a kind it does not handle. This PR does not touch it.Reverses an earlier hold
Uint8Arrayarm and its binding "soallockeeps mirroringcreate". The bodies of Remove dead code from bun init, bun create, bun_md, bun_install, and the socket and Postgres code #44102 and Remove dead code from Progress, Tag::infer and bun_jsc::Error #45974 repeat that hold. The one kind ofallocthat has callers returns a NodeBuffer(JSBufferSubclassStructure()inBun__allocArrayBufferForCopy), soallocdoes not mirrorcreate. This PR removes the arm together with its C++ function, so no half stays behind.Uint8Arraycall panic at run time. The assert makes that call a build failure. It is the guard a maintainer asked for in the same situation in Remove dead code from bun_jsc, bun_core, the bindgenv2 codegen, 15 error types, and the C++ bindings #40915 (JSDOMAttribute.h).Found dead and left alone
us_nq_settings_set_scid_lenandus_nq_settings_set_delay_onclose(packages/bun-usockets/src/node_quic_shim.c,src/lsquic_sys/lib.rs): no caller. Remove dead code from the bake client, uSockets, uWS HTTP/2, uws_sys, and the CI pipeline script #43976 and Remove dead code from bun_lint, bun_sema, BunProcess.cpp, and NodeDirent.cpp #45764 held them becausenode:quicis under active work, and node:quic: remove unsafe from the endpoint, session, stream and TLS code #40220 edits the line above them.headers,url,statusText(JSResponse) andheaders,url,signal(JSRequest) injs_class_module!(src/jsc/generated.rs:261-262): no wrapper has a caller. Remove cross-crate layering workarounds #33152 moves theJSResponseline, and theJSRequestline is next to it.JSWebSocket::toWrapped(JSWebSocket.cpp, directly above the removed function): Remove dead code from the WebCore bindings, the JSSink codegen, and two Rust helpers #39929 deletes it.resultValueslot ofExpect(jest.classes.ts): no user on main. Draft bun test: Vitest compatibility #44833 reads and writes it.Bun__JSBigIntStatsObjectConstructor: onlyStatType::<true>::get_constructorreaches it, and nothing instantiates that. A removal needs a change to a live generic function.*_getterfunctions fromWEBCORE_GENERATED_CONSTRUCTOR_GETTER(ZigGlobalObject.cpp): the macro also defines*ConstructorCallback, which has users.How the items were found
-Wl,--gc-sections -Wl,--print-gc-sections: the linker drops 229 hand-written C and C++ functions. Open pull requests delete 160 of them.cargo mordant(unused_pub) for Linux, Windows and macOS: 160 findings, equal tomordant-baseline.toml.matcharm on a const generic, and two are proc-macro output.Open pull requests
JSWebSocket.cpp(7 to 8 files). It deletestoWrappednext to the removed function. Keep both deletions.JSWebSocket.cppconflict hunks 1 to 2JSWebSocket.cpp(11 to 10 files)Self-review
allocchange into its own pull request. No issue tracks the arm, and Remove dead code from react_compiler, the WebCore/IDL bindings, install_jsc, node-fallbacks, and misc crates #39420, Remove dead code from bun init, bun create, bun_md, bun_install, and the socket and Postgres code #44102, Remove dead code from bun_format and napi.cpp #45420 and Remove dead code from Progress, Tag::infer and bun_jsc::Error #45974 each named it and left it.alloc. Remove dead code from Progress, Tag::infer and bun_jsc::Error #45974 tried it and put it back because of Make unrooted JSValues, views across JS re-entry, and JSC buffer-ownership contracts compile errors #34230, Bun.DurableObject #42984, Rename Zig-prefixed identifiers to Bun/Rust equivalents #30747 and Remove cross-crate layering workarounds #33152.generated.rsentries (it conflicts with Remove cross-crate layering workarounds #33152).Where each check ran
bun bd,cargo check --workspace --all-targetsand the tests below: b1a896d on main 9634520.bun run rust:check-all(12 targets): ac09a8c, the same diff on main 3880151.Tests (debug build,
bun bd test)test/js/first_party/ws(21 files): 20 files pass (3,106 pass, 80 skip).ws-syscall-fault.test.tsneeds more than the 5 s default on the debug build: 3 pass with--timeout 60000(5.3 s to 5.6 s each).test/js/web/websocket/websocket-client.test.ts: 104 passtest/regression/issue/11866.test.ts: 1 passtest/js/node/crypto/crypto-random.test.ts: 23 pass, 1 skiptest/js/node/crypto/scrypt.test.ts: 4 passtest/js/bun/spawn/spawn.test.ts: 181 pass, 8 skip, 2 fail:gcTick > spawn > stdout can be read(5 s timeout on the debug build) and theBUN_FEATURE_FLAG_FORCE_WAITER_THREADcase that runs the file again. The same timeout occurred on a debug build of main 3880151 without this change.no test proof · iteration 1 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check