Remove dead code from jsc FFI glue, C++ bindings, bun_core, http_types, and bunfig - #37012
Remove dead code from jsc FFI glue, C++ bindings, bun_core, http_types, and bunfig#37012robobun wants to merge 11 commits into
Conversation
…s, options, and bunfig
Every symbol was verified to have zero references across src/, scripts/,
and freshly regenerated build/debug/codegen output, then validated by
cargo check on all 10 CI target triples plus a full debug build.
- bun_jsc: DeprecatedStrong::unref, unused extern imports of
JSCommonJSExtensions__{appendFunction,setFunction,swapRemove},
ZigString__free, Zig__GlobalObject__reportUncaughtException,
AbortSignal__Timeout__run, Resolver__propForRequireMainPaths
- C++ bindings: the now-unreferenced JSCommonJSExtensions
m_registeredFunctions vector (writers, GC visit loop, member) and the
stale header prototypes for the removed Rust exports
- bun_core: fmt::raw, the schema wrapper module, NAPI_MODULE_REGISTER
feature counter
- bun_http_types: orphan mime_type_list.txt (fully duplicated by the
hand-maintained table, not read by any build step), outbound-only
FrameType ALTSVC/ORIGIN variants a client never sends
- websocket deflate: never-constructed OutOfMemory error variants and
their unreachable match arm
- webcore: ReadableStream::detach_if_possible no-op vestige
- options/bunfig: the write-only [bundle].packages cluster
(BundlePackage enum, DebugOptions::package_bundle_map, parser block)
- perf: WASM-stub-only Timer::lap
- runtime/image: re-export names with no external users
Adds a source lint guarding against reintroduction.
WalkthroughThis change removes obsolete public APIs, FFI symbols, configuration types, runtime exports, and WebSocket error variants. It simplifies ChangesDead symbol cleanup
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline nits, I checked that the FrameType::HTTP_FRAME_{ALTSVC,ORIGIN} removal from http_types/h2.rs does not affect runtime/api/bun/h2_frame_parser.rs — that file defines its own local FrameType enum (line 241) rather than importing the bun_http_types one. Also confirmed the removed #[no_mangle] FFI exports (ZigString__free, Zig__GlobalObject__reportUncaughtException, AbortSignal__Timeout__run, Resolver__propForRequireMainPaths) and m_registeredFunctions have no remaining references anywhere under src/jsc/bindings/.
Extended reasoning...
Large 30-file dead-code sweep touching Rust↔C++ FFI export surface, a JSC GC visitChildren loop, and bunfig parsing — worth a human confirming the m_registeredFunctions WriteBarrier-vector removal and the bunfig [bundle].packages drop. The three inline findings are cosmetic nits only; spot-checks of the riskier removals (h2 FrameType variants, FFI exports, analytics counter) held up.
- ReadableStream::done: drop the now-unused JSGlobalObject parameter (its only use was the deleted detach_if_possible call) and update all 17 call sites - DeprecatedStrong: drop SafetyData::ref_count, which became a write-once constant with a vacuous debug_assert once unref was removed - NodeModuleModule: replace the comment describing the deleted C++ WriteBarrier vector with one matching the Strong-based storage
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Addressed the three review findings in 1d44393 and trimmed the two flagged comments in 410121e:
Re-verified: cargo check on all 10 triples, full debug build, and the streams/require-extensions/source-lint suites (176 tests) pass. |
A stash round-trip restored the deleted file in the working tree and a blanket git add swept it back into the previous commit.
With m_registeredFunctions gone the override visited nothing custom; the method table inherits Base::visitChildren without it.
With no data members left it has nothing to destruct: the declared destructor was never defined (nothing odr-used it), and subspaceForImpl selects the non-destructible cell type from the base automatically.
|
Updated 4:57 AM PT - Aug 6th, 2026
✅ @Jarred-Sumner, your commit 8041adce46f8f28e9e35774bda8b3b2cc60b5404 passed in 🧪 To try this PR locally: bunx bun-pr 37012That installs a local version of the PR into your bun-37012 --bun |
…signature enable_safety's only invocation was the const that suppressed its own unused-macro warning. The WASM reset stub returned u64 while the real impl returns unit, so no portable caller could use the value.
There was a problem hiding this comment.
No issues found in this pass — all six earlier nits have been addressed. I'm deferring rather than approving because the sweep is broad (34 files, −2.4k lines) and several removals sit on load-bearing boundaries: the JSCommonJSExtensions base-class change to JSNonFinalObject (subspace/GC), six #[no_mangle] extern "C" exports at the Rust↔C++ FFI edge, and the [bundle].packages bunfig parse block (write-only and undocumented, but previously validated its inputs). Worth a human sanity-check.
What was reviewed
- Confirmed zero remaining references in
src/to the six removed FFI exports and the threeJSCommonJSExtensions__*C++ writers. - Verified
h2_frame_parser.rshas its own localFrameTypeenum, so removingHTTP_FRAME_ALTSVC/ORIGINfromhttp_types/h2.rsis isolated. - Checked
[bundle].packagesis absent fromdocs/runtime/bunfig.mdx— undocumented as claimed. ReadableStream::done()signature change: the removeddetach_if_possiblebody was already empty, so dropping theglobal_thisparam across 17 call sites is behavior-preserving.
Extended reasoning...
Overview
Dead-code sweep across 34 files (net −2419 lines): removes uncalled #[no_mangle] FFI exports (ZigString__free, Zig__GlobalObject__reportUncaughtException, AbortSignal__Timeout__run, Resolver__propForRequireMainPaths), unused Rust extern imports of JSCommonJSExtensions__{append,set,swapRemove}Function and the C++ m_registeredFunctions WriteBarrier vector they wrote, orphan mime_type_list.txt (2310 lines), the write-only [bundle].packages bunfig cluster, never-constructed websocket-deflate OutOfMemory variants, unused h2 FrameType variants, fmt::raw, the bun_core::schema shim module, an unused analytics counter, the WASM-only Timer::lap, unused image:: re-exports, and the no-op ReadableStream::detach_if_possible (with done() losing its global_this param across 17 call sites). Follow-up commits (in response to earlier review rounds) additionally changed JSCommonJSExtensions's base from JSDestructibleObject to JSNonFinalObject, dropped its visitChildren override and never-defined destructor, removed DeprecatedStrong's ref_count/enable_safety! machinery, and aligned the WASM Timer::reset signature.
Security risks
None identified. The changes are pure removals of unreferenced code plus mechanical signature updates; no auth, crypto, input parsing, or trust-boundary logic is touched. The only user-visible behavior change is that an undocumented [bundle].packages bunfig key no longer validates package-name shape (it previously emitted "Expected package name" for non-package keys but never used the result) — now it falls through like any other unknown key.
Level of scrutiny
Moderate-to-high. Individually most hunks are trivially verifiable deletions, and the PR's verification is thorough (rg across src/scripts/codegen, 10-triple cargo check, full debug build, targeted smoke suites, plus a source-lint guard test). But the aggregate touches memory-safety-adjacent surfaces the REVIEW.md flags as the most-blocked category: a JSC cell's base class and subspace selection, removal of a GC-visited WriteBarrier vector, and six extern "C" symbols at the Rust/C++ ABI seam. Each removal's correctness depends on a negative claim ("nothing calls this") that codegen or a platform-gated path could invalidate; the PR description notes two such near-misses caught only by the multi-triple check. The JSNonFinalObject change relies on subspaceForImpl keying off is_base_of_v<JSDestructibleObject, T> — sound per robobun's follow-up, but structural enough to merit a maintainer glance.
Other factors
I have already reviewed this PR across six prior passes, each surfacing a nit that was promptly fixed (vestigial visitChildren, the JSDestructibleObject base, the self-suppressing enable_safety! macro, the WASM reset signature mismatch, a broken [\raw`]intra-doc link, and a stale audit-note citation). All are resolved at HEAD (2b0a08d9). This run I spot-checked the FFI-export removals (zero residual references insrc/), confirmed h2_frame_parser.rsdefines its ownFrameTypeso thehttp_typesvariant removal is isolated, and verified[bundle].packages` is absent from the bunfig docs. Given the breadth and the FFI/GC surface, deferring for a human review rather than approving.
There was a problem hiding this comment.
No issues found this pass — every prior nit is addressed and resolved. Deferring to a human because of scope: 33 files across FFI exports, JSC C++ bindings (JSCommonJSExtensions base-class change), bunfig parsing, and the ReadableStream::done() signature fan-out.
What was reviewed:
- Grepped src/ for every removed
#[no_mangle]/extern-"C" symbol (ZigString__free, Zig__GlobalObject__reportUncaughtException, AbortSignal__Timeout__run, Resolver__propForRequireMainPaths, JSCommonJSExtensions__*) — zero remaining references. - Verified the JSDestructibleObject → JSNonFinalObject switch against
WebCore::subspaceForImplin BunClientData.h — theis_base_of_vbranch selectscellHeapCellTypeand theneedsDestructionstatic_assert holds. - Confirmed
BundlePackage/package_bundle_map/detach_if_possible/NAPI_MODULE_REGISTERhave no remaining readers; the removeddetach_if_possiblewas already an empty body, sodone()is behavior-preserving.
Extended reasoning...
Overview
Dead-code sweep removing ~2.4k lines across 33 files: unused #[no_mangle] FFI exports and their C++ header prototypes, the JSCommonJSExtensions::m_registeredFunctions WriteBarrier vector plus its extern-C writers, the write-only [bundle].packages bunfig cluster, an orphan 2310-line MIME data file, never-constructed error variants, a WASM-only Timer::lap, the no-op ReadableStream::detach_if_possible, and assorted stale re-exports. Follow-up commits addressed cascading dead code my earlier passes flagged (empty visitChildren, JSDestructibleObject base, self-suppressing enable_safety!, WASM reset() signature, stale intra-doc links). The final commit (8041adc, pushed by a maintainer) deletes the source-lint test file per REVIEW.md's "do not add tests to check dead code stays dead".
Security risks
None identified. No auth, crypto, path-handling, or input-validation logic is touched; removals are orphaned symbols and no-op call chains. The bunfig [bundle].packages block that is removed only populated a map nothing ever read, so dropping it cannot weaken any check.
Level of scrutiny
High. This is not a mechanical rename or config tweak — it removes #[no_mangle] exports whose only consumers would be C++ or generated code (grep is necessary but not sufficient), changes a JSC cell's base class and therefore its heap subspace, drops an undocumented-but-user-reachable bunfig key, and rewrites a ReadableStream method signature across 17 call sites in server/webcore hot paths. Each removal looks correct on inspection and the author's verification (10-triple cargo check, full debug build, targeted test suites) is thorough, but the blast radius warrants a maintainer's eyes.
Other factors
I spot-checked the highest-risk items: (1) WebCore::subspaceForImpl in BunClientData.h keys the heap-cell type on std::is_base_of_v<JSDestructibleObject, T> with a needsDestruction static_assert, so the JSNonFinalObject base compiles to the non-destructible cell type as claimed; (2) none of the removed extern-C symbols appear anywhere in src/; (3) detach_if_possible already had an empty body, so removing it and the done() parameter is behavior-preserving. All eight of my prior inline findings were nits and are resolved. A maintainer has already pushed a commit to this branch, so they are aware of it — I'm deferring rather than approving given the breadth and the GC-adjacent C++ change.
2 participants
Dead-code sweep across crates not covered by the other open dead-code PRs (#36237, #35775, #36970, #36115, #35437, #35880). Net -2419 lines. Every removal was verified to have zero references across src/, scripts/, and freshly regenerated build/debug/codegen output, then validated by cargo check on all 10 CI target triples and a full debug build.
Removed
bun_jsc FFI glue
DeprecatedStrong::unref: never called; the sole user (test_runner/Collection.rs) usesinit+ Drop only, per the file's own audit noteJSCommonJSExtensions__{appendFunction,setFunction,swapRemove}in NodeModuleModule.rs: zero Rust call sitesZigString__free: exported fn whose only cross-reference was its prototype in headers-handwritten.h; no C++ caller (helpers.h frees viaZigString__freeGlobal, which stays)Zig__GlobalObject__reportUncaughtException: exported fn with no C++ caller; the internal safe wrapperreport_uncaught_exceptionstaysAbortSignal__Timeout__run: orphaned wrapper; AbortSignal.cpp calls only__create/__deinit, and the timeout actually fires through the Rust timer dispatch (EventLoopTimerTag::AbortSignalTimeout)Resolver__propForRequireMainPaths: no caller; removed together with its stale extern decl in JSCommonJSModule.cpp (getterPaths usesResolver__nodeModulePathsJSValue)C++ bindings
JSCommonJSExtensions::m_registeredFunctionsvector mechanism: with the Rust imports above gone, its three extern "C" writers had zero callers, so the member and its GC-visit loop were dead with them (custom require.extensions functions are stored in Rust-side Strongs)bun_core
fmt::raw: shorthand constructor duplicatings(), zero callers (theRawstruct stays, it is constructed vias)schemawrapper module in lib.rs: zero users; everyone imports the flattenedbun_core::StringPointerNAPI_MODULE_REGISTERfeature counter: never incremented, and bun_analytics has no mapping for itbun_http_types
mime_type_list.txt(2309 lines): orphan data file fully duplicated by the hand-maintained table in mime_type_list_enum.rs; not read by any build step (absent from cargo dep-info), referenced only by two doc comments, both updatedFrameType::HTTP_FRAME_ALTSVC/HTTP_FRAME_ORIGIN: the enum is used only for outbound frame construction (inbound dispatch is on raw u8 per the file comment) and a client never sends these frameswebsocket client
CompressError::OutOfMemory/DecompressError::OutOfMemory: never constructed (compress returns only DeflateFailed; decompress only InflateFailed/TooLarge), plus the unreachable match arm in websocket_client.rsbun_runtime
ReadableStream::detach_if_possible: no-op vestige whose body was emptied in 8553428; removed with its sole call indone()image/mod.rs: trimmed the flattened re-export list to the two names actually addressed viacrate::image::(Image, AsyncImageTask)options / bunfig
[bundle].packagescluster:BundlePackageenum,DebugOptions::package_bundle_map, the bunfig parse block that populated it, and two re-exports. The map had zero readers; the key is undocumented and now falls through bunfig's per-key lookup like any other unknown keybun_perf
Timer::lap: existed only in the WASM stub impl; the real Timer never defined it, so no portable caller can existVerification
rg -wfor every symbol across src/, scripts/, cmake/, and regenerated build/debug/codegen: zero references outside the definition sitesbun run rust:check-all: 10/10 triples pass (this caught and reverted two candidates that grep missed: zig_hash_mapkeys/values_mutused inside the allow(dead_code) h2 file, andfile_poll::Pollableused by the darwin ParentDeathWatchdog path)bun bddebug build passesNoted but not removed
DebugOptions::editorandDebugOptions::output_fileare write-only (bunfig[debug].editorand--output-fileparse into them, nothing reads them); they look like unported feature hooks rather than leftovers, so they are flagged here instead of deletedbun_resolver::Error::{MissingResolveDir,InvalidResolveDir}are never constructed, but the surrounding commented-out block is an intentional TODO referencing Missing Esbuild plugin API options (resolveDir, pluginData) #8994[review] gate passed · iteration 1 · 34 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 7 passed · 0 rejected · iteration 1
evidence per changed file