From 06d24c5627790f0e780a99f7e91ca1e37648f5d7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:02:40 +0000 Subject: [PATCH] js_parser: mangle switch case bodies after every case is visited The single-use let/const inliner at the tail of visit_stmts trusts use_count_estimate. A switch case body is one slice of the switch block scope, so a declaration in one case can still be used by a later case that has not been visited yet. The inliner saw a count of 1 and deleted the declaration, which left the later use dangling. Move the minify-syntax tail of visit_stmts into mangle_stmts. visit_stmts skips it for StmtsKind::SwitchStmt and s_switch runs it on each case body once all cases are visited. Case bodies keep the same inlining and statement merging as any other block. Bump the runtime transpiler cache version so cached output from the old inliner is not reused. --- src/js_parser/visit/mod.rs | 22 +++++++- src/js_parser/visit/visit_stmt.rs | 11 ++++ src/jsc/RuntimeTranspilerCache.rs | 4 +- test/bundler/bundler_minify.test.ts | 82 +++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 3 deletions(-) diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index 53786e4db37d..67929fd41281 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -1571,10 +1571,29 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } - if !p.options.features.minify_syntax || !p.options.features.dead_code_elimination { + // A case body is one slice of the switch block scope, so a `let` or + // `const` declared here can still be used by a later case that has not + // been visited yet. `s_switch` mangles every case body once all of them + // have been visited and the use counts are final. + if kind == StmtsKind::SwitchStmt { return Ok(()); } + if p.options.features.minify_syntax && p.options.features.dead_code_elimination { + p.mangle_stmts(stmts); + } + Ok(()) + } + + /// The minify-syntax pass over one visited statement list: drops fully + /// inlined constants, inlines single-use `let`/`const` declarations into the + /// statement that follows them, and merges adjacent statements. + /// + /// Every use of a `let`/`const` declared in `stmts` must have been visited + /// before this runs, since it trusts `use_count_estimate`. + fn mangle_stmts(&mut self, stmts: &mut ListManaged<'a, Stmt>) { + let p = self; + // SAFETY: current_scope is a valid arena ptr for the parse. if p.current_scope().parent.is_some() && !p.current_scope().contains_direct_eval { // Remove inlined constants now that we know whether any of these statements @@ -1934,7 +1953,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // stmts.deinit(); — Drop handles freeing the old buffer (BumpVec is arena-backed). *stmts = output; - Ok(()) } } diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index 2b3cd18271bc..d821a0f990a3 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -2107,6 +2107,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } p.fn_or_arrow_data_visit.is_inside_switch = old_is_inside_switch; + // All case bodies share this block scope, so a `let`/`const` + // declared in one case can be used by a later case. Only now, with + // every case visited, are the use counts final. + if p.options.features.minify_syntax && p.options.features.dead_code_elimination { + for i in 0..cases.len() { + let mut _stmts = stmts_to_list(p.arena, cases[i].body); + p.mangle_stmts(&mut _stmts); + cases[i].body = list_to_stmts(_stmts); + } + } + for i in 0..cases.len() { if p.should_lower_using_declarations(cases[i].body.slice()) { lowered_using = true; diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 597bcd6952aa..10640af6251d 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -55,7 +55,9 @@ bun_core::declare_scope!(cache, visible); /// offsets picked by a header byte) plus a body of tagged records with /// u8/u16/u32 ids and implied slots dropped, instead of fixed u32 arrays. /// Version 27: ModuleInfo string table holds Latin-1 / UTF-16 bodies, not WTF-8. -const EXPECTED_VERSION: u32 = 27; +/// Version 28: The single-use `let`/`const` inliner no longer deletes a declaration +/// in one switch case clause that a later clause still uses. +const EXPECTED_VERSION: u32 = 28; /// Source files smaller than this are not written to / read from the on-disk /// transpiler cache. Originally 50 KiB, which excluded almost every file in a diff --git a/test/bundler/bundler_minify.test.ts b/test/bundler/bundler_minify.test.ts index 11724653c61f..1d2280b2a92c 100644 --- a/test/bundler/bundler_minify.test.ts +++ b/test/bundler/bundler_minify.test.ts @@ -1614,6 +1614,50 @@ describe("bundler", () => { expect(code.match(/let keep = /g)).toHaveLength(10); }, }); + + // A `let`/`const` declared in one case clause is scoped to the whole switch + // block, so a later clause can still read or assign it. The single-use + // inliner used to run per clause and deleted `tag`, `s` and `n` after it + // had only seen the uses in the declaring clause. `only` has its single use + // in the same clause and must still be inlined. + itBundled("minify/SwitchCaseDeclUsedInLaterCase", { + files: { + "/entry.js": /* js */ ` + function capture(v) { return v; } + function fallthrough(k) { + switch (k) { + case 1: const tag = { id: 1 }; capture(tag); + case 2: return typeof tag; + } + } + function defaultClause() { + switch (1) { + case 1: let s = "s1"; capture(s.length); + default: { return s + "!"; } + } + } + function reassigned(k) { + switch (k) { + case 1: let n = k; capture(n); + case 2: n = 5; return n; + } + } + function sameClause(v) { + switch (v) { + case 1: const only = v; return capture(only + 1); + } + } + console.log(JSON.stringify([fallthrough(1), defaultClause(), reassigned(1), sameClause(1)])); + `, + }, + capture: ["v", "tag", "s.length", "n", "v + 1"], + minifySyntax: true, + minifyIdentifiers: false, + target: "bun", + run: { + stdout: '["object","s1!",5,2]', + }, + }); }); // The runtime transpiler (`bun run`/`bun test`) implicitly enables @@ -1647,3 +1691,41 @@ test("runtime transpiler does not collapse single-return arrow bodies", async () expect(stderr).toBe(""); expect(exitCode).toBe(0); }); + +// The runtime transpiler runs the same single-use inliner. A `const` declared +// in one case clause and read after a fall-through into the next clause must +// keep its declaration. +test("runtime transpiler keeps a switch case declaration that a later case reads", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + function f(k) { + switch (k) { + case 1: const tag = { id: 1 }; use(tag); + case 2: return typeof tag; + } + } + function g() { + switch (1) { + case 1: let s = "s1"; use(s.length); + default: { return s + "!"; } + } + } + function use(v) { return v; } + let r; + try { r = g(); } catch (e) { r = e.constructor.name; } + console.log(JSON.stringify([f(1), r])); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout).toBe('["object","s1!"]\n'); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); +});