Skip to content

fix(minifier): treat Object.isExtensible/isFrozen/isSealed as pure#23916

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/minifier-object-isextensible-pure
Jun 29, 2026
Merged

fix(minifier): treat Object.isExtensible/isFrozen/isSealed as pure#23916
graphite-app[bot] merged 1 commit into
mainfrom
fix/minifier-object-isextensible-pure

Conversation

@Dunqing

@Dunqing Dunqing commented Jun 29, 2026

Copy link
Copy Markdown
Member

What

Object.isExtensible, Object.isFrozen, and Object.isSealed were grouped with the throwing introspection methods (Object.keys, getOwnPropertyDescriptor, …) in CallExpression::may_have_side_effects, so a call with a missing / null / undefined argument was conservatively kept.

Unlike those methods, the three is* methods never ToObject their target — per spec a non-object receiver returns a primitive (false / true) instead of throwing, so the call is side-effect-free and an unused one can be dropped.

// in
Object.isExtensible();
Object.isFrozen(x);
Object.isSealed();
Object.keys();

// out
Object.isFrozen(x), Object.keys();

isExtensible() / isSealed() drop (pure); isFrozen(x) stays — an undetermined target could be a Proxy, whose [[IsExtensible]] / [[OwnPropertyKeys]] traps are observable — and keys() stays (it throws on the undefined receiver).

Covered by new cases in crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs; minsize unchanged.

Closes #23779

Dunqing commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the A-minifier Area - Minifier label Jun 29, 2026
@Dunqing
Dunqing marked this pull request as ready for review June 29, 2026 01:54
@codspeed-hq

codspeed-hq Bot commented Jun 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing fix/minifier-object-isextensible-pure (eab01b0) with main (0b07c4c)

Open in CodSpeed

Footnotes

  1. 9 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.

@sapphi-red sapphi-red added the 0-merge Merge with Graphite Merge Queue label Jun 29, 2026

sapphi-red commented Jun 29, 2026

Copy link
Copy Markdown
Member

Merge activity

…23916)

## What

`Object.isExtensible`, `Object.isFrozen`, and `Object.isSealed` were grouped with the throwing introspection methods (`Object.keys`, `getOwnPropertyDescriptor`, …) in `CallExpression::may_have_side_effects`, so a call with a missing / `null` / `undefined` argument was conservatively kept.

Unlike those methods, the three `is*` methods never `ToObject` their target — per [spec](https://tc39.es/ecma262/#sec-object.isextensible) a non-object receiver returns a primitive (`false` / `true`) instead of throwing, so the call is side-effect-free and an unused one can be dropped.

```js
// in
Object.isExtensible();
Object.isFrozen(x);
Object.isSealed();
Object.keys();

// out
Object.isFrozen(x), Object.keys();
```

`isExtensible()` / `isSealed()` drop (pure); `isFrozen(x)` stays — an undetermined target could be a `Proxy`, whose `[[IsExtensible]]` / `[[OwnPropertyKeys]]` traps are observable — and `keys()` stays (it throws on the `undefined` receiver).

Covered by new cases in `crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs`; `minsize` unchanged.

Closes #23779
@graphite-app
graphite-app Bot force-pushed the fix/minifier-object-isextensible-pure branch from eab01b0 to dcc9a73 Compare June 29, 2026 13:51
@graphite-app
graphite-app Bot merged commit dcc9a73 into main Jun 29, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jun 29, 2026
@graphite-app
graphite-app Bot deleted the fix/minifier-object-isextensible-pure branch June 29, 2026 13:55
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
…nts (#23917)

## What

`is_pure_global_method_call` (and the bare-global path) in `CallExpression::may_have_side_effects` treated a call as side-effect-free whenever its arguments were, ignoring that several of these functions **throw** at runtime. Unused calls that throw were therefore dropped as dead code. terser and esbuild both keep them.

Guard the cases oxc can prove:

| Call | Throws | Before | After |
| --- | --- | --- | --- |
| `String.raw()`, `Symbol.keyFor()`, `Symbol.keyFor(42)` | `TypeError` (missing/invalid required arg) | dropped | kept |
| `URL.canParse()` | `TypeError` (required first arg) | dropped | kept |
| `String.fromCodePoint(-1 / 1.5 / 0x110000)` | `RangeError` (code point outside `[0, 0x10FFFF]`) | dropped | kept |
| `Math.abs(10n)`, `Date.UTC(10n)`, `String.fromCharCode(10n)`, `isNaN(10n)`, … | `TypeError` (`ToNumber(BigInt)`) | dropped | kept |

Safe optimizations are preserved — `URL.canParse("x")`, `String.fromCodePoint(65)`, `String.fromCharCode(65)`, `Math.abs(1)` still drop. `String.raw` / `Symbol.keyFor` are removed from the pure-method list (never provably safe); the rest get argument guards.

### A Symbol argument is left droppable, on purpose

A Symbol argument also throws (`ToString` / `ToNumber` on a Symbol), but oxc has no `ValueType::Symbol` and `to_primitive` never resolves a pure expression to `Symbol`, so a Symbol throw is **never provable**. Guarding it would mean keeping every `Math.abs(x)` / `parseInt(x)` on an undetermined argument — a large minification regression. esbuild makes the same trade-off (it drops `Math.abs(Symbol.iterator)` too), so only the provable cases are guarded.

oxc already guards exactly this for the `Number` / `Symbol` / `BigInt` / `Error` constructors; this brings the method-call path in line. The assumption (including the non-provable-Symbol carve-out) is documented in `crates/oxc_minifier/docs/ASSUMPTIONS.md`.

Covered by `test_throwing_global_calls` in `crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs`; cross-checked against terser and esbuild; `minsize` unchanged.

---

Stacked on #23916.
camc314 pushed a commit that referenced this pull request Jul 3, 2026
…23916)

## What

`Object.isExtensible`, `Object.isFrozen`, and `Object.isSealed` were grouped with the throwing introspection methods (`Object.keys`, `getOwnPropertyDescriptor`, …) in `CallExpression::may_have_side_effects`, so a call with a missing / `null` / `undefined` argument was conservatively kept.

Unlike those methods, the three `is*` methods never `ToObject` their target — per [spec](https://tc39.es/ecma262/#sec-object.isextensible) a non-object receiver returns a primitive (`false` / `true`) instead of throwing, so the call is side-effect-free and an unused one can be dropped.

```js
// in
Object.isExtensible();
Object.isFrozen(x);
Object.isSealed();
Object.keys();

// out
Object.isFrozen(x), Object.keys();
```

`isExtensible()` / `isSealed()` drop (pure); `isFrozen(x)` stays — an undetermined target could be a `Proxy`, whose `[[IsExtensible]]` / `[[OwnPropertyKeys]]` traps are observable — and `keys()` stays (it throws on the `undefined` receiver).

Covered by new cases in `crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs`; `minsize` unchanged.

Closes #23779
camc314 pushed a commit that referenced this pull request Jul 3, 2026
…nts (#23917)

## What

`is_pure_global_method_call` (and the bare-global path) in `CallExpression::may_have_side_effects` treated a call as side-effect-free whenever its arguments were, ignoring that several of these functions **throw** at runtime. Unused calls that throw were therefore dropped as dead code. terser and esbuild both keep them.

Guard the cases oxc can prove:

| Call | Throws | Before | After |
| --- | --- | --- | --- |
| `String.raw()`, `Symbol.keyFor()`, `Symbol.keyFor(42)` | `TypeError` (missing/invalid required arg) | dropped | kept |
| `URL.canParse()` | `TypeError` (required first arg) | dropped | kept |
| `String.fromCodePoint(-1 / 1.5 / 0x110000)` | `RangeError` (code point outside `[0, 0x10FFFF]`) | dropped | kept |
| `Math.abs(10n)`, `Date.UTC(10n)`, `String.fromCharCode(10n)`, `isNaN(10n)`, … | `TypeError` (`ToNumber(BigInt)`) | dropped | kept |

Safe optimizations are preserved — `URL.canParse("x")`, `String.fromCodePoint(65)`, `String.fromCharCode(65)`, `Math.abs(1)` still drop. `String.raw` / `Symbol.keyFor` are removed from the pure-method list (never provably safe); the rest get argument guards.

### A Symbol argument is left droppable, on purpose

A Symbol argument also throws (`ToString` / `ToNumber` on a Symbol), but oxc has no `ValueType::Symbol` and `to_primitive` never resolves a pure expression to `Symbol`, so a Symbol throw is **never provable**. Guarding it would mean keeping every `Math.abs(x)` / `parseInt(x)` on an undetermined argument — a large minification regression. esbuild makes the same trade-off (it drops `Math.abs(Symbol.iterator)` too), so only the provable cases are guarded.

oxc already guards exactly this for the `Number` / `Symbol` / `BigInt` / `Error` constructors; this brings the method-call path in line. The assumption (including the non-provable-Symbol carve-out) is documented in `crates/oxc_minifier/docs/ASSUMPTIONS.md`.

Covered by `test_throwing_global_calls` in `crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs`; cross-checked against terser and esbuild; `minsize` unchanged.

---

Stacked on #23916.
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.

Object.isExtensible() should be treated as sideeffect free

2 participants