Parser: destructuring catch parameters allow a same-named var

Related: #930 (missed lexical/var early errors), #929 (where this was found)

Invalid programs accepted

Found while gating #929 — it surfaced in that fix's 220-program corpus as a case wrong both before and after, and turned out to have nothing to do with static blocks. Every row was executed against node and zig-js, with each program wrapped in a never-called function so the verdict reflects parsing alone.

And the neighbours zig-js already gets right, which bound the fix:

The rule, as observed

A var may reuse a catch parameter's name only when the parameter is a plain BindingIdentifier — the Annex B.3.5 relaxation. For a destructuring pattern the ordinary early error applies: the pattern's BoundNames must not occur in the catch block's VarDeclaredNames, including vars hoisting out of nested blocks and for heads.

zig-js appears to apply the plain-identifier relaxation to destructuring parameters too, and also does not look through nested blocks for them.

Worth recording because I got it wrong from memory first: for-of is not an exception here. I expected catch (e) { for (var e of []); } to be an error on the recollection that B.3.5 excludes for-of heads; node accepts it, and zig-js agrees. So any fix should be driven by the executed table above, not by that recollection.

Suggested order

1. Pin all twelve rows as a parser test.
2. Find where catch parameter BoundNames are compared with the block's var names, and restrict the relaxation to the BindingIdentifier form. Note #928 already threads open lexical names through the block walk; a destructuring catch parameter is effectively a lexical binding for its block, so it may fit that machinery rather than a separate scan.

Fixed in 33e5222

A destructuring catch parameter now opens its names in the #928 lexical walk for the duration of its block, so every var that hoists into that block is tested against them once. A plain catch (e) opens nothing, which preserves the Annex B.3.4 relaxation.

Why not in checkCatchClause

That function runs at parse time, and checking vars there means walking the whole catch block for each clause. The parser has no nesting limit, so nested destructuring catches would re-walk one another — the #928 quadratic, reintroduced. The walk tests each var once and brings the right function boundary along with it.

The one design point worth calling out, confirmed against node before implementing: catch ([e]) { try {} catch (e) { var e; } } is an error. The inner plain catch permits its own var e, but that var still hoists into the outer destructuring parameter's block. A shared open-names map gets this right automatically; a check local to each clause would not.

Results

49 programs, each verdict taken from node and each wrapped in a never-called function so the result reflects parsing alone:

The 26 cover every destructuring form ([e], {e}, {a: e}, [e = 1], [...e], nested [[e]] and {a: {b: e}}), vars reaching the block through nested blocks, if, labels, switch, do/while, every for head, nested try/finally, destructuring var declarations, catch inside a function body, and catch inside a class static block.

Both remaining cases were predicted before the build, and both are reach gaps of the walk already tracked in #930 rather than flaws in this rule:

• (function(){ try {} catch ([e]) { var e; } })() — the walk does not enter function bodies nested in expressions (family 1);
• try {} catch ([e]) { with ({}) { var e; } } — the walk has no with arm (family 2).

The first is noted beside the pinned test cases, so a future reader does not take its absence for an oversight.

Verification

• parser destructuring catch parameters reject a same-named var: 16 cases that must be rejected, 9 that must stay valid. 75/75 parser tests pass.
• test262 before vs after, across statements/try, annexB/language, block-scope, both class trees, function-code, eval-code and for-of: 10,932 tests, 0 fail under both. statements/try has early-error tests for duplicate, function and lexical catch conflicts but none for this one, which is why the old parser passed it at 100%.

Found on the way — #932

To check that this fix did not add a quadratic, I timed nested destructuring catches at doubling depths. It did not — but that shape already was quadratic before the fix: x3.9 per doubling, 1.87 s at depth 4000. Isolating it showed the cause is unrelated to vars or to this rule: destructuring catch parameters inside a function body are quadratic, while the same nesting is linear at top level and linear with plain parameters. Filed as #932 with the isolation table.

It also exposed that the #926 and #928 benchmarks measure top-level scripts only, so neither could see an effect confined to function bodies; that caveat is recorded on #932.