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); +});