[robobun]

Behaviour change: none

Problem

• Three member functions in src/jsc/bindings/ncrypto.cpp have no caller: CryptoErrorList::pop_back(), CipherCtxPointer::isWrapMode() and BignumPointer::reset(const unsigned char*, size_t).
• Merged pull requests removed the last callers. node:crypto, webcrypto, tls: more Node compatibility fixes #47567 removed errorStack.pop_back() (node/crypto/CryptoUtil.cpp). node:crypto, webcrypto: a third batch of Node compatibility fixes #47591 removed m_ctx.isWrapMode() (node/crypto/JSCipherPrototype.cpp). node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671 removed four two-argument reset calls (node/crypto/CryptoPrimes.cpp).
• No check in CI reports an unused C++ function that has external linkage.

Fix

• Delete the three definitions and their declarations: 23 lines in ncrypto.cpp and ncrypto.h, no added line.
• Correct because a missed caller cannot compile: the declarations go too, and bun bd builds and links. BignumPointer::reset(BIGNUM*) and Cipher::isWrapMode() (a method of another class) keep their callers.
• Verified: bun bd, then eight crypto test files (1233 tests pass) and 28 Node.js test-crypto-* files (Notes).
• Self-reviewed: 27 concerns raised on two larger first cuts, 21 addressed, 6 not taken (Notes).

Background

• ncrypto is Bun's copy of the OpenSSL wrapper classes of Node.js. CryptoErrorList captures the OpenSSL error queue. CipherCtxPointer owns an EVP_CIPHER_CTX. BignumPointer owns a BIGNUM.
• No other design was weighed: a deletion has one place.

Downsides

• None found for users. No code calls the three functions, so no code path changes.
• Binary size: no change in a release build, because the release link already drops a function that nothing references (CI numbers in Notes). The debug binary loses the three functions (715 bytes).

Notes

What is removed (line numbers on main 2596ac5)

• src/jsc/bindings/ncrypto.cpp: CryptoErrorList::pop_back() (97-104), BignumPointer::reset(const unsigned char*, size_t) (261-265) and CipherCtxPointer::isWrapMode() (2922-2927), each with the blank line after it.
• src/jsc/bindings/ncrypto.h: the three declarations (122 with one blank line, 554 and 640).

Commits

• The first commit deletes pop_back() and isWrapMode().
• A merge of main (2596ac5) brings node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671 into the branch.
• The last commit deletes the reset overload. It cannot come before the merge, because the first base still has the four callers.

Checks (on the merged head, unless a line names another commit)

• rg -w pop_back over src/ and packages/ finds three calls. Each is on a standard container: sni_tree.cpp:65, Http2Context.h:887 and lint_js.rs:923.
• rg isWrapMode finds Cipher::isWrapMode() (definition, declaration, and its call at node/crypto/JSCipherConstructor.cpp:196) and nothing else.
• A scan of each C and C++ file in src/ and packages/ for a reset with two or more arguments finds DataPointer::reset(void*, size_t) (called at ncrypto.cpp:165) and the grapheme state classes of sliceAnsi.cpp and stringWidth.cpp. No call is on a BignumPointer.
• bun bd (debug, ASAN) builds and links. clang-format (LLVM 23) reports nothing on the two files.
• Sizes in the debug object of ncrypto.cpp on main: 503 bytes (pop_back), 124 bytes (isWrapMode) and 88 bytes (reset).
• A relink of the debug binary with -Wl,--gc-sections dropped the first two functions (on the first base, 6cdb383). I made no such relink for the reset overload.
• The release link has -Wl,--gc-sections on Linux, -dead_strip on macOS and /OPT:REF on Windows (scripts/build/flags.ts).
• CI measures the release binaries. The binary-size report of build 138552 compares 12 targets with main build 138398. Its 12 numbers equal those of build 138422 (docs: say that mock.restore() restores spies and leaves other mocks as they are #48153, documentation only) and of build 138485 (test: wait for the warmup stream's descriptor to close in tty.test.ts #48167, one test file).
• Tests on that build, all pass (1233): test/js/node/crypto/node-crypto.test.js (326), crypto.test.ts (467), crypto.key-objects.test.ts (112), crypto-rsa.test.js (45), x509.test.ts (62), ecdh.test.ts (16), crypto-random.test.ts (27) and test/js/web/crypto/web-crypto.test.ts (178).
• In the first run of crypto-random.test.ts, directly after the build, randomInt does not call RAND_bytes per iteration reached the 5 s limit. It passes alone in 1.1 s and in the second run of the file.
• These files of test/js/node/test/parallel/ exit with 0. For ciphers and signatures: test-crypto-cipheriv-decipheriv.js, test-crypto-authenticated.js, test-crypto-authenticated-stream.js, test-crypto-padding.js, test-crypto-padding-aes256.js, test-crypto-aes-wrap.js, test-crypto-des3-wrap.js, test-crypto-getcipherinfo.js, test-crypto-key-objects.js, test-crypto-sign-verify.js and test-crypto-rsa-dsa.js.
• For primes, Diffie-Hellman and random numbers: test-crypto-prime.js, test-crypto-random.js, test-crypto-ecdh-convert-key.js, test-crypto-keygen-dh-classic.js, test-crypto-dh.js and the 12 files test-crypto-dh-*.js (constructor, curves, errors, generate-keys, group-setters, leak, modp2, modp2-views, odd-key, padding, shared, stateless).
• 13 of these 28 files print a skip line: 12 for a cipher, a group or a key type that BoringSSL does not have, and test-crypto-dh-leak.js because the build has ASAN.
• For the first two functions, a script with 19 cipher and error cases printed the same output on main (6cdb383) and on the first commit.
• No Rust file changes, so bun run rust:check-all does not apply.

Open pull requests

• First two functions (heads fetched on 2026-11-02): of the 4804 open pull requests of robobun and the 1199 of other authors, none deletes one of them and none adds a call.
• The reset overload (2026-11-08): I read the diffs of the 49 open pull requests that name ncrypto, BignumPointer, CryptoPrimes or BN_bin2bn. None deletes the overload and none adds a call. crypto: accept ArrayBuffer and SharedArrayBuffer for generatePrime options.add/rem #38680 deletes the four old callers, as node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671 did.
• Remove dead code from ZigGlobalObject, FuzzilliREPRL.cpp, BunBuiltinNames.h and launch.json #47672 found pop_back() one day earlier and left it, because Remove dead code from the JSC, WebCore, and node bindings, uWS/uSockets, SecretsLinux, and seven Rust crates #42431 deletes the two lines above its declaration (rbegin(), rend()). This diff does not touch those lines, and a three-way merge with Remove dead code from the JSC, WebCore, and node bindings, uWS/uSockets, SecretsLinux, and seven Rust crates #42431 has no conflict in ncrypto.h.
• 20 of these 49 pull requests change ncrypto.cpp, ncrypto.h or CryptoPrimes.cpp. For each, a trial merge with this branch conflicts in the same files as a trial merge with main. Six merge cleanly with both: crypto: port OpenSSL's prime search for generatePrime with add #37963, node:crypto: implement encapsulate() and decapsulate() #40550, crypto: apply a UTCTime timezone offset in the X509Certificate validity strings and Dates #42415, node:crypto: keep the sign/verify context bytes on the job and decide support in runTask #45621, Remove dead code from JSDOMWrapper.h and ncrypto.cpp: two preprocessor blocks that never compile #48067 and node:crypto: refuse BLAKE2b for RSA signatures, read detached buffers as empty, name the key argument like Node #48086.

Found dead in this run, and left alone

• ncrypto, named by code but never true: Cipher::isWrapMode(), CipherCtxPointer::setAllowWrap() and the branch at JSCipherConstructor.cpp:196. BoringSSL never returns EVP_CIPH_WRAP_MODE (vendor/boringssl/include/openssl/cipher.h:425). The sweep removes what no code names, so these stay.
• ncrypto, unused but not a plain deletion: CryptoErrorList::empty(), the Option enum, and the CryptoErrorList* parameter of ClearErrorOnReturn and MarkPopErrorOnReturn (no guard passes one). They are not left over from node:crypto, webcrypto, tls: more Node compatibility fixes #47567, node:crypto, webcrypto: a third batch of Node compatibility fixes #47591 or node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671, and the removal edits live constructors. rbegin() and rend() are in Remove dead code from the JSC, WebCore, and node bindings, uWS/uSockets, SecretsLinux, and seven Rust crates #42431.
• ncrypto, special member functions with no caller: the default and move constructors and release() of ECDSASigPointer, ECGroupPointer and ECPointPointer, and the move constructor of HMACCtxPointer. The removal makes the classes non-movable. That needs a maintainer's decision.
• The resource timing constructors (82 lines in 6 files): PerformanceResourceTiming::create has no caller, so its constructor, the ResourceTiming constructor, populateServerTiming and PerformanceServerTiming::create never run. Remove dead code: 15 trait impls nothing uses and two C++ overloads #47264 held them because Remove dead code from bun_core, the JSC bindings, the debugger, and 24 other crates #41385 deletes a line inside the ResourceTiming constructor. The cut also leaves ResourceTiming.cpp with one allocator macro and ServerTiming.h with no user but ServerTiming.cpp, which Remove dead code from the build scripts, the vscode and debug-adapter packages, the bake overlay, and two WebCore files #40122 deletes. The unit needs one pull request of its own and a maintainer's answer: keep the classes for performance.markResourceTiming(), or reduce them to the two globals. (State of 2026-11-02.)
• The WebIDL converters for IDLByte, IDLShort and IDLLongLong, 14 convertToInteger* specializations with toSmallerInt, and eight JSConverter specializations that nothing on main instantiates (365 lines). bindgen: delete b.any and .variadic #46789 maps the bindgen i64 to IDLLongLong, and the bun:dom draft bun:dom and the Vitest work in one tree (not for merging) #45415 uses the rest. Remove dead code from ZigGlobalObject, FuzzilliREPRL.cpp, BunBuiltinNames.h and launch.json #47672, Remove dead code from the zsh completions, the SQLite lazy loader, and the C++ bindings #47439 and Remove dead code from headers.h, the REPL shims, builtin-names.ts and the builtins code generator: what #45631 left unused #47324 held them for the same reason. (State of 2026-11-02.)
• The linker drops 293 functions of src/jsc/bindings and packages/ (on 6cdb383). An open pull request deletes at least 169 of them (Remove dead code from the JSC bindings, headers.h, the console builtin, and bun_jsc #40232 has 45, Remove dead code from the WebCore bindings, the JSSink codegen, and two Rust helpers #39929 has 43). Others have a user that the Linux debug link does not see: bsd_socket_export, bsd_socket_import, us_loop_integrate and uws_get_loop_with_native (Windows), Bun__prepareTrimmedExecutable and posix_spawnattr_reset_signals (macOS), Bun__askForAddressSpaceToStart (builds without ASAN), JSPerformanceEntry::subspaceForImpl (builds without assertions), Bun__CallFrame__describeFrame (the lldb btjs command) and JSC__ArrayBuffer__ref (a required trait method).
• Rust (on 6cdb383): a probe (below) reports 114 items with no user. Open pull requests delete 106 of them. bun_sys::windows_impl::umask, UvHandle::data, bun_errno::posix::mode_t and MEMORYSTATUS_PRESSURE_NORMAL are 17 lines and stay for a later run. bun_zstd::inflate_embedded* looks unused, but a macro arm under cfg(bun_codegen_embed) calls it. 52 trait impls that the linker drops were not tested. Remove dead code: 15 trait impls nothing uses and two C++ overloads #47264 tested 84 of that kind and the compiler needed 68.

The self-review

• The first cut had 484 lines and the second 99. The review asked for this shape: the ncrypto functions alone, the resource timing constructors held.
• It also showed that my comparison with open pull requests read the pull requests of robobun only. That is how the WebIDL converters got into the first cut.
• The reset overload joined after the review. It has the shape the review asked for: one definition and one declaration with no caller, in the same two files.
• Not taken, 3 concerns: open no pull request and leave a note on node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671, or add a commit to Remove dead code from ZigGlobalObject, FuzzilliREPRL.cpp, BunBuiltinNames.h and launch.json #47672. The sweep is asked for a pull request when a run finds dead code, and the first commit does not touch a line of node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671.
• Not taken, 2 concerns: make this pull request a carry-over of about 950 lines from open sweeps that conflict with main, or restore files on the branches of Remove dead code from bun_core, the JSC bindings, the debugger, and 24 other crates #41385 and Remove dead code from the build scripts, the vscode and debug-adapter packages, the bake overlay, and two WebCore files #40122 to release their lines. The sweep must not repeat a deletion of an open pull request. That question is with a maintainer.
• Not taken, 1 concern: keep the resource timing cut here. The other concerns and the verdict say to hold it.

The Rust probe, for the next run

• #[deprecated(note = "<id>")] went on 16,839 items: each function with no symbol in the --gc-sections link, each module-level const, static and type alias, and each struct, enum and trait with no live method. Then cargo check --workspace --all-targets ran for Linux, Windows and macOS with the lint at warn. Each warning is one use. An item that no code outside the set reaches has no user. One pass takes about 8 minutes.
• Traps: bun_ptr allows deprecated for the whole crate. A file can start with #![cfg(...)]. A const fn and an #[inline(always)] function have no symbol. A macro arm under a cfg that no check enables hides its uses.

---

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]

^{Updated 3:04 AM PT - Nov 8th, 2026}

❌ @robobun, your commit 8731992 has 1 failures in Build #138552 (All Failures):

• test/aot/loader-access-in-unreachable-code.test.ts - code 1 on 🐧 13 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 47813

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

bun-47813 --bun

[robobun]

Status: ready for review. CI is red on one test that this diff does not touch.

This pull request has no repro. It deletes three C++ member functions that have no caller: 23 lines in ncrypto.cpp and ncrypto.h, no added line.

How they were checked:

• A search of src/ and packages/ finds no other use of CryptoErrorList::pop_back, CipherCtxPointer::isWrapMode or the two-argument BignumPointer::reset. node:crypto, webcrypto, tls: more Node compatibility fixes #47567, node:crypto, webcrypto: a third batch of Node compatibility fixes #47591 and node:crypto, webcrypto: a fourth batch of Node compatibility fixes #47671 removed the last callers.
• bun bd (debug, ASAN) builds and links. The declarations go too, so a missed caller cannot compile.
• Tests on the local build: eight crypto test files (1233 pass) and 28 Node.js test-crypto-* files.
• In CI build 138552 all 14 build-bun jobs pass. The 12 release binaries have the same sizes as those of a branch that changes only documentation (build 138422).

The branch now contains main at 2596ac5. The third function, the BignumPointer::reset overload, joined on 2026-11-08. The old base still had its four callers, so the merge of main comes first.

CI build 138552 is finished: 187 of 188 jobs pass. The one red test is test/aot/loader-access-in-unreachable-code.test.ts on Debian 13 aarch64. It compares the sizes of two executables that bun build --compile --aot makes. It allows a difference of 32 KB. It fails on each of its 4 attempts here and passes on main at the same base (build 138398).

The same test is red with the same message in 19 builds of other branches since 2026-11-07. Two examples are builds 138554 and 138536. The 20 failing jobs ran on 20 different CI machines, and one of those machines also ran a job that passes. So the result follows the content of the build, not the machine. The test can also turn red on main after a merge of this pull request. This diff does not touch the ahead-of-time compiler.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Two methods, CryptoErrorList::pop_back() and CipherCtxPointer::isWrapMode(), were removed from the ncrypto bindings, including their declarations and definitions.

Changes

Crypto helper API removals

Layer / File(s)	Summary
Remove method declarations and implementations src/jsc/bindings/ncrypto.h, src/jsc/bindings/ncrypto.cpp	The declarations and definitions of CryptoErrorList::pop_back() and CipherCtxPointer::isWrapMode() were removed.

Suggested reviewers: jarred-sumner, dylan-conway

Merge Risk: Minimal · up to 96ac4

No actionable merge-blocking risk is identified in these helper removals. Mergeable subject to normal build and test checks.

Pre-merge checks: 4

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 names the main change: removing dead code from ncrypto, specifically CryptoErrorList::pop_back and CipherCtxPointer::isWrapMode, plus a BignumPointer::reset overload. The raw summary lists o…
Description check		The description follows the repository template. It has both required sections, "What does this PR do?" and "How did you verify your code works?". The Problem and Fix sections state the change, why it…

Full details: Title check

Explanation

The title names the main change: removing dead code from ncrypto, specifically CryptoErrorList::pop_back and CipherCtxPointer::isWrapMode, plus a BignumPointer::reset overload. The raw summary lists only the first two removals and omits the reset overload, which the description and title include. The title is long (129 characters), but it is specific and matches the changeset.

Full details: Description check

Explanation

The description follows the repository template. It has both required sections, "What does this PR do?" and "How did you verify your code works?". The Problem and Fix sections state the change, why it is safe, and the scope. The Verified line lists the build, test files, and pass counts. The description is long and dense, and a large part of it sits in a collapsed Notes block that covers open pull requests, dead code found elsewhere, and a Rust probe. That material goes beyond this change. The core content still satisfies the template, so the check passes.

• 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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: robobun/06ca9e3d/dead-code-sweep
• File: .coderabbit.yaml
• Commit: 96ac414
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: c969311d-ec86-43ac-bd2e-2a31492128c7

Commits

Reviewing files that changed from the base of the PR and between 6cdb383 and 96ac414.

Files selected for processing: 2

• src/jsc/bindings/ncrypto.cpp
• src/jsc/bindings/ncrypto.h

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

[claude]

LGTM — a pure deletion of two member functions with no callers; declarations go with definitions, so any missed caller would fail to compile rather than silently change behavior.

What was reviewed:

• Searched src/ and packages/ for isWrapMode and pop_back: the only isWrapMode call (JSCipherConstructor.cpp:196) is on a Cipher, whose method is untouched; remaining pop_back calls are on std containers.
• Checked that neither symbol is a C++ member that Rust could import via extern "C", and that no generated binding names them.
• CODEOWNERS does not cover ncrypto.cpp/.h.

Extended reasoning...

The change removes 17 lines from src/jsc/bindings/ncrypto.cpp and ncrypto.h: the definition and declaration of CryptoErrorList::pop_back and CipherCtxPointer::isWrapMode, with nothing added or modified. It touches the crypto bindings layer, but only by deleting dead code, so no cipher, key, or error-handling behavior changes. Repo-wide search confirms no remaining callers, the identically named Cipher::isWrapMode is a different class and keeps its caller, and a missed call site would be a compile error, not a runtime regression. The bug hunt ran dry, no CODEOWNER covers these files, and there are no open objections, which decided approve.

[claude]

LGTM — pure deletion of three unreferenced member functions, declarations and definitions removed together, so any missed caller would fail to compile rather than misbehave.

What was reviewed:

• Searched all of src/ and packages/ for isWrapMode, pop_back(), BN_bin2bn, and two-argument .reset( calls: the only isWrapMode() caller (src/jsc/bindings/node/crypto/JSCipherConstructor.cpp:196) is on Cipher, whose own isWrapMode is untouched; no CryptoErrorList::pop_back or BignumPointer::reset(data, len) call sites exist, including platform-gated code.
• The latest commit (8731992) also removes BignumPointer::reset(const unsigned char*, size_t), which the PR title and description do not mention — worth adding to the description for the record.
• Build/link and tests were not run in this review; the no-caller conclusion rests on reading and repo-wide search.

Extended reasoning...

The diff removes 23 lines from src/jsc/bindings/ncrypto.cpp and ncrypto.h: CryptoErrorList::pop_back, the two-argument BignumPointer::reset overload, and CipherCtxPointer::isWrapMode, with no additions and no changes to any live function. It touches the crypto helper layer but only deletes definitions with no callers, so there is no reachable behavior or security surface change. Repo-wide search confirmed no remaining references in src/ or packages/ and that the still-used Cipher::isWrapMode is a different class's method left intact. The changed files are not covered by CODEOWNERS, and since the prior review the PR gained a merge of main plus one additional equally mechanical deletion. The mechanical nature of the change and compile-time enforcement of the no-caller claim decided approval.