Skip to content

fix: expand spread args to compiled Array.prototype.concat (#952)#962

Merged
nickna merged 1 commit into
mainfrom
wrk/issue-952-concat-spread
Jun 26, 2026
Merged

fix: expand spread args to compiled Array.prototype.concat (#952)#962
nickna merged 1 commit into
mainfrom
wrk/issue-952-concat-spread

Conversation

@nickna

@nickna nickna commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Closes #952.

Problem

In compiled mode, a spread argument to Array.prototype.concat was not flattened:

console.log(JSON.stringify([0].concat(...[[1, 2]])));
// compiled: [0,[1,2]]   node: [0,1,2]

The array-method emitters built concat's argument array by emitting each AST arg via EmitExpression, but a spread ...arr is an Expr.Spread whose emit yields the inner array object — so the spread element reached the runtime ArrayConcat(list, object?[] items) as a single nested item, and concat's per-argument one-level flatten left it nested. [0].concat(b) (no spread) was always correct; it failed for boxed (non-numeric) arrays too.

The interpreter already expanded spreads correctly (EvaluateCallGetIterableElements), so this was compiled-only, exactly as the issue stated.

Same family as the Math.max(...arr)NaN gap (#951, PR #957), which introduced the helper this fix reuses.

Changes

Route concat's args through the existing EmitArgsArrayWithSpread helper (on IEmitterContext, added in #957) at every compiled dispatch path that emits Array.prototype.concat, so the spread is flattened before concat applies its per-argument flatten. The helper is identical to the old inline loop when no spread is present, so non-spread concat IL is unchanged → no regression.

  • Compilation/Emitters/ArrayEmitter.cs — primary arr.concat(...) path (statically-typed array). Stash the receiver list, build the spread-expanded args array await-safely (the helper evaluates each arg into a temp with a clear stack, so the spread may contain await), then reload list under items. Excluded from the Compiled async-arrow write-capture emits invalid IL (PathStackDepth), even without shadowing #850 plain-arg pre-spill (!concatSpread) to avoid double-evaluating the args.
  • Compilation/ILEmitter.Calls.AmbiguousDispatch.cs and Compilation/ExpressionEmitterBase.CallHelpers.cs — the two runtime-typed (string-or-list / any) dispatch paths for x.concat(...).
  • Compilation/ILEmitter.Calls.cs — the shared argsArray case (Array.prototype.X.call), which also closes the same spread gap for reduce/slice/toSpliced/with via .call.

Tests

SharpTS.Tests/SharedTests/ArrayConcatSpreadTests.cs (new) — the issue repro plus flat-array / multi-array / mixed spread+plain / boxed-string / variable-source / empty / async spreads, run in both interpreter (the Node-matching oracle) and compiled modes.

Verification

  • New tests: 16/16 green (8 cases × 2 modes); adjacent array/spread tests 156/156.
  • Full suite: 14383 passed, 0 failed.
  • The exact repro produces Node-identical output in both modes via the CLI.

Compiled `[0].concat(...[[1, 2]])` returned `[0,[1,2]]` instead of
`[0,1,2]`. The array-method emitters built concat's argument array by
emitting each AST arg via EmitExpression, but a spread `...arr` is an
Expr.Spread whose emit yields the inner array object — so the spread
element reached the runtime ArrayConcat as a single nested item, and
concat's per-argument one-level flatten left it nested.

Route concat's args through the existing EmitArgsArrayWithSpread helper
(added for Math.max spread, #951) at every compiled dispatch path that
emits Array.prototype.concat, so the spread is flattened before concat
applies its per-argument flatten. The helper is identical to the old
inline loop when no spread is present, so non-spread concat IL is
unchanged (no regression):

- ArrayEmitter (statically-typed `arr.concat(...)`, the repro path):
  stash the receiver list, build the spread-expanded args array
  await-safely (the helper evaluates into temps with a clear stack, so
  the spread may contain await), reload list under items. Excluded from
  the #850 plain-arg pre-spill to avoid double-evaluating the args.
- The two runtime-typed (string-or-list / any) dispatch paths in
  ILEmitter.Calls.AmbiguousDispatch and ExpressionEmitterBase.CallHelpers.
- The shared argsArray case in ILEmitter.Calls (Array.prototype.X.call),
  which also closes the same spread gap for reduce/slice/toSpliced/with.

The interpreter already expanded spreads correctly (EvaluateCall via
GetIterableElements), so this was compiled-only.

Tests: ArrayConcatSpreadTests covers the repro plus flat/multi-array/
mixed/boxed-string/variable/empty/async spreads across interpreter and
compiled modes. Full suite 14383 green.
@nickna nickna merged commit a651a12 into main Jun 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compiled: Array.prototype.concat with a spread arg doesn't flatten (concat(...[arr]))

1 participant