You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Remove dead code from the JSC prototype classes, bun_paths, bun_sys, the JSON5 parser, and two code generators - #46388
Nothing reads WriteFileArgs.encoding. No path type selects PathSeparators::Posix or PathSeparators::Windows. Nothing constructs the JSON5 parser's out-of-memory errors.
Fix
Delete the 72 templates. 15 classes had their only STATIC_ASSERT_ISO_SUBSPACE_SHARABLE inside the template, where nothing evaluated it. It now follows the class, so the check runs.
Delete the field, the variants and their branches, and the smaller items in Notes. A word search finds no user. No open pull request that merges with main adds a user or gets a new conflict (6,021 checked).
Verified: bun bd, bun run rust:check-all (12 targets), cargo clippy, unchanged generator output, 20 test files (Notes).
Self-reviewed: 19 concerns raised, 17 addressed. Rejected: a split into several pull requests and a handoff of the held items (Notes).
Background
subspaceFor tells JSC::allocateCell<T> which GC subspace holds a T. A class that allocates with Bun::allocatePlainObjectCell never asks.
The templates emitted no symbol, so that part does not change the binary.
Downsides
A later use of a removed item must restore it. A missed use is a compile error.
Size, Linux x64, CI size test against the merge base: a --compile --aot hello world maps 8,192 bytes less, and 4,096 more with --trim.
Behaviour change: none
Notes
Removed
src/jsc/bindings (67 files, 443 lines): the subspaceFor member template of 72 classes. Each class allocates with Bun::allocatePlainObjectCell(vm, sizeof(T)), and no allocateCell<T> or tryAllocateCell<T> of these classes exists in the tree or in the generated code. The -O0 debug binary of main 6f58f6b, linked without --gc-sections, had no T::subspaceFor<...> symbol for them, and had one for each class that still calls allocateCell. JSNonFinalObject has no such template in the pinned WebKit, so a later allocateCell<T> of one of these classes does not compile until the template is back.
WebCrypto and WebCore (4 files, 14 lines): the Padding parameter of CryptoAlgorithmAES_CBC::platformEncrypt and platformDecrypt (ignored, both callers pass three arguments), MessagePortPipe::operator==, StreamQueue::first() const, and the empty analyzeEntry overload for ByteQueueEntry.
src/sys, src/bundler, build_command.rs (14 lines): WriteFileEncoding and WriteFileArgs.encoding. Four writers, no reader.
src/paths/Path.rs (38 lines): PathSeparators::{Posix, Windows}. Every SEP_OPT in the workspace is ANY or AUTO, and no constant exists for the other two. The sep_dispatch! macro had one arm left, so its four uses call path::<fn>::<path::platform::Auto> directly.
src/parsers/json5.rs, JSON5Object.rs (16 lines): ParseError::OutOfMemory, Error::Oom, AddToLogError::OutOfMemory, ExternalError::OutOfMemory. The parser has no fallible allocation, so nothing constructs the first one, and each of the others comes only from the one before.
src/semver/lib.rs, Scripts.rs (16 lines): impl StringBuilder for semver_string::Builder. Every generic caller gets the lockfile's StringBuilder.
Generators (23 lines): a bindings/ filter in generate-host-exports.ts that matches no file, and the if (!tree) block in cppbind.ts (Parser.parse of @lezer/common always returns a tree).
bun bd (debug, ASAN) builds. bun run rust:check-all: 12 of 12 targets pass. cargo clippy --no-deps for the seven changed crates: no finding. rustfmt, clang-format and prettier report no change.
Generators: the scripts of main and of this branch were run into two scratch folders. cpp.rs and generated_host_exports.rs are identical.
One test fails, and this change is not the cause: process.test.js "should be the node version on the host that we expect" compares with the node binary of the machine, v26.3.0 here where the test expects v26.12.1. The release build of main fails it the same way.
Size (release builds of CI: main 3bf1a06 in build 137162, this branch in build 137225)
test/bundler/compile-aot-size.test.ts on Ubuntu 25.04 x64 prints the numbers. A bun build --compile --aot hello world maps 68,134,014 bytes on main and 68,125,822 with this branch (8,192 less). With --trim it maps 25,961,598 and 25,965,694 (4,096 more, one page). "The compiler takes" 23,663,481 and 23,663,546 bytes (65 more).
That test fails on main and on this branch: with --trim its bound is 25,880,000 bytes.
Release archives: bun-linux-x64.zip is 8,956 bytes smaller, bun-linux-aarch64.zip 5,532 and bun-windows-aarch64.zip 6,883. The other seven differ by less than 750 bytes. The archives are compressed. The binaries inside were not compared with size or bloaty: the machine for this work cannot reach the artifact store.
Open pull requests (compared on 2026-11-03 with the heads of the 6,021 other open pull requests)
Same deletions: Remove cross-crate layering workarounds #33152 moves the WriteFileEncoding lines, so it removes six of the same lines. It conflicts with main already. No other head removes a line that this pull request removes.
Conflicts: 570 pull requests touch one of the 82 files, and 164 of them merge with main. A trial merge (git merge-tree) of each of the 164 with this branch is clean.
Nothing evaluates the assert of those six classes. A typed Bun::allocatePlainObjectCell<T>(vm) that holds the assert can check every class in one place. That is a separate change.
Other classes still allocate with allocateCell<T>, for example RequireFunctionPrototype, NodeVMModulePrototype and the ProcessBinding objects. Their template is live.
Found dead on 2026-10-21, and left alone (not checked again after the rebase)
Same class as the json5 item, not checked to the end: the oom_from_alloc! stamps in src/parsers/yaml.rs.
Self-review (of the larger diff before the rebase)
Addressed: the max_output_size and BOOL_FIELD_SETTERS removals are out. ModuleGraph.cpp, NodeVMScript.cpp, JSCommonJSModule.cpp and JSEnvironmentVariableMap.h are in. The lists above were derived again from the final head.
Rejected: a split into several pull requests. The sweep is one run, and the subspaceFor part alone is 67 files.
Rejected: a handoff of the held items now. Each one waits for an open pull request or for a decision.
Status: ready for review. Rebased on main 3bf1a06 on 2026-11-03. The conflict is gone.
How the items were found: a read of the code for what the lints cannot see (templates nothing instantiates, fields with one value, impls nothing uses), then a word search for each item.
How they were checked: debug ASAN build, cargo check for 12 targets, cargo clippy, generator output before and after, 20 test files, and the heads of the 6,021 other open pull requests for uses, same deletions and new merge conflicts. The description has the details.
CI: build 137225 compiled this branch on every platform. Its red tests also fail on main in build 137162, which is the merge base of this branch. They are 28 files of ahead-of-time compile tests (test/aot and test/bundler/compile-aot-size.test.ts). This pull request does not touch that code. No other test is red.
This pull request adds no conflict to a pull request that merges with main today. It is best merged soon, before that changes.
robobun
changed the title
Remove dead code from the JSC prototype classes, the decompressors, bun_paths, and three code generators
Remove dead code from the JSC prototype classes, bun_brotli, bun_libarchive, bun_paths, and four code generators
Oct 22, 2026
No actionable comments were generated in the recent review.
Walkthrough
The PR removes the file-write encoding field and updates its callers. It also changes code generation, JavaScriptCore prototype subspace declarations, path separator handling, JSON5 error mapping, and several C++ and Rust APIs.
Changes
File write arguments
Layer / File(s)
Summary
Write-file contract and callers src/sys/lib.rs, src/bundler/OutputFile.rs, src/bundler/linker_context/writeOutputFilesToDisk.rs, src/runtime/cli/build_command.rs
WriteFileEncoding and the WriteFileArgs.encoding field are removed. Buffer write call sites no longer set the field.
WebCore prototype classes no longer define custom subspaceFor methods.
Streams and WebCrypto prototypes src/jsc/bindings/webcore/streams/*, src/jsc/bindings/webcrypto/JSCryptoKey.cpp, src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp
Stream and WebCrypto prototype classes no longer define custom subspaceFor methods.
Path separator options
Layer / File(s)
Summary
Separator options and path operations src/paths/Path.rs
PathSeparators retains Any and Auto. Separator insertion, joins, and undo no longer dispatch to separate POSIX and Windows implementations.
JSON5 parser errors
Layer / File(s)
Summary
Parser error types and mapping src/parsers/json5.rs, src/runtime/api/JSON5Object.rs
JSON5 allocation-failure error variants and their conversion paths are removed. The runtime API no longer matches the removed parser error.
AES-CBC platform methods
Layer / File(s)
Summary
Platform encrypt and decrypt signatures src/jsc/bindings/webcrypto/CryptoAlgorithmAES_CBC.h, src/jsc/bindings/webcrypto/CryptoAlgorithmAES_CBCOpenSSL.cpp
The Padding enum and the padding parameters are removed from the platform encrypt and decrypt methods. Their crypt-helper calls and error handling remain.
Stream and message-port contracts
Layer / File(s)
Summary
Queue and message-port APIs src/jsc/bindings/webcore/streams/StreamQueue.h, src/jsc/bindings/webcore/MessagePortPipe.h
StreamQueue loses its const first() overload and no-op analyzeEntry overload. MessagePortPipe loses its identity-based equality operator.
The StringBuilder implementation for semver_string::Builder is removed. Comments in both files no longer describe specific builder implementations.
Suggested reviewers: jarred-sumner
Merge Risk: Minimal · up to df078
No concrete merge-blocking issue was identified in the dead-code removals. The change appears mergeable subject to normal build and test checks.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change, removing dead code. It also lists the affected areas: JSC prototype classes, bun_paths, bun_sys, the JSON5 parser, and two code generators. The raw summary and PR desc…
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 follows the repository template with both required sections, "What does this PR do?" and "How did you verify your code works?". It states the scope (72 subspaceFor templates plus sma…
Full details: Title check
Explanation
The title names the main change, removing dead code. It also lists the affected areas: JSC prototype classes, bun_paths, bun_sys, the JSON5 parser, and two code generators. The raw summary and PR description confirm these deletions. The title is long (110 characters), but it is specific and matches the changeset.
Full details: Description check
Explanation
The description follows the repository template with both required sections, "What does this PR do?" and "How did you verify your code works?". It states the scope (72 subspaceFor templates plus smaller dead-code removals across 82 files), the rationale (no instantiation since Bun::allocatePlainObjectCell was introduced, and no readers for the removed fields and variants), and the expected impact (no behavior change, with a noted size difference). The verification section is detailed: debug and release builds, rust:check-all across 12 targets, clippy, generator output comparison, a listed set of test files and results, and one documented unrelated test failure. The description is long, but the template sections are addressed in full.
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.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
…the JSON5 parser, and two code generators
- src/jsc/bindings: remove the subspaceFor member template of 72 classes
that allocate with Bun::allocatePlainObjectCell. Nothing instantiates
it. 15 classes had their only STATIC_ASSERT_ISO_SUBSPACE_SHARABLE in
the template and get it at file scope.
- WebCrypto and WebCore: remove the ignored Padding parameter of
CryptoAlgorithmAES_CBC::platformEncrypt and platformDecrypt,
MessagePortPipe::operator==, StreamQueue::first() const, and the empty
analyzeEntry overload for ByteQueueEntry.
- bun_sys, bun_bundler: remove WriteFileEncoding and WriteFileArgs.encoding.
- bun_paths: remove PathSeparators::{Posix, Windows} and sep_dispatch!.
- bun_parsers: remove the JSON5 out-of-memory error variants that
nothing constructs.
- bun_semver: remove impl StringBuilder for semver_string::Builder.
- cppbind.ts, generate-host-exports.ts: remove a branch and a filter
that cannot apply.
robobun
changed the title
Remove dead code from the JSC prototype classes, bun_brotli, bun_libarchive, bun_paths, and four code generators
Remove dead code from the JSC prototype classes, bun_paths, bun_sys, the JSON5 parser, and two code generators
Nov 3, 2026
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the updated push and didn't find any bugs. Because this touches 82 files including GC subspace declarations across the JSC prototype classes and a WebCrypto signature, a human look would still be worthwhile, mainly to confirm the build on all platforms.
What was reviewed:
All 72 classes losing subspaceFor: each is allocated via Bun::allocatePlainObjectCell(vm, sizeof(T)) and no allocateCell<T>/tryAllocateCell<T> for any of them exists in src/; the re-emitted file-scope STATIC_ASSERT_ISO_SUBSPACE_SHARABLE lines match the deleted ones.
PathSeparators::{Posix,Windows} removal: every SEP_OPT in src/ is ANY or AUTO (no numeric 2/3 or into_sep to those), so collapsing the matches to SEP/platform::Auto is behavior-preserving.
CryptoAlgorithmAES_CBC::Padding: both callers in CryptoAlgorithmAES_CBC.cpp pass three args and the OpenSSL impl never read the parameter; json5 OOM variants had no constructor (no fallible allocation in the parser) and the one external match arm in JSON5Object.rs is removed; semver_string::Builder callers all use inherent methods, not the deleted trait impl; MessagePortPipe comparisons use raw pointers, not the deleted operator==.
Extended reasoning...
The change is a single commit of pure deletions (+40/-564) across 82 files: the subspaceFor member template on 72 JSC prototype/plain-object classes, dead Rust enum variants and fields (bun_sys WriteFileEncoding, PathSeparators Posix/Windows, json5 OOM variants, semver StringBuilder impl), an ignored Padding parameter on AES-CBC platformEncrypt/platformDecrypt, and two codegen guards. The only security-adjacent surface is the AES-CBC signature, where the removed parameter was never read, so no crypto behavior changes. I grepped every removed item for remaining users and found none, and confirmed every affected C++ class allocates through allocatePlainObjectCell. Deferring rather than approving because the size (82 files, GC class definitions) and the webcrypto touch exceed the bar for an unassisted approve, and compilation cannot be verified in this environment.
C++: each of the 71 changed files under src/jsc/bindings carries the same patch as in the earlier head c5633b3. Build 130907 compiled and tested that head on every CI platform, and 181 of 181 jobs passed. The rebase took one more file out, JSEnvironmentVariableMap.cpp.
Rust: bun run rust:check-all passes on this head for the 12 targets (Linux gnu and musl, Android, macOS, Windows and FreeBSD, each for x64 and aarch64).
Build 137225 runs the same platforms for this head.
This branch has not been deployed
No deployments
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
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.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Problem
src/jsc/bindingsdefine asubspaceFormember template. Since Trim ~2 MB from the release binary without touching hot paths #39770 they allocate withBun::allocatePlainObjectCell, so nothing instantiates it.WriteFileArgs.encoding. No path type selectsPathSeparators::PosixorPathSeparators::Windows. Nothing constructs the JSON5 parser's out-of-memory errors.Fix
STATIC_ASSERT_ISO_SUBSPACE_SHARABLEinside the template, where nothing evaluated it. It now follows the class, so the check runs.bun bd,bun run rust:check-all(12 targets),cargo clippy, unchanged generator output, 20 test files (Notes).Background
subspaceFortellsJSC::allocateCell<T>which GC subspace holds aT. A class that allocates withBun::allocatePlainObjectCellnever asks.Downsides
--compile --aothello world maps 8,192 bytes less, and 4,096 more with--trim.Behaviour change: none
Notes
Removed
src/jsc/bindings(67 files, 443 lines): thesubspaceFormember template of 72 classes. Each class allocates withBun::allocatePlainObjectCell(vm, sizeof(T)), and noallocateCell<T>ortryAllocateCell<T>of these classes exists in the tree or in the generated code. The-O0debug binary of main 6f58f6b, linked without--gc-sections, had noT::subspaceFor<...>symbol for them, and had one for each class that still callsallocateCell.JSNonFinalObjecthas no such template in the pinned WebKit, so a laterallocateCell<T>of one of these classes does not compile until the template is back.Paddingparameter ofCryptoAlgorithmAES_CBC::platformEncryptandplatformDecrypt(ignored, both callers pass three arguments),MessagePortPipe::operator==,StreamQueue::first() const, and the emptyanalyzeEntryoverload forByteQueueEntry.src/sys,src/bundler,build_command.rs(14 lines):WriteFileEncodingandWriteFileArgs.encoding. Four writers, no reader.src/paths/Path.rs(38 lines):PathSeparators::{Posix, Windows}. EverySEP_OPTin the workspace isANYorAUTO, and no constant exists for the other two. Thesep_dispatch!macro had one arm left, so its four uses callpath::<fn>::<path::platform::Auto>directly.src/parsers/json5.rs,JSON5Object.rs(16 lines):ParseError::OutOfMemory,Error::Oom,AddToLogError::OutOfMemory,ExternalError::OutOfMemory. The parser has no fallible allocation, so nothing constructs the first one, and each of the others comes only from the one before.src/semver/lib.rs,Scripts.rs(16 lines):impl StringBuilder for semver_string::Builder. Every generic caller gets the lockfile'sStringBuilder.bindings/filter ingenerate-host-exports.tsthat matches no file, and theif (!tree)block incppbind.ts(Parser.parseof@lezer/commonalways returns a tree).Verification (this branch on main 3bf1a06)
bun bd(debug, ASAN) builds.bun run rust:check-all: 12 of 12 targets pass.cargo clippy --no-depsfor the seven changed crates: no finding.rustfmt,clang-formatandprettierreport no change.cpp.rsandgenerated_host_exports.rsare identical.streams.test.js727 pass,headers.test.ts142,fetch_headers.test.js10,web-crypto.test.ts178,json5.test.ts325,message-channel.test.ts17,message-event.test.ts10,string-decoder.test.js143,mock-fn.test.js848,fs-stats-constructor.test.ts7,sqlite.test.js671,node-sqlite.test.ts154,vm.test.ts372,module-graph-callbacks.test.ts1,bun-build-api.test.ts91,path/basename.test.js4,path/dirname.test.js3,path/join.test.js1,path/resolve.test.js6,process.test.js197.process.test.js"should be the node version on the host that we expect" compares with thenodebinary of the machine, v26.3.0 here where the test expects v26.12.1. The release build of main fails it the same way.Size (release builds of CI: main 3bf1a06 in build 137162, this branch in build 137225)
test/bundler/compile-aot-size.test.tson Ubuntu 25.04 x64 prints the numbers. Abun build --compile --aothello world maps 68,134,014 bytes on main and 68,125,822 with this branch (8,192 less). With--trimit maps 25,961,598 and 25,965,694 (4,096 more, one page). "The compiler takes" 23,663,481 and 23,663,546 bytes (65 more).--trimits bound is 25,880,000 bytes.bun-linux-x64.zipis 8,956 bytes smaller,bun-linux-aarch64.zip5,532 andbun-windows-aarch64.zip6,883. The other seven differ by less than 750 bytes. The archives are compressed. The binaries inside were not compared withsizeorbloaty: the machine for this work cannot reach the artifact store.Open pull requests (compared on 2026-11-03 with the heads of the 6,021 other open pull requests)
WriteFileEncodinglines, so it removes six of the same lines. It conflicts with main already. No other head removes a line that this pull request removes.git merge-tree) of each of the 164 with this branch is clean.Left out after the rebase on 2026-11-03
bun_brotli::DecoderOptions(one value): Replace the brotli C library with safe Rust #47073 removes it.NODE_BUILTIN_PATTERNS_RAWandBUN_NODE_BUILTIN_PATTERNS_COMPAT(no reader): resolver: match exactexternalnames before any lookup #47455 adds lines next to the re-export inoptions.rs, so a change there gives it a conflict.src/libarchive,generate-jssink.tsandgenerate-js2native.ts: main deleted or rewrote those files (Replace libarchive with a tar reader and writer in safe Rust #46638, bindgen: move the stream sinks to a schema and deletegenerate-jssink.ts#46641, bindgen: move every Rust function that takes aCallFrameto schemas and delete#[host_fn],$rustand$newRustFunction#46540).Same template, left alone
JSStatFSPrototypeandJSBigIntStatFSPrototype(node:fs: build statfs results with one reachable StatFs class #42554 removes the lines),ImportMetaObjectPrototype(Make import.meta members discoverable as own properties #32248 removes the lines),TTYWrapPrototype(tty: native TTY handle, ReadStream extends net.Socket #41495 removes the lines),JSSharedEnvMap(fix(worker): preserve process.env identity when sharing environments #44371 moves the class),ImportMetaEnv(new on main since the scan, bun test: Vitest compatibility #44833).Bun::allocatePlainObjectCell<T>(vm)that holds the assert can check every class in one place. That is a separate change.allocateCell<T>, for exampleRequireFunctionPrototype,NodeVMModulePrototypeand theProcessBindingobjects. Their template is live.Found dead on 2026-10-21, and left alone (not checked again after the rebase)
max_output_sizeof the brotli, zstd and zlib decoders isusize::MAXin every instance. Issue Bun.zstdDecompress lacks decompressed size limit #28162 asks for an output cap forBun.zstdDecompress, which this field can carry, and http: judge the byte after a gzip member once #45715 and zstd: delete ZstdReaderArrayList in favour of StreamingDecoder #37555 edit the same lines. Needs a decision.bun_md::Options::BOOL_FIELD_SETTERSthat the only reader on main skips: feat(mdx): add native compilation, loading, and direct serving #27047 reads them inMdxObject.rs.ResetEventtimeout (bun test: count code that runs in a Worker under --coverage #43283),Channelclose path (threading: publish into a Channel without holding a reference into it past the release #38358),MemfdFlags::Executable(napi: stub link slots for post-compile .node injection #30033).generate-string-map.ts(Remove the unused options of generate-string-map.ts #46162),Parsed.has_choiceinbun_glob(Remove dead code from bun_glob and bun_css #46196).ErrorCode.tsthat no code reaches through the table,ThreadPool.needs_stack_bounds(alwaystrue, its doc asks callers to clear it),bun_alloc::MaxHeapAllocator(a type with no allocation method, Windows AddressSanitizer build target #35480 and riscv64 target: cross-compiles and runs, looking for guidance rather than a merge #42801 use itsMAX_ALIGN_T), 12 overrides inhawk.tomlfor enum variants that no longer exist (they sit next to the lines Remove dead code from libtcc1.c, four C++ preprocessor conditions, JSPropertyIterator, and hawk.toml #44126 removes), and the prototype classes themselves: most can be built byBun::Bindgen::createClassPrototype, which is visible throughbun:jscheap statistics and is a behaviour change.oom_from_alloc!stamps insrc/parsers/yaml.rs.Self-review (of the larger diff before the rebase)
max_output_sizeandBOOL_FIELD_SETTERSremovals are out.ModuleGraph.cpp,NodeVMScript.cpp,JSCommonJSModule.cppandJSEnvironmentVariableMap.hare in. The lists above were derived again from the final head.subspaceForpart alone is 67 files.