Skip to content

fix(minifier): bail optimizing Array with unknown arg count#22188

Merged
graphite-app[bot] merged 1 commit into
mainfrom
c/fix-array-with-unknown-arg-length
May 6, 2026
Merged

fix(minifier): bail optimizing Array with unknown arg count#22188
graphite-app[bot] merged 1 commit into
mainfrom
c/fix-array-with-unknown-arg-length

Conversation

@camc314

@camc314 camc314 commented May 6, 2026

Copy link
Copy Markdown
Contributor

fixes #22185

From the ECMAScript Spec:

23.1.1.1 Array ( ...values )
This function performs the following steps when called:

1. If NewTarget is undefined, let newTarget be the [active function object](https://tc39.es/ecma262/#active-function-object); else let newTarget be NewTarget.
2. Let proto be ? [GetPrototypeFromConstructor](https://tc39.es/ecma262/#sec-getprototypefromconstructor)(newTarget, "%Array.prototype%").
3. Let numberOfArgs be the number of elements in values.
4. If numberOfArgs = 0, return ! [ArrayCreate](https://tc39.es/ecma262/#sec-arraycreate)(0, proto).
5. If numberOfArgs = 1, then
   a. Let len be values[0].
   b. Let array be ! [ArrayCreate](https://tc39.es/ecma262/#sec-arraycreate)(0, proto).
   c. If len [is not a Number](https://tc39.es/ecma262/#sec-ecmascript-language-types-number-type), then
      i. Perform ! [CreateDataPropertyOrThrow](https://tc39.es/ecma262/#sec-createdatapropertyorthrow)(array, "0", len).
      ii. Let intLen be 1𝔽.
   d. Else,
      i. Let intLen be ! [ToUint32](https://tc39.es/ecma262/#sec-touint32)(len).
      ii. If [SameValueZero](https://tc39.es/ecma262/#sec-samevaluezero)(intLen, len) is false, throw a RangeError exception.
   e. Perform ! [Set](https://tc39.es/ecma262/#sec-set-o-p-v-throw)(array, "length", intLen, true).
   f. Return array.
6. [Assert](https://tc39.es/ecma262/#assert): numberOfArgs ≥ 2.
7. Let array be ? [ArrayCreate](https://tc39.es/ecma262/#sec-arraycreate)(numberOfArgs, proto).
8. Let k be 0.
9. Repeat, while k < numberOfArgs,
   a. Let propertyKey be ! [ToString](https://tc39.es/ecma262/#sec-tostring)([𝔽](https://tc39.es/ecma262/#%F0%9D%94%BD)(k)).
   b. Let itemK be values[k].
   c. Perform ! [CreateDataPropertyOrThrow](https://tc39.es/ecma262/#sec-createdatapropertyorthrow)(array, propertyKey, itemK).
   d. Set k to k + 1.
10. [Assert](https://tc39.es/ecma262/#assert): The [mathematical value of](https://tc39.es/ecma262/#mathematical-value-of) array's "length" property is numberOfArgs.
11. Return array.

If there is a spread in the arguments, we cannot statically identify the number of arguments - it it incorrect to assume that the spread-ed entry it non-zero in length.

We could technically improve this approach by realizing if they are >1 non-spread arguments, then it's ok to move this into an [], but i've left it out of scope of this PR

Copilot AI review requested due to automatic review settings May 6, 2026 16:03
@camc314 camc314 self-assigned this May 6, 2026
@camc314 camc314 added the A-minifier Area - Minifier label May 6, 2026
@codspeed-hq

codspeed-hq Bot commented May 6, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing c/fix-array-with-unknown-arg-length (f76cfee) with main (d25279e)2

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (f97bca9) during the generation of this report, so d25279e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@camc314 camc314 requested review from Copilot and removed request for Copilot May 6, 2026 16:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 6, 2026

camc314 commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

fixes #22185

From the ECMAScript Spec:

```
23.1.1.1 Array ( ...values )
This function performs the following steps when called:

1. If NewTarget is undefined, let newTarget be the [active function object](https://tc39.es/ecma262/#active-function-object); else let newTarget be NewTarget.
2. Let proto be ? [GetPrototypeFromConstructor](https://tc39.es/ecma262/#sec-getprototypefromconstructor)(newTarget, "%Array.prototype%").
3. Let numberOfArgs be the number of elements in values.
4. If numberOfArgs = 0, return ! [ArrayCreate](https://tc39.es/ecma262/#sec-arraycreate)(0, proto).
5. If numberOfArgs = 1, then
   a. Let len be values[0].
   b. Let array be ! [ArrayCreate](https://tc39.es/ecma262/#sec-arraycreate)(0, proto).
   c. If len [is not a Number](https://tc39.es/ecma262/#sec-ecmascript-language-types-number-type), then
      i. Perform ! [CreateDataPropertyOrThrow](https://tc39.es/ecma262/#sec-createdatapropertyorthrow)(array, "0", len).
      ii. Let intLen be 1𝔽.
   d. Else,
      i. Let intLen be ! [ToUint32](https://tc39.es/ecma262/#sec-touint32)(len).
      ii. If [SameValueZero](https://tc39.es/ecma262/#sec-samevaluezero)(intLen, len) is false, throw a RangeError exception.
   e. Perform ! [Set](https://tc39.es/ecma262/#sec-set-o-p-v-throw)(array, "length", intLen, true).
   f. Return array.
6. [Assert](https://tc39.es/ecma262/#assert): numberOfArgs ≥ 2.
7. Let array be ? [ArrayCreate](https://tc39.es/ecma262/#sec-arraycreate)(numberOfArgs, proto).
8. Let k be 0.
9. Repeat, while k < numberOfArgs,
   a. Let propertyKey be ! [ToString](https://tc39.es/ecma262/#sec-tostring)([𝔽](https://tc39.es/ecma262/#%F0%9D%94%BD)(k)).
   b. Let itemK be values[k].
   c. Perform ! [CreateDataPropertyOrThrow](https://tc39.es/ecma262/#sec-createdatapropertyorthrow)(array, propertyKey, itemK).
   d. Set k to k + 1.
10. [Assert](https://tc39.es/ecma262/#assert): The [mathematical value of](https://tc39.es/ecma262/#mathematical-value-of) array's "length" property is numberOfArgs.
11. Return array.
```

If there is a spread in the arguments, we cannot statically identify the number of arguments - it it incorrect to assume that the spread-ed entry it non-zero in length.

We could technically improve this approach by realizing if they are >1 non-spread arguments, then it's ok to move this into an `[]`, but i've left it out of scope of this PR
@graphite-app graphite-app Bot force-pushed the c/fix-array-with-unknown-arg-length branch from f76cfee to 3b385e2 Compare May 6, 2026 16:49
@graphite-app graphite-app Bot merged commit 3b385e2 into main May 6, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 6, 2026
@graphite-app graphite-app Bot deleted the c/fix-array-with-unknown-arg-length branch May 6, 2026 16:53
graphite-app Bot pushed a commit that referenced this pull request May 7, 2026
Given `new Array(a,b,...foo)`, the number of arguments is guarenteed to be `>=2` so we can perform the optimization.

fixes #22194

follow on from #22188
camc314 added a commit that referenced this pull request May 11, 2026
### 🚀 Features

- 66c9b01 transformer/typescript: Debug_assert that `enum_eval` ran in
semantic (#22252) (Dunqing)
- ffe6475 minifier: Fold `Array` constructor with safe spreads (#22215)
(camc314)

### 🐛 Bug Fixes

- d3d0b18 traverse: Handle `ChainElement::TSNonNullExpression` in
`GatherNodeParts` (#22247) (leaysgur)
- 4e880de transformer/object-rest-spread: Declare temp vars for computed
keys (#22284) (camc314)
- a7c3e22 semantic: Clear member write target for computed keys (#22302)
(camc314)
- 6a8852d codegen: Emit newline after legal-comment orphan flush
(#22304) (Dunqing)
- 5da9fda transformer/explicit-resource-management: Preserve class names
(#22306) (Dunqing)
- b5d970f transformer/explicit-resource-management: Preserve class names
(#22290) (camc314)
- bc54fd4 minifier: Keep function / class names if direct eval is
present in the scope (#22241) (sapphi-red)
- 7a810c0 minifier: Refresh direct eval flags after DCE (#21787)
(Dunqing)
- dd88726 transformer/legacy-decorator: Preserve accessor type
annotation for emitDecoratorMetadata (#21966) (Dunqing)
- 29a3cd7 codegen: Swap mapping/indent order for top-level decls
(#22206) (Dunqing)
- 73b4f40 minifier: Preserve catch binding with direct eval (#22221)
(camc314)
- 0e13d17 minifier: Preserve optional chain base side effects (#22219)
(camc314)
- 0c7c01c transformer/typescript: Inline optional-chain enum member
access (#21834) (Dunqing)
- a6aff7e codegen: Emit block/array/object end mapping at close char
(#22200) (Dunqing)
- a099b03 codegen: Emit call end mapping at `)` position, not past it
(#22199) (Dunqing)
- 5753774 minifier: Cap if-return ternary collapse for firefox (#21841)
(Gurupungav Narayanan)
- 2493bdd codegen: Correct sourcemap end mappings for closing delimiters
(#22001) (Mark Dalgleish)
- 3b385e2 minifier: Bail optimizing `Array` with unknown arg count
(#22188) (camc314)
- 9fa2122 parser: Parse array computed class keys (#22159) (camc314)

### 📚 Documentation

- a4a6892 napi/parser: Correct code comment (#22278) (overlookmotel)
- 9305373 oxc: Update README (#22178) (camc314)

Co-authored-by: Cameron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect minification of new Array(Foo, ...Bar) drops spread arguments

2 participants