Repository navigation
Conversation
- js_printer: print_internal_bun_import has one caller, which passes Some(b"globalThis.Bun"). The three None arms are removed and the parameter is a plain slice. - bun_io: nothing inserts Flags::PollProcess, Flags::Process or Flags::NeedsRearm. The three members and the code that tests them are removed. - bun_install: UpdateRequest::parse_with_error has one caller, which passes fatal = true. The parameter and the two else branches are removed, and a comment that points at a deleted file.
|
Status: ready for review.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — three self-contained dead-code deletions, each verified unreachable from the code rather than the description.
What was reviewed:
bun_io::Flagsremoval: grepped everyPollProcess/Process/NeedsRearmreference in the tree; all remaining hits (windows_event_loop.rs, ParentDeathWatchdog.rs viafile_poll::Flags, posix_event_loop.rs) resolve toposix_event_loop::Flags, not the lib.rs enum, so no cfg-gated target loses a variant it names.register_for_epoll: both callers passPollReadable/PollWritableand the firstmatch flagalreadyunreachable!()s on anything else, so the deleted second match (and its linux/android branch) was dead; nothing in lib.rs ever insertedNeedsRearm, so the EPOLL_CTL_MOD/ADD choice is unchanged in practice.print_internal_bun_import/parse_with_error: single caller each (Some(b"globalThis.Bun"),fatal = true);print_require_or_import_exprkeeps two other callers andLochas no remaining use in UpdateRequest.rs.
Extended reasoning...
The change is a single commit of +19/-104 across src/js_printer/lib.rs, src/io/lib.rs, and src/install/PackageManager/UpdateRequest.rs that only deletes unreachable branches, three enum variants, and one always-constant parameter. It touches no security-sensitive surface: the epoll registration logic keeps the same syscall flags and MOD/ADD selection for every reachable input, and the install error path still emits the identical message for its only caller. Every deletion was confirmed dead by grepping callers and cross-checking which of the two same-named Flags enums each cfg-gated reference binds to, which also covers the Windows and macOS paths without needing a build. No changed file is under CODEOWNERS, the hunt ran dry with no findings, and there are no third-party objections on the timeline.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. WalkthroughThe changes update update-request error reporting, epoll registration flags, and printing of the global Bun import statement. ChangesDependency Format Errors
Epoll Polling
Bun Import Printing
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This dead-code removal is not expected to change behavior, and no actionable merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Both automated reviews found nothing to change. The branch stays at 346d7c9. Check status at this time:
|
This branch has not been deployed
2 participants
Behaviour change: none
Problem
print_internal_bun_importinsrc/js_printer/lib.rshas threeNonearms, and its one caller passesSome(b"globalThis.Bun").dead_codecounts a forwarded argument, a flag test and a match arm as a use.mordanthas no finding for these files.Fix
bun bd,bun run rust:check-all(12 targets), 8 morecargo checkconfigurations, 7 test files (Notes)..textsize estimate (not measured) and two origin pull requests thatgit log -Sdoes not confirm.Background
print_internal_bun_importprints animport ... from "bun"statement as a read ofglobalThis.Bun, when the printer targets Bun.bun_io::Pollis the epoll request of the I/O thread on Linux.FilePoll, the event-loop poll, has its ownFlagsset.UpdateRequest::parsereads the package arguments ofbun add,bun removeandbunx.Downsides
src/,packages/,scripts/or the generated code, on 12 targets.Notes
Removed, one group each
src/js_printer/lib.rsNonearms inprint_internal_bun_import. The parameter is&'static [u8]now.print_global_bun_import_statement, which passesSome(b"globalThis.Bun"). The two names have 4 hits in the tree together, all in this file.git log -Sshows noNonecaller since the Rust port (#30412). The printed bytes are the same.src/io/lib.rsbun_io::Flags::{PollProcess, Process, NeedsRearm}, theProcesspattern, theNeedsRearmtest and removal, and amatchthat mappedPollReadableandPollWritableto themselves.Poll::register_for_epollpassPollReadableorPollWritable. Nothing inserts the three flags, andgit log -Sshows no setter since the Rust port. The firstmatch flagstill stops on any other flag. The flag set has noreprand is not stored. The one cast isflag as u8in a log line, which prints 0 or 1 as before.posix_event_loop::Flagshas members with the same three names. They have users and stay.src/install/PackageManager/UpdateRequest.rsfatalparameter ofparse_with_error, the twoelsebranches, and a comment that namesfrom_js.UpdateRequest::parse, which passestrue. #39420 deletedupdate_request_jsc.rs, which heldfrom_js, the one caller that passedfalse.Left in the same functions
register_for_epoll(one_shot)andbun_io::Flags::OneShot: both callers passtrue, so the same proof holds (about 13 lines). They stay because fix(io): emulate EPOLLONESHOT with DEL + ADD #40473 readsFlags::OneShotin this function.statementparameter ofprint_internal_bun_importhas one value at its one call site. It stays because js_printer: fix require("bun") and other printer literals being captured by same-named locals #35739 passes another value.Verification
All on the final commit, rebased on 2722608.
bun bdpasses.bun run rust:check-all: 12 ok, 0 failed.cargo check --workspaceconfigurations pass: the host with--all-targets, then Linux, Windows and macOS with--cfg=bun_asan --cfg=bun_debug --cfg=socket_fault_injection, the same three in the release profile with--cfg=bun_codegen_embed, andbun_shim_implwith--features shim_standalone.cargo clippy --no-depsforbun_io,bun_js_printerandbun_installreports nothing.cargo fmt --all --check: no change.test/cli/install/bun-add.test.ts(71 pass),test/bundler/transpiler/runtime-transpiler.test.ts(15),test/bundler/transpiler/transpiler.test.js(237),test/js/bun/io/bun-write.test.js(86) andtest/js/bun/spawn/spawn-streaming-stdout.test.ts(1), all with no failure.test/js/bun/spawn/spawn.test.ts: the first run had 4 tests that stopped at the 5 s timeout, while the load average of the machine was above 400. The second run passed (177 and 178 pass, 0 fail). The file does not reach the changed code.test/cli/install/bunx.test.ts: 33 pass, 1 fail. The failure isshould set "npm_config_user_agent" to bun: the test expectsbun/1.4.3-debug(Bun.versionof a debug build) and the user agent saysbun/1.4.3. This change touches neither string. The test was not run on a debug build of main.import ... from "bun"(default, namespace, named, default with named, bare) prints the same values with the debug build and with the released bun. The runtime transpiler prints avardeclaration that readsglobalThis.Bunfor each form.test/internal/source-lints/: 170 pass, 0 fail.cargo checkcovers the Rust code.Self-review
Flags::Closedhunks. The hunks are out.one_shotandFlags::OneShotare dead by the same proof. They are listed above with the reason they stay.from_jscomment inUpdateRequest.rswas stale. It is removed.git log -Sshows no link to the three groups that are left.flag as u8is a cast. The table says so now.connection_strings..textsize estimate. Rejected: no release build was made here, so there is no number.Open pull requests
git merge-tree) with main and with this branch, at the heads of 2026-09-30.import ... from "bun"above the module body at runtime #39206, fix(io): emulate EPOLLONESHOT with DEL + ADD #40473, bundler: apply the CommonJS unwrap list only to CommonJS files #42500. No pull request that merges cleanly today gains a conflict.Some(..)value toprint_internal_bun_import. bundler: fix re-exports of the "bun" builtin with --target=bun #37829 moves the threematchblocks into a helper and keeps theOption. Deterministic dependency resolution: an ordered walk over a live tree #36476 changes another argument ofparse_with_errorand still passestrue. Replace boolean flag parameters with two-variant enums #37180 turnsfatalinto an enum. io: hand ReadFile/WriteFile to the io thread through the owner's pointer, not &mut self.io_request #37787 renames the receiver in the lines that testNeedsRearm. None of the five adds a second value, aNonecaller, or a setter of a removed flag.Applied, compiled on 12 targets, then taken out
The first version of this change had 26 files and removed 708 lines.
bun run rust:check-allpassed for it, and so did 13 test files. 11 groups are out because an open pull request removes, rewrites or uses the same code:migrate_yarn_lockfile: the localspackage_dependents,usage_count,scoped_count,created_packages, andVersionInfo.yarn_idx(src/install/yarn.rs)tag_print_help,pm_print_help,Tag::UpdateInteractiveCommand(src/runtime/cli/mod.rs)pm_print_helpValkeyClient.connection_stringsand 8 parameters nothing reads (src/runtime/valkey_jsc)connection_strings, #40139 rewrites the three filesNodeFS::watch_fileand theResultofStatWatcher::init(src/runtime/node)RmState::ParseOpts.wait_write_err,RmState::Err,DirTask.deleting_after_waiting_for_children(src/runtime/shell/builtin/rm.rs)Linker::generate_import_path(use_hashed_name)andintern(src/bundler/linker.rs)Parser.jsobjs,JSValueRaw,Lexer.delimit_quote(src/shell_parser)Resultofchanged_files_filter::filter,Scanner.exclusion_names(src/runtime/cli/test)ArenainGlob::__scanand 3 parameters (src/runtime/api/glob.rs)ByteStream.pending_buffer,FileReader.pending_view(src/runtime/webcore)Flags::Closedin the POSIXFilePoll(src/io/posix_event_loop.rs). Only the WindowsFilePollinserts the flag.Two more were applied and taken out in review, not for an open pull request.
src/runtime/api/bun/Terminal.rs: thesignalargument of the exit callback is alwaysnull, andbun.d.tscalls it reserved.src/runtime/shell/builtin/which.rs: the removal of two fallbacks needs anexpectin their place.How the candidates were found
mordantlists 170unused_pubitems: each is in an open pull request, or is an entry of an OS constant table that earlier sweeps kept.src/andscripts/were read for 8 classes: a parameter with one value at each call site, a variant with no constructor, a field or local with no reader, a branch behind a constant, a dead chain behind one of those, a trait method with no caller, a duplicate item, and the same in TypeScript. A second reading tried to refute each finding. 366 findings passed, 9 did not.syngave a second list (variants with no constructor by name, parameters with one literal value, fields with no read). Most of its hits were false: a derive, a string map or FFI builds the variant.Findings that are a bug, not residue
Each has a reproduction with the released bun, and an open pull request already covers each one.
[install] dryRun = trueinbunfig.tomlis parsed and not read.bun installinstalls. install: honor[install] dryRunfrom bunfig.toml #34979 fixes it.(a?.b)`x`. The test for an optional chain reads the template node, not its tag, so it is always false. The output does not parse. js_printer: keep the parentheses a new callee or template tag needs #41638 fixes it.function f(a, a) {}in a CommonJS file fails to parse.is_unique_formal_parametersistrueat both constructors. parser: report strict-mode early errors and accept legacy sloppy syntax #40827 fixes it.Bun.markdownacceptslatexMath,underline,hardSoftBreaksandcollapseWhitespaceand ignores them (Bun.markdown: hardSoftBreaks and collapseWhitespace options are documented but do nothing #39491). Bun.markdown: implement the latexMath and underline options #39493 and Bun.markdown: implement the hardSoftBreaks and collapseWhitespace options #39495 implement them and need the renderer arms that have no producer today.Found and left in place
Read twice, not compiled, and not verified further. One finding of this kind did not hold when it was run:
fs.promises.readdirwithrecursivedoes honourencoding: "buffer". The line counts are estimates of the readers. The 44 findings of 60 lines or more:src/react_compiler/validation/validate_no_set_state_in_effects.rs(+10)src/runtime/shell/interpreter.rs(+9)src/js/node/repl.js(+4)src/crash_handler/lib.rs(+3)src/react_compiler/program.rs(+8)src/bundler/options.rs(+7)src/resolver/lib.rs(+6)src/runtime/test_runner/pretty_format.rs(+2)src/runtime/bake/client/JavaScriptSyntaxHighlighter.tssrc/sql_jsc/mysql/JSMySQLQuery.rs(+13)src/runtime/api/bun/subprocess/Readable.rs(+2)src/react_compiler/codegen.rs(+5)src/css/css_parser.rs(+6)src/css/css_parser.rs(+1)src/react_compiler/hir/mod.rs(+10)src/css/rules/mod.rs(+7)src/runtime/api/bun/spawn/stdio.rs(+2)src/js_parser/visit/mod.rs(+5)src/runtime/test_runner/pretty_format.rs(+2)src/ast/lib.rssrc/bun_alloc/lib.rs(+2)src/react_compiler/validation/validate_use_memo.rs(+4)src/css/generics.rs(+4)src/react_compiler/hir/dominator.rs(+10)src/bundler/linker_context/writeOutputFilesToDisk.rs(+2)src/runtime/bake/DevServer.rs(+6)src/css/selectors/parser.rs(+1)src/css/selectors/parser.rs(+1)src/standalone_graph/StandaloneModuleGraph.rs(+1)src/runtime/webcore/Blob.rs(+1)src/bundler/options.rs(+5)src/bundler/transpiler.rs(+4)src/runtime/shell/subproc.rs(+1)src/react_compiler/options.rs(+5)src/bundler/transpiler.rs(+2)src/install/lockfile.rs(+3)src/jsc/VirtualMachine.rs(+2)src/js/bun/sql.ts(+2)src/md/ansi_renderer.rs(+5)src/bundler/linker_context/postProcessCSSChunk.rs(+2)src/css/printer.rs(+4)src/s3_signing/credentials.rs(+5)src/runtime/webcore/Blob.rs(+1)src/runtime/node/path.rsAlso left in place, by a decision in this run:
src/react_compiler: 12 findings, about 1,800 lines (for exampleInstructionValue::JSXTextandTypeCastExpression, which nothing constructs, andTerminal::Unsupported).scripts/sync-react-compiler.shkeepshir/,validation/,inference/and 6 more directories close to the upstream crates. A removal of variants and match arms works against that. The removal of the three variants compiled on 12 targets.configure_allocatorinsrc/bun_core/Global.rsis an empty function with 5 callers. Its comment says that it stays on purpose.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