Skip to content

Remove dead code from the JSC FFI bindings, the uws HTTP/2 glue, and misc sources - #40881

Merged
Jarred-Sumner merged 1 commit into
mainfrom
robobun/287486ec/dead-code-ffi-shims
Aug 30, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
robobun/287486ec/dead-code-ffi-shims

Conversation

@robobun

@robobun robobun commented Aug 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The C++ side of the Rust/C++ FFI still defines shims that no Rust code calls. The Rust side now implements JSValue::is_int32, is_null, is_object, js_type, and friends inline (src/jsc/JSValue.rs), but the [[ZIG_EXPORT]] / CPP_DECL functions they replaced stayed in src/jsc/bindings/bindings.cpp and headers.h, and cppbind.ts still emitted Rust wrappers for them.
  • src/uws_sys/libuwsockets_h2.cpp defines seven uws_h2_res_* C shims with no Rust declaration, and one (uws_h2_res_override_write_offset) whose only Rust wrapper nothing calls.

Fix

  • Delete 21 functions from bindings.cpp and their 15 headers.h declarations: JSC__JSValue__{isCell, isCustomGetterSetter, isGetterSetter, isInt32, isInt32AsAnyInt, isNull, isNumber, isObject, isUndefined, isUndefinedOrNull, jsNumberFromChar, jsNumberFromU16, jsTDZValue, jsType}, JSC__JSMap__has, JSC__JSPromise__isHandled, JSC__JSInternalPromise__{reject, resolve}, JSC__VM__{isJITEnabled, isTerminationException, performOpportunisticallyScheduledTasks}.
  • Delete uws_h2_res_{override_write_offset, get_write_offset, prepare_for_sendfile, get_native_handle, get_socket_data, uncork, is_corked} and Response::override_write_offset in src/uws_sys/h2.rs.
  • Delete the unused BREAKING_CHANGES_BUN_1_2 macro in JSSQLStatement.cpp, the unreachable #else branches of two #if 1 blocks in CryptoAlgorithmRSA_PSSOpenSSL.cpp, and the never-exported SlowBuffer in src/node-fallbacks/buffer.js.
  • Verified: every symbol has zero references in src/**, packages/**, build/debug/codegen/**, and the prebuilt WebKit libraries. bun bd links, bun run rust:check-all passes on all 12 targets. Ran serve.test.ts, serve-http2.test.ts, serve-http2-protocol.test.ts, sqlite.test.js, web-crypto.test.ts, buffer.test.js, bundler_browser.test.ts.
  • No test: unreferenced code has no observable behavior, and test/internal/source-lints/CLAUDE.md asks for no dead-symbol lint tests.

Background

  • [[ZIG_EXPORT(mode)]] marks a C++ function that src/codegen/cppbind.ts scrapes into build/*/codegen/cpp.rs as a Rust extern "C" declaration plus a safe wrapper. Rust calls the wrapper as bun_jsc::cpp::Name. A function with no Rust caller is unreachable: C++ never calls these shims directly.
  • headers.h carries hand-written CPP_DECL prototypes for the same extern "C" surface. Both sides were trimmed together.
  • The prebuilt WebKit libraries reference a few Bun__* symbols and define two weak defaults that Bun overrides (Bun__thisThreadHasVM, Bun__analyzeTranspiledModule). None of the deleted symbols is in either set.
Notes

How the candidates were found:

  • Per crate, every pub Rust item with zero textual references outside its crate was rewritten to pub(crate), then cargo check -p <crate> was run with dead_code as a warning, on linux, windows, and darwin targets. Items flagged on all three targets and not referenced from macro bodies ($crate::) were the Rust candidates. After excluding files and symbols that the 13 open dead-code PRs already remove, the only survivor was Response::override_write_offset. The workspace already compiles with dead_code = "deny" and unreachable_pub = "deny", so private dead code cannot exist.
  • nm --defined-only over every object in build/debug/obj and libbun_runtime.a, minus nm --undefined-only over all objects and the WebKit libraries (libWTF.a, libJavaScriptCore.a, libbmalloc.a, including their weak symbols), gave the set of C-linkage symbols nothing references at link time. A test link with --gc-sections --print-gc-sections (without -rdynamic) confirmed the same set. Codegen-emitted symbols (*__fromJS, *GetCachedValue, JSSink *__createObject) were excluded because they are regenerated from .classes.ts.
  • Every remaining bun C++ translation unit was also compiled with -fsyntax-only -Wunused-function -Wunused-template -Wunused-member-function. The 14 warnings were either in files open PRs already edit or header templates used by other translation units.
  • src/js/** and src/node-fallbacks/** were checked export by export. oxlint already enforces no-unused-vars there, and all 235 bundled files and every internal/* export have consumers.

Not touched on purpose:

…misc sources

Delete 21 extern "C" functions in bindings.cpp (and their headers.h
declarations) that no Rust code calls: JSC__JSValue__isInt32 and the other
JSValue predicates the Rust side implements inline, JSC__JSMap__has,
JSC__JSPromise__isHandled, JSC__JSInternalPromise__reject/resolve,
JSC__JSValue__jsNumberFromChar/U16, jsTDZValue, jsType, JSC__VM__isJITEnabled,
isTerminationException, and performOpportunisticallyScheduledTasks.

Delete seven uws_h2_res_* C shims in libuwsockets_h2.cpp that have no Rust
declaration, plus the one Rust wrapper (Response::override_write_offset)
nothing calls.

Drop an unreferenced macro in JSSQLStatement.cpp, the unreachable #else
branches of two #if 1 blocks in CryptoAlgorithmRSA_PSSOpenSSL.cpp, and the
unexported SlowBuffer helper in the browser buffer fallback.
@robobun

robobun commented Aug 29, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:35 PM PT - Aug 28th, 2026

❌ @robobun, your commit 37b0212 has 1 failures in Build #108168 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 40881

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

bun-40881 --bun

@coderabbitai

coderabbitai Bot commented Aug 29, 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: Pro

Run ID: b95f51f0-0d83-4d98-a89b-c5d6ace008de

📥 Commits

Reviewing files that changed from the base of the PR and between b49398c and 37b0212.

📒 Files selected for processing (8)
  • src/jsc/JSValue.rs
  • src/jsc/bindings/bindings.cpp
  • src/jsc/bindings/headers.h
  • src/jsc/bindings/sqlite/JSSQLStatement.cpp
  • src/jsc/bindings/webcrypto/CryptoAlgorithmRSA_PSSOpenSSL.cpp
  • src/node-fallbacks/buffer.js
  • src/uws_sys/h2.rs
  • src/uws_sys/libuwsockets_h2.cpp
💤 Files with no reviewable changes (7)
  • src/uws_sys/h2.rs
  • src/node-fallbacks/buffer.js
  • src/jsc/bindings/webcrypto/CryptoAlgorithmRSA_PSSOpenSSL.cpp
  • src/uws_sys/libuwsockets_h2.cpp
  • src/jsc/bindings/sqlite/JSSQLStatement.cpp
  • src/jsc/bindings/headers.h
  • src/jsc/bindings/bindings.cpp

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


Walkthrough

Changes

The pull request removes obsolete JavaScriptCore and HTTP/2 bindings, simplifies RSA-PSS signing and verification, removes a SQLite compatibility macro, deletes the Node SlowBuffer fallback, and updates js_type documentation.

Binding and runtime cleanup

Layer / File(s) Summary
JavaScriptCore bridge cleanup
src/jsc/JSValue.rs, src/jsc/bindings/...
Removed unused JSValue, promise, Map, and VM bridge APIs. Updated the js_type documentation.
Crypto and SQLite path cleanup
src/jsc/bindings/webcrypto/..., src/jsc/bindings/sqlite/...
RSA-PSS signing and verification now select the key digest directly. Removed the obsolete SQLite compatibility macro.
Node fallback cleanup
src/node-fallbacks/buffer.js
Removed the internal SlowBuffer function.
HTTP/2 API cleanup
src/uws_sys/h2.rs, src/uws_sys/libuwsockets_h2.cpp
Removed unused response write-offset, sendfile, native-handle, socket-data, and cork-state APIs.

Suggested reviewers: jarred-sumner

Merge Risk: ⚪ Minimal · up to 37b02

This change removes unused native bindings and unreachable code without changing intended runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the primary change: removing dead code from JSC FFI bindings, uWebSockets HTTP/2 glue, and miscellaneous sources.
Description check ✅ Passed The description provides detailed problem, fix, background, verification steps, test coverage, and implementation notes. It does not use the template headings exactly, but it contains the required inf…
Full details: Description check

Explanation

The description provides detailed problem, fix, background, verification steps, test coverage, and implementation notes. It does not use the template headings exactly, but it contains the required information and is complete.


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

@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 dead-code removal with no behavior change.

Grepped every deleted symbol (JSC__JSValue__is*/jsType/jsNumberFromChar/jsNumberFromU16/jsTDZValue, JSC__JSPromise__isHandled, JSC__JSInternalPromise__reject/resolve, JSC__VM__isJITEnabled/isTerminationException/performOpportunisticallyScheduledTasks, JSC__JSMap__has, uws_h2_res_{get_write_offset,override_write_offset,prepare_for_sendfile,get_native_handle,get_socket_data,uncork,is_corked}, override_write_offset) across the tree — zero remaining references. headers.h prototypes and the Rust extern were removed in lockstep with the .cpp definitions; the few [[ZIG_EXPORT]]-only ones (isCell, isNull, isUndefined, isUndefinedOrNull) never had a headers.h decl to begin with. SlowBuffer in the browser fallback was never exported; the #if 1 unwrap in RSA-PSS and the BREAKING_CHANGES_BUN_1_2 macro removal are trivially no-ops (the test/harness.ts constant of the same name is unrelated JS).

Extended reasoning...

Overview

This PR deletes ~150 lines of unreferenced code across eight files: 21 C++ FFI shim functions in src/jsc/bindings/bindings.cpp with their matching CPP_DECL prototypes in headers.h, seven uws_h2_res_* C wrappers in src/uws_sys/libuwsockets_h2.cpp plus the one Rust extern and Response::override_write_offset wrapper in src/uws_sys/h2.rs, an unused BREAKING_CHANGES_BUN_1_2 macro in JSSQLStatement.cpp, two always-true #if 1 guards in CryptoAlgorithmRSA_PSSOpenSSL.cpp, the never-exported SlowBuffer helper in src/node-fallbacks/buffer.js, and a doc-comment reword in src/jsc/JSValue.rs that drops a stale bindings.cpp:2755 line reference. Only three lines are added, all in the reworded comment.

Security risks

None. Nothing added is executable; every hunk is a deletion of code that had no reachable caller, or an unwrap of an #if 1 that leaves the compiled output byte-identical. The RSA-PSS change keeps the exact same sign/verify body that was already active. No auth, permission, or input-validation path is touched.

Level of scrutiny

Low-to-medium — the only real risk in a dead-code PR is deleting something that still has a caller hidden behind codegen, a #[cfg] gate, or an external linker reference. I grepped the full tree for every removed symbol name and found zero hits outside the deleted lines themselves; the headers.h declarations were removed in the same commit as their definitions, and the [[ZIG_EXPORT]]-only functions that lacked a headers.h entry (isCell, isNull, isUndefined, isUndefinedOrNull) are confirmed absent from headers.h on HEAD. The PR description documents nm --undefined-only checks against the prebuilt WebKit libraries and bun run rust:check-all across all 12 targets, which covers the platform-gated-code concern.

Other factors

REVIEW.md explicitly requires deleting dead code in the PR that makes it dead and updating every consumer atomically — this change does exactly that (definition + header prototype + Rust extern removed together). No CODEOWNERS pattern matches the changed files. The bug-hunt exit reason was dry_streak with no findings and no ruled-out candidates. The one remaining SlowBuffer string in src/node-fallbacks/buffer.js is inside a comment, and the BREAKING_CHANGES_BUN_1_2 constant in test/harness.ts is an unrelated TypeScript export. The PR notes adjacent-hunk overlap with two other open PRs, but the deletions are disjoint and any conflict is a trivial rebase.

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