Skip to content

fix(minifier): propagate PURE annotation when inlining pure IIFE#21526

Closed
gthb wants to merge 25 commits into
oxc-project:mainfrom
gthb:worktree-oxc-iife
Closed

fix(minifier): propagate PURE annotation when inlining pure IIFE#21526
gthb wants to merge 25 commits into
oxc-project:mainfrom
gthb:worktree-oxc-iife

Conversation

@gthb

@gthb gthb commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

What

Fix bug: pure-annotated IIFEs that should be dead-code-eliminated were leaving behind a bare expression statement. var removeThis = /* @__PURE__ */ (() => stuff())() minified to stuff(); instead of being dropped entirely.

Fixes #17480

Root Cause

IIFE substitution runs before the unused-declaration pass and was discarding the outer PURE when unwrapping (() => foo())() to foo(). By the time the unused-declaration pass saw var x = foo() it could no longer prove the RHS pure.

Fix

The three inlining branches in substitute_iife_call now go through a small helper take_and_propagate_pure that, when the outer IIFE was pure, calls mark_inlined_pure on the taken expression. mark_inlined_pure walks every shape the outer PURE legitimately covers — Sequence / Conditional / Logical / Unary / Binary / Chain containers, call & new callee/argument positions (including spreads), member-object positions, and TS wrappers (stripped via get_inner_expression_mut) — stamping pure = true on each Call / New it finds. A sibling mark_chain_element_pure mirrors the logic for ChainElement children.

Expression::may_have_side_effects in oxc_ecmascript is also taught that the five TS wrappers are runtime-transparent and delegates to get_inner_expression. Required for the IIFE fix to actually drop a (() => foo() as any)() repro.

Not addressed

A few positions silently lose PURE because the AST has no pure flag for them (TaggedTemplateExpression, array/object elements, template-literal interpolations, computed-member indices). Missed-optimization gaps, not correctness issues, and pinned by negative tests.

Drive-by

The NewExpression::pure doc comment was attached to the arguments field; moved it onto pure where it belongs.

gthb added 2 commits April 17, 2026 10:04
…itializer

Reproduces oxc-projectGH-17480: when a pure-annotated IIFE is inlined as a variable
initializer, the annotation is dropped from the inner call, so the
unused-declaration pass cannot remove it.
When substituting a pure-annotated IIFE with its body expression (e.g.
`/* @__PURE__ */ (() => foo())()` -> `foo()`), the annotation was
dropped. This prevented downstream passes from removing the call when
its result is unused, so `var x = /* @__PURE__ */ (() => foo())()`
became `foo();` instead of being removed entirely.

Carry the annotation onto the inlined CallExpression or NewExpression so
the unused-declaration pass can still drop it.

Closes oxc-project#17480
@github-actions github-actions Bot added A-minifier Area - Minifier C-bug Category - Bug labels Apr 17, 2026
@codspeed-hq

codspeed-hq Bot commented Apr 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing gthb:worktree-oxc-iife (225789c) with main (611b3b6)2

Open in CodSpeed

Footnotes

  1. 3 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 (b0817ae) during the generation of this report, so 611b3b6 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

gthb added 15 commits April 17, 2026 10:53
…notation

Add tests for `/* @__PURE__ */ (() => { foo() })()` in both the
inline-preservation and end-to-end unused-var-removal cases. This path
takes the sequence branch of `substitute_iife_call` that wasn't covered
by the initial fix-up tests. Fold the three take/propagate-pure call
sites into a single `take_propagating_pure` helper.
…e` paths

Plug the two coverage gaps noted in branch review: a function-expression
IIFE (which isn't inlined — its outer `pure` is what the
unused-declaration pass keys off), and the `annotations: false` path
(which must ignore PURE annotations end-to-end, including during IIFE
body inlining).
…ng PURE

When inlining a pure-annotated IIFE whose body is a sequence expression
(e.g. `/* @__PURE__ */ (() => (foo(), bar()))()`), the outer PURE flag
was dropped because `take_propagating_pure` only annotated a top-level
`CallExpression` or `NewExpression`. Extract the annotation step into
`mark_inlined_pure` and recurse into `SequenceExpression.expressions`
so every call/new element inherits the outer annotation, letting later
passes drop them.
If the inlined call already carries its own `/* @__PURE__ */`, the outer
annotation simply lands on a `pure` flag that is already set. Lock that
in so a future change to the propagation helper doesn't accidentally
duplicate, clear, or require a conditional around the inner annotation.
Extend `mark_inlined_pure` so the outer `/* @__PURE__ */` of an arrow
IIFE reaches inner calls inside more container shapes when the IIFE is
inlined:

- `ConditionalExpression` (test, consequent, alternate)
- `LogicalExpression` (left, right)
- `ChainExpression` (via a small `ChainElement` helper)
- TS wrappers: `as`, `!`, `satisfies`, `<T>x`, and `x<T>`

Each arm recurses with the same helper, so arbitrary nesting (e.g.
`(() => c ? (a(), b()) : d())()`) is handled too. Without this, code
like `/* @__PURE__ */ (() => c ? foo() : bar())()` was left with the
conditional's call branches un-annotated and the surrounding unused
variable could not be dropped.

Cover the common body shapes in `remove_unused_expression.rs` (showing
the annotation lands on the inner call) and in
`remove_unused_declaration.rs` (showing the declaration now drops for
conditional/logical/optional-chain bodies). The TS wrappers are covered
at the expression level only — the unused-declaration pass does not
strip them, so `var x = /* @__PURE__ */ a() as any;` stays put.
`TSAsExpression`, `TSNonNullExpression`, `TSSatisfiesExpression`,
`TSTypeAssertion`, and `TSInstantiationExpression` are all
runtime-transparent — they are stripped before execution — but the
`Expression::may_have_side_effects` match was falling through to the
conservative `_ => true` default for all of them. That kept e.g.
`var x = /* @__PURE__ */ a() as any;` alive even when `x` is unused,
because the unused-declaration pass consults that predicate before
dropping the initializer.

Delegate to the inner expression for each wrapper. Combined with the
existing PURE propagation during IIFE inlining, declarations like
`var x = /* @__PURE__ */ (() => a() as any)();` now drop entirely in
TypeScript sources.

This crate is shared; the minifier, linter, transformer, and isolated-
declarations test suites all still pass, and `cargo coverage -- minifier`
shows no snapshot regressions.
Pin the contract at the predicate layer, not just via the minifier's
end-to-end drops: for each TS wrapper (`as`, `!`, `satisfies`, generic
instantiation, and a nested mix), verify the side-effect answer matches
the inner expression. This catches a future regression — a typo in one
arm, a feature-flag gate, or accidental removal — at the unit level
rather than needing the whole unused-declaration pipeline to surface it.
Extend `mark_inlined_pure` to recurse through `UnaryExpression` and
`BinaryExpression` so a `/* @__PURE__ */`-annotated IIFE with a body
like `!foo()` or `a() == b()` still has its annotation preserved on
the inner calls when inlined. For `!foo()` the side-effect check
delegates purely to the argument, so this is enough to drop an unused
declaration; for comparison operators the check is a disjunction over
operand side effects, likewise sufficient. Arithmetic binary operators
still short-circuit in `may_have_side_effects` via `to_primitive`
value-type checks that can't be satisfied by an unknown call's return,
so PURE forwarding buys nothing there — harmless when it doesn't help.

`TaggedTemplateExpression` has no `pure` flag, so the annotation is
silently dropped for tagged-template IIFE bodies. Documented in the
helper's doc comment and covered by a test showing the current output.
…ression`

The same five TS wrapper variants were enumerated verbatim in
`may_have_side_effects` and in `mark_inlined_pure`. Collapse each into
a single multi-variant arm that delegates to `get_inner_expression` /
`get_inner_expression_mut` — the canonical TS/paren-unwrap helpers in
`oxc_ast`. No behavior change (the helpers skip the same set plus
`ParenthesizedExpression`, which already has its own arm in
`may_have_side_effects` and is stripped before `mark_inlined_pure`
runs).
…gate_pure`

Reads as two verbs rather than one ambiguous participle-as-adjective.
Previously all tests for `mark_inlined_pure` used a single-level body
(call, unary+call, binary+calls, etc.). Add a test where the body
stacks a `BinaryExpression` over a `UnaryExpression` over a call, so
the recursion actually descends multiple container levels before
reaching the call it marks.
…sitions

`mark_inlined_pure` previously stopped at the outermost call/new and at
member-access expressions. When the IIFE body was something like
`foo(bar())` or `foo().bar`, only the outer call got the PURE mark,
leaving the inner `bar()` or `foo()` unmarked even though the outer
`/* @__PURE__ */` guaranteed the whole body was side-effect-free.

Extend the recursion to:

- `CallExpression` / `NewExpression`: descend into the callee and each
  argument (including spread elements) after marking the outer pure.
- `StaticMemberExpression` / `ComputedMemberExpression` /
  `PrivateFieldExpression`: descend into the object, which may contain
  nested calls.
- `ChainElement`: apply the same treatment to chain-element variants so
  optional chains with inner calls carry the annotation through.

The outer mark alone was already enough to drop the declaration in the
typical case; the added recursion keeps the PURE invariant consistent
for every call in the body, so later passes can make the same decisions
if the outer call is ever unwrapped.
…d_pure`

The explicit match arm for the five TS wrapper variants was just calling
`get_inner_expression_mut()` and recursing. Moving that unwrap to the
top of the function folds all wrapper (and `ParenthesizedExpression`)
handling into a single line, keeping the subsequent match focused on
runtime-meaningful expression shapes. No behavior change.
The doc string describing the `pure` flag was sitting above `arguments`,
so generated builder docs (e.g., `expression_new_with_pure`) attributed
the "marked with /* @__PURE__ */" comment to the wrong parameter. Move
the doc comment one field down to match `CallExpression`, and regenerate
`ast_builder.rs`.
@github-actions github-actions Bot added the A-ast Area - AST label Apr 19, 2026
gthb added 8 commits April 19, 2026 01:10
…aration`

The positive cases assert that pure-annotated IIFEs around various body
shapes drop entirely. Add the symmetric negative assertions so reviewers
can see what stays behind when the body itself has side effects the
outer PURE annotation can't cover:

- An undeclared global in the conditional's test or the logical's
  left-hand side leaves a bare `undeclaredGlobal;` after the now-pure
  arms fold away.
- A spread argument leaves `[...xs];` because the iterator protocol
  runs regardless of whether the surrounding call is pure.
- A tagged-template body has no `pure` flag to receive the annotation,
  so the call is preserved as `foo``;`.
The unary / binary positive cases (`!a()`, `a() == b()`) drop because
`==` and `!` are simple disjunctions over operand side effects. Add
matching negative cases for `a() + b()` and `+a()` to make the
arithmetic-side asymmetry visible: even with both inner calls marked
PURE, `to_numeric` / `to_primitive` returns `Undetermined` for an
unknown call, so the `+` / unary-plus stays side-effectful and only
the `var` part collapses. The actual minified output —
`/* @__PURE__ */ a() + /* @__PURE__ */ b();` — also doubles as
visual evidence that the inner-call PURE marks did propagate.

Also rewrite the inscrutable comment on the positive cases to forward
to the new negative cases instead of describing the limitation in
isolation.
The doc on `take_and_propagate_pure` only listed the original container
shapes, omitting the call/new callee/argument and member-object recursion
added later. Refresh the list and add an explicit "missed-optimization
gaps" section covering `TaggedTemplateExpression`, `ArrayExpression`,
`ObjectExpression`, and `TemplateLiteral` interpolations, so a future
reader doesn't think those positions are deliberately ignored without
context.

Also note that the top-of-function `get_inner_expression_mut()` strip
relies on `normalize.rs` having removed `ParenthesizedExpression`
already, and add reciprocal "keep in sync" notes on `mark_inlined_pure`
and `mark_chain_element_pure` to highlight that the two enums overlap
on the call / TS-non-null / member variants.
…alse`

The existing `treeshake_options_annotations_false` case exercised the
no-inner-mark variant: the IIFE inlining happens but PURE is not
forwarded onto the unwrapped call. Add the inner-marked variant to
cover the orthogonal property — when the parser already attached PURE
to the inner call, the inlining must leave it intact, since the
minifier's annotations option only governs whether the minifier acts
on the flag, not whether codegen prints it.

Without this assertion, a future regression that naively cleared inner
PURE flags during the no-propagate path would go unnoticed.
Pin the missed-optimization gap noted on `mark_inlined_pure`:
`(() => g.x)()` inlines to `g.x`, which has no `pure` flag of its
own, so the var declaration can't drop and we collapse to `g.x;`.
Two new cases for `mark_chain_element_pure`:

- `(() => foo()?.bar())()` — the chain's tail call gets PURE (codegen
  prints it at the front of the chain) and the recursion through the
  member object also marks the inner `foo()`.
- `(() => foo?.[bar()])()` — the computed-member index isn't recursed
  into, so the call in the index position keeps no PURE annotation.
Previously only exercised indirectly via `annotations: false`, which masks
the outer PURE. A direct positive test documents the plain inline path.
… gap

`mark_inlined_pure` doesn't recurse into `AssignmentExpression.right`. The
assignment itself is side-effectful so the inlined expression can't drop
anyway, but a later pass that substitutes the assigned value would lose
the inner call's PURE mark. Add it to the listed gaps.
@gthb gthb marked this pull request as ready for review April 19, 2026 02:17

@Dunqing Dunqing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review

Carefully traced through the recursion in mark_inlined_pure / mark_chain_element_pure and the negative-test surface. LGTM — recommend approval.

What this fixes

The outer pure = true was silently dropped when substitute_iife_call replaced *e = expr.take_in(ctx.ast). By the time the unused-declaration pass saw var x = foo(), the assertion was gone and the RHS couldn't be proved pure. The new helpers carry the annotation onto every shape that was syntactically inside the original IIFE call.

Strengths

  • Both DCE-only and full minify benefit. substitute_iife_call runs in both branches in peephole/mod.rs (:317 and :373), so rolldown treeshake: true / minify: "dce-only" users — likely the ones who reported #17480 — get the fix too.
  • MayHaveSideEffects change for TS wrappers is a strict improvement. Previously fell through to _ => true, so any (pureCall as T) / pureCall! / pureCall satisfies T was over-flagged. Now delegates via get_inner_expression.
  • Negative tests pin the conservative cases: undeclared globals (c ? a() : b() where c is an unknown global), spread / iterator protocol, tagged templates, member access on unknown globals, arithmetic / unary-plus coercion. Each is a documented missed-optimization gap, not a correctness gap.
  • Idempotent under repeated PURE marks and under annotations: false. The two annotations: false tests are the right shape: inlining still fires, but propagation is gated on is_pure, and an inner PURE that the parser already attached survives unchanged.
  • mark_chain_element_pure is exhaustive on ChainElement (no wildcard). New variants will fail compilation — right call for this kind of helper.
  • get_inner_expression_mut at the top defends against pre-normalization callers — the inline comment correctly notes parens would be silently stripped if anyone wired this in before normalize.rs. Worth keeping as a guardrail.
  • The drive-by NewExpression::pure doc-comment fix is correct.

Soundness check

The recursion preserves the developer's contract: /* @__PURE__ */ was a promise about the entire IIFE call. Unwrapping moves the body into the call's syntactic position, so any sub-expression that was originally covered by the assertion remains covered. The recursion stops at shapes the outer PURE couldn't have rescued anyway (assignment RHS, tagged-template tag, array/object/template-literal elements), which is also exactly the line the negative tests draw.

I couldn't find a case where marking a call pure = true here would change observable behavior.

Minor (non-blocking) observations

  • Doubled annotations in output like var a = /* @__PURE__ */ (/* @__PURE__ */ foo())?.bar() are semantically equivalent and helpful for follow-on passes; just slightly noisy. Not worth changing.
  • SequenceExpression wrapping in the { stmt() } branch can produce ((a(), b()), undefined) if the inlined body is already a sequence. remove_sequence_expression flattens this later — just noting it.
  • ConditionalExpression::test recursion: marked pure even though the test is evaluated unconditionally. Correct (outer PURE covered the test too), and the inline comment already says this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-minifier Area - Minifier C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

minifier: pure annotated iife call on unused variable initializer is not removed

2 participants