parser: drop dead-branch var with zero identifiers instead of emitting var;

[robobun]

Fixes #31002.

Repro

$ bun -e "if(0)var[]"
panic: internal error: entered unreachable code (src/js_printer/lib.rs:2308:17)
Crashed while printing …/[eval]

if (0) var [] / if (0) var {} — a destructuring var with zero
bindings inside a statically-dead if body — crashed the printer in
print_decls where unreachable!() guards against empty decl lists
("var;" is invalid syntax). if(0) is load-bearing in the reproducer:
non-dead branches ship the statement through to JSC, which rejects it
with a regular SyntaxError; the dead-branch path never reaches JSC.

Cause

scan_side_effects.rs::should_keep_stmt_in_dead_control_flow (the DCE
helper that decides what to retain in if (false) …) handles var by
hoisting every identifier out of its destructuring pattern so scope
semantics are preserved:

if (0) var [a, b] = expr();   // becomes: var a, b;  (value dropped, names hoisted)

For var [] the destructuring pattern binds no identifiers, so the
hoisted vector is empty. The code then installed the empty vec as the
new decl list and returned true, telling the visitor to keep an
S::Local carrying zero decls. The next pass handed that to the
printer and tripped the invariant.

Fix

src/js_parser/scan/scan_side_effects.rs: when the collected
identifier list is empty, return false and drop the statement
entirely — nothing to hoist and no valid var output exists. This is
symmetric with the existing "omit entirely" branches for
S::Empty/S::Expr/S::Throw/etc.

 let mut decls: Vec<G::Decl> = Vec::with_capacity(local.decls.len_u32() as usize);
 for i in 0..(local.decls.len_u32() as usize) {
     let binding = local.decls.at(i).binding;
     Self::find_identifiers(binding, &mut decls);
 }

+if decls.is_empty() {
+    return false;
+}
+
 local.decls = G::DeclList::move_from_list(decls);
 true

Verification

$ bun bd -e 'if(0)var[]'
$ echo $?
0
$ bun bd -e 'if(0) var [a, b] = []; console.log(typeof a, typeof b);'
undefined undefined

New test at test/regression/issue/31002.test.ts runs each crash
repro in a subprocess and asserts exitCode === 0 with no output,
plus a regression guard that destructuring vars with real
identifiers still hoist them as undefined.

Alive-path behavior is unchanged: var [] / var {} at top level or
in truthy branches still reach JSC, which rejects them with the
expected "Expected an initializer in destructuring variable
declaration" SyntaxError.

Note on debug-build compile error

The current debug build on main fails in src/jsc/bindings/wtf-bindings.cpp
with use of undeclared identifier 'assert' (unrelated; tracked
upstream in #30988, fix already open in #30992). That is not in this
diff; rebasing after #30992 lands will unblock the debug lane.

[robobun]

^{Updated 12:03 PM PT - May 18th, 2026}

✅ @robobun, your commit 1eeb7004f94f7d1275ce9e5bd812efb571bc80ad passed in Build #55859! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 31003

That installs a local version of the PR into your bun-31003 executable, so you can run:

bun-31003 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1958b49d-8a77-42bd-8417-7496cb6bc1ad

📥 Commits

Reviewing files that changed from the base of the PR and between 77a9ed6 and 1eeb700.

📒 Files selected for processing (1)

• src/jsc/bindings/wtf-bindings.cpp

---

Walkthrough

This PR fixes a crash in Bun's parser when encountering invalid destructuring var declarations inside dead-code branches like if(0) var[]. The parser's dead-control-flow handler now returns false for patterns that bind no identifiers, preventing emission of invalid syntax. Two regression tests verify the fix prevents crashes and preserves valid destructuring patterns with real bindings. A small TTY binding assertion is adjusted from assert to ASSERT.

Changes

Dead-code destructuring handling

Layer / File(s)	Summary
Parser guard for empty destructuring in dead flow src/js_parser/scan/scan_side_effects.rs	should_keep_stmt_in_dead_control_flow returns false when a var destructuring pattern's identifier list is empty, preventing emission of invalid empty decl list syntax.
Regression test suite for issue #31002 test/regression/issue/31002.test.ts	Test suite validates that empty var[] and var{} patterns in dead branches produce no output without crashing, and that patterns with real bindings are preserved with correct hoisting.
WTF binding TTY assertion change src/jsc/bindings/wtf-bindings.cpp	Replaces assert(tio != NULL); with ASSERT(tio != NULL); in uv__tty_make_raw.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: preventing emission of invalid empty var declarations in dead code branches by dropping them instead.
Description check	✅ Passed	The description comprehensively explains the issue, cause, fix, and verification with clear code examples and test coverage details.
Linked Issues check	✅ Passed	The changes fully address issue #31002 by preventing the printer crash when dead-branch DCE encounters destructuring var patterns with zero identifiers.
Out of Scope Changes check	✅ Passed	All changes directly address the linked issue: the parser fix handles empty destructuring patterns, the test validates the fix, and the wtf-bindings change is a related debug-build fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

LGTM — minimal, well-targeted crash fix with a solid regression test.

Extended reasoning...

Overview

This PR touches three files. The core change is a 4-line early-return in src/js_parser/scan/scan_side_effects.rs::should_keep_stmt_in_dead_control_flow: when a dead-branch var destructuring pattern yields zero hoisted identifiers (e.g. if(0) var []), the statement is now dropped instead of being kept with an empty decl list — which previously tripped an unreachable!() in the printer. A new regression test at test/regression/issue/31002.test.ts exercises both the crash repros and a guard that non-empty destructuring still hoists correctly. A second commit swaps assert() → ASSERT() in wtf-bindings.cpp to unblock the debug build (matching the existing ASSERT usage elsewhere in that same file).

Security risks

None. The parser change only affects what gets emitted for statically-dead var declarations with empty destructuring patterns — there is no user-controlled data flow, allocation, or boundary crossing involved. The C++ change is a debug-only assertion macro swap with no runtime semantics change in release builds.

Level of scrutiny

Low. The Rust change is a textbook empty-collection guard placed exactly where the invariant would otherwise be violated, symmetric with the surrounding "omit entirely" branches for SEmpty/SExpr/etc. The fast-path above it (single BIdentifier decl) is unaffected. The assert → ASSERT swap is a one-token change to the project's standard WTF assertion macro, already used at line ~256 of the same file.

Other factors

The regression test is thorough: it runs each crash repro in a subprocess (so a regression panics the child, not the runner), covers both var [] and var {} forms plus a nested-scope variant, and separately verifies that if(0) var [a,b] = [] still hoists a/b as undefined. No CODEOWNERS paths are touched. The robobun CI failures reference the first commit (pre-ASSERT fix) and are the musl debug-build break the second commit addresses, plus an unrelated Windows-aarch64 agent provisioning failure. The wtf-bindings change is a small ride-along but is acknowledged in the PR description and overlaps with #30992; it's trivially safe.