Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bundler/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use bun_ast::RuntimeTranspilerCache;

/// Bump when the cache wire format or parser output changes. Mirrors
/// `expected_version` in src/jsc/RuntimeTranspilerCache.zig.
pub const RUNTIME_TRANSPILER_CACHE_VERSION: u32 = 20;
pub const RUNTIME_TRANSPILER_CACHE_VERSION: u32 = 21;

/// Mirrors the Zig `pub var is_disabled` mutable global — written by T6
/// (src/runtime/cli/Arguments.zig:1603, src/jsc/VirtualMachine.zig:1383) and
Expand Down
16 changes: 15 additions & 1 deletion src/js_parser/visit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1606,8 +1606,22 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
// Ignore declarations if the scope is shadowed by a direct "eval" call.
// The eval'd code may indirectly reference this symbol and the actual
// use count may be greater than 1.
//
// Ignore declarations inside a switch case body: every case in a switch
// shares one lexical scope but we visit one case at a time, so
// `use_count_estimate` for a decl in case 0 has not yet seen references
// from later cases and may spuriously read as 1 — inlining then would
// delete the decl out from under those later references (issue #30932).
// `is_inside_switch` resets at function boundaries (it lives on
// `FnOrArrowDataVisit`), so nested functions inside a case body still
// get the optimization. Unlike `StmtsKind::SwitchStmt`, using this
// flag keeps the using-lowering path at L1488 independent from the
// case-body guard here.
// SAFETY: current_scope is a valid arena ptr for the parse.
if p.current_scope != p.module_scope && !p.current_scope().contains_direct_eval {
if !p.fn_or_arrow_data_visit.is_inside_switch
&& p.current_scope != p.module_scope
&& !p.current_scope().contains_direct_eval
{
Comment thread
robobun marked this conversation as resolved.
// Keep inlining variables until a failure or until there are none left.
// That handles cases like this:
//
Expand Down
11 changes: 11 additions & 0 deletions src/js_parser/visit/visit_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2153,6 +2153,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
{
p.push_scope_for_visit_pass(js_ast::scope::Kind::Block, data.body_loc)
.expect("unreachable");
// Every case body shares one lexical scope but is entered conditionally,
// so a `const` declared in one case does not dominate references to that
// name in other cases. Disable the const-local-prefix inlining for the
// entire switch body — otherwise `case 'a': console.log(X)` could be
// rewritten to use the value from `case '*': const X = 2;` above it and
// silently bypass the TDZ error the spec requires (issue #30932).
p.cur_scope().is_after_const_local_prefix = true;
Comment thread
robobun marked this conversation as resolved.
let old_is_inside_switch = p.fn_or_arrow_data_visit.is_inside_switch;
p.fn_or_arrow_data_visit.is_inside_switch = true;
let cases = data.cases.slice_mut();
Expand All @@ -2164,6 +2171,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
// p.warnAboutTypeofAndString(s.Test, *c.Value)
}
let mut _stmts = stmts_to_list(p.arena, cases[i].body);
// Passes `StmtsKind::None` (not `SwitchStmt`) so the using-lowering
// path in `visit_stmts` stays enabled for each case body. The
// single-use-substitution pass is instead gated on
// `fn_or_arrow_data_visit.is_inside_switch`, which is set above.
p.visit_stmts(&mut _stmts, StmtsKind::None)
.expect("unreachable");
cases[i].body = list_to_stmts(_stmts);
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ bun_core::declare_scope!(cache, visible);
/// Version 18: Include ESM record (module info) with an ES Module, see #15758
/// Version 19: Sourcemap blob is InternalSourceMap (varint stream + sync points), not VLQ.
/// Version 20: InternalSourceMap stream is bit-packed windows.
const EXPECTED_VERSION: u32 = 20;
/// Version 21: Parser no longer inlines const declarations across switch case bodies (#30932).
const EXPECTED_VERSION: u32 = 21;
Comment thread
robobun marked this conversation as resolved.

/// 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
Expand Down
101 changes: 101 additions & 0 deletions test/regression/issue/30932.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import { describe, expect, test } from "bun:test";

// https://github.com/oven-sh/bun/issues/30932 (and #18477)
//
// Every case in a switch shares one lexical scope but each case is entered
// conditionally, so a `const` declared in one case does not dominate references
// to the same name in sibling cases. Two inlining passes in the parser used to
// ignore this and produced code that silently bypassed the spec-required TDZ
// `ReferenceError`:
//
// 1. const-local-prefix inlining — `const X = literal` in the first case was
// recorded and every later reference to `X` in the switch body was
// rewritten to `literal`.
// 2. single-use substitution — `visit_stmts` ran once per case body but
// `use_count_estimate` is global, so when visiting `case "*"` the
// counter had only seen `case "*"`s references. `const X = expr` +
// `return X` read as single-use and the decl was deleted, leaving
// sibling-case references dangling.
Comment thread
robobun marked this conversation as resolved.
describe("switch-case const does not leak across cases", () => {
test("literal-initialized const is not inlined into sibling case", () => {
function test(action: string) {
switch (action) {
case "*":
const CONSTANT = 2;
return "matched " + CONSTANT;
case "a":
return "a=" + CONSTANT;
}
}
expect(() => test("a")).toThrow(ReferenceError);
expect(test("*")).toBe("matched 2");
});

test("non-foldable const is not substituted out of sibling case", () => {
function test(action: string) {
switch (action) {
case "*":
const X = Math.random();
return "* " + X;
case "a":
return "a " + X;
}
}
// Post-fix JSC raises "Cannot access 'X' before initialization" (TDZ).
// Pre-fix, Bun deleted the `const X` decl entirely as a single-use
// substitution while visiting case "*", turning the case "a" reference
// into an unbound "X is not defined" — also a ReferenceError, but with
// different semantics — so match on name + message shape.
expect(() => test("a")).toThrow(
expect.objectContaining({
name: "ReferenceError",
message: expect.stringContaining("before initialization"),
}),
);
});

test("outer const is shadowed by inner const, not leaked through", () => {
const CONSTANT = 1;
function test(action: string) {
switch (action) {
case "*":
const CONSTANT = 2;
return "* " + CONSTANT;
case "a":
return "a " + CONSTANT;
}
return "outer=" + CONSTANT;
}
// The inner `const CONSTANT` hoists over the entire switch body and
// shadows the outer one, so touching it from `case "a"` before the
// `case "*"` declaration has evaluated is TDZ.
expect(() => test("a")).toThrow(ReferenceError);
expect(test("*")).toBe("* 2");
// Outside the switch body the outer `CONSTANT` is still visible.
expect(test("other")).toBe("outer=1");
});

test("declaration + use in the same case still works", () => {
function test() {
switch ("a") {
case "a": {
const X = 42;
return X;
}
}
}
expect(test()).toBe(42);
});

test("fall-through from declaring case still initializes binding", () => {
function test() {
switch ("a") {
case "a":
const X = 100;
case "b":
return "X=" + X;
}
}
expect(test()).toBe("X=100");
});
});
Loading