Skip to content

fix(minifier): refresh direct eval flags after DCE#21787

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/minifier-direct-eval-idempotency
May 8, 2026
Merged

fix(minifier): refresh direct eval flags after DCE#21787
graphite-app[bot] merged 1 commit into
mainfrom
fix/minifier-direct-eval-idempotency

Conversation

@Dunqing

@Dunqing Dunqing commented Apr 26, 2026

Copy link
Copy Markdown
Member

Stacked on #21645.

Summary

Fixes stale DirectEval scope flags after peephole passes remove a dead direct eval.

Problem

Semantic analysis marks scopes that contain direct eval. The minifier uses that flag conservatively: if a scope may contain direct eval, declarations in that scope cannot always be removed because eval may reference them dynamically.

However, peephole compression can remove the direct eval itself:

function f(){if(false)eval('x');var x}f()

Before this fix, the AST no longer contained the eval, but the semantic scope still kept the stale DirectEval flag. That stale flag made declaration removal too conservative during the same minifier run. A fresh reparse did not have the stale flag, so a second minifier run could produce smaller output, which exposed the monitor-oxc idempotency failure.

Fix

  • After a changed peephole pass, collect direct eval scope IDs from the live AST.
  • Refresh DirectEval flags in semantic scoping from that live set.
  • Keep DirectEval protection for scopes that still contain a live direct eval.
  • Remove stale DirectEval protection only for scopes whose direct eval was eliminated by earlier compression.

Test

Adds a minimal regression in remove_unused_declaration.rs using the existing test_options helper:

function f(){if(false)eval('x');var x}f()

The test verifies the dead eval no longer keeps var x alive in the same minifier run.

Testing

  • cargo test -p oxc_minifier remove_unused_declaration_after_dead_direct_eval

Dunqing commented Apr 26, 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.

@Dunqing Dunqing force-pushed the fix/minifier-compressor-idempotency branch from fc0be6a to d1c71e5 Compare April 26, 2026 15:02
@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 0eca4b1 to 9b1aadc Compare April 26, 2026 15:02
@codspeed-hq

codspeed-hq Bot commented Apr 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix/minifier-direct-eval-idempotency (5bfa1a7) with fix/minifier-compressor-idempotency (5e7520a)

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.

@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 9b1aadc to 627e0ac Compare April 26, 2026 15:10
@Dunqing Dunqing force-pushed the fix/minifier-compressor-idempotency branch from d1c71e5 to f5ad5f5 Compare April 26, 2026 15:10
@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 627e0ac to 1718603 Compare April 26, 2026 15:18
@Dunqing Dunqing force-pushed the fix/minifier-compressor-idempotency branch from f5ad5f5 to bb6bc86 Compare April 26, 2026 15:18
@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 1718603 to 28f2e93 Compare April 26, 2026 15:35
@Dunqing Dunqing force-pushed the fix/minifier-compressor-idempotency branch 2 times, most recently from ead983e to e2d329d Compare April 26, 2026 15:38
@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 28f2e93 to aacbb9f Compare April 26, 2026 15:38
@Dunqing Dunqing force-pushed the fix/minifier-compressor-idempotency branch from e2d329d to f9554bb Compare April 26, 2026 15:50
@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch 2 times, most recently from f423d92 to b7a5415 Compare April 26, 2026 16:12
@Dunqing Dunqing force-pushed the fix/minifier-compressor-idempotency branch from f9554bb to 5e7520a Compare April 26, 2026 16:12
@graphite-app graphite-app Bot changed the base branch from fix/minifier-compressor-idempotency to graphite-base/21787 April 27, 2026 05:54
@camc314 camc314 added the A-minifier Area - Minifier label May 7, 2026
@Dunqing Dunqing marked this pull request as ready for review May 7, 2026 14:10
@Dunqing Dunqing requested review from camc314 and sapphi-red May 7, 2026 14:10
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 7, 2026

camc314 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 7, 5:05 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 7, 5:06 PM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • May 7, 5:06 PM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..
  • May 7, 5:06 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 8, 1:01 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.
  • May 8, 1:53 AM UTC: camc314 added this pull request to the Graphite merge queue.
  • May 8, 1:58 AM UTC: Merged by the Graphite merge queue.

@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 5e4f20f to 866155d Compare May 8, 2026 01:00
@camc314 camc314 force-pushed the graphite-base/21787 branch from 5e7520a to 24946be Compare May 8, 2026 01:00
@graphite-app graphite-app Bot changed the base branch from graphite-base/21787 to main May 8, 2026 01:01
@Dunqing Dunqing force-pushed the fix/minifier-direct-eval-idempotency branch from 866155d to 45dd0cf Compare May 8, 2026 01:49
Stacked on #21645.

## Summary

Fixes stale `DirectEval` scope flags after peephole passes remove a dead direct eval.

## Problem

Semantic analysis marks scopes that contain direct eval. The minifier uses that flag conservatively: if a scope may contain direct eval, declarations in that scope cannot always be removed because eval may reference them dynamically.

However, peephole compression can remove the direct eval itself:

```js
function f(){if(false)eval('x');var x}f()
```

Before this fix, the AST no longer contained the `eval`, but the semantic scope still kept the stale `DirectEval` flag. That stale flag made declaration removal too conservative during the same minifier run. A fresh reparse did not have the stale flag, so a second minifier run could produce smaller output, which exposed the `monitor-oxc` idempotency failure.

## Fix

- After a changed peephole pass, collect direct eval scope IDs from the live AST.
- Refresh `DirectEval` flags in semantic scoping from that live set.
- Keep `DirectEval` protection for scopes that still contain a live direct eval.
- Remove stale `DirectEval` protection only for scopes whose direct eval was eliminated by earlier compression.

## Test

Adds a minimal regression in `remove_unused_declaration.rs` using the existing `test_options` helper:

```js
function f(){if(false)eval('x');var x}f()
```

The test verifies the dead eval no longer keeps `var x` alive in the same minifier run.

## Testing

- `cargo test -p oxc_minifier remove_unused_declaration_after_dead_direct_eval`
@graphite-app graphite-app Bot force-pushed the fix/minifier-direct-eval-idempotency branch from 45dd0cf to 7a810c0 Compare May 8, 2026 01:54
@graphite-app graphite-app Bot merged commit 7a810c0 into main May 8, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 8, 2026
@graphite-app graphite-app Bot deleted the fix/minifier-direct-eval-idempotency branch May 8, 2026 01:58
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.

3 participants