Skip to content

fix(transformer): sync scoping after lowering an enum to IIFE#22502

Draft
Dunqing wants to merge 1 commit into
mainfrom
fix/transformer-enum-scoping-after-iife-lower
Draft

fix(transformer): sync scoping after lowering an enum to IIFE#22502
Dunqing wants to merge 1 commit into
mainfrom
fix/transformer-enum-scoping-after-iife-lower

Conversation

@Dunqing

@Dunqing Dunqing commented May 17, 2026

Copy link
Copy Markdown
Member

transform_ts_enum rewrites enum E { … } into var/let E = (function(E) { … })(E || {}) but left the semantic table describing the old shape:

  • the enum's symbol still carried RegularEnum/ConstEnum flags, even though the binding now backs a plain var/let;
  • the function-expression body scope (re-used from the enum body) had default scope flags instead of Function;
  • member names (Up, Down, …) remained bound in that scope despite living on the runtime enum object now, not as JS-level bindings.

TransformerReturn exposes this scoping table, so downstream consumers — oxc_minifier, the mangler, NAPI clients — were reading a model that no longer described the emitted code. The transform-checker conformance suite was snapshotting this as ~20 fixtures' worth of Bindings mismatch / Symbol flags mismatch / Scope flags mismatch errors (optimize-enums/typeof-kept, exported-not-removed, and the new const-enum-*-kept fixtures from the parent PR).

Fix

Sync the scoping table when lowering happens:

  • set the symbol's flags to FunctionScopedVariable / BlockScopedVariable based on var vs let;
  • add ScopeFlags::Function to the body scope;
  • drop enum-member bindings from that scope.

Skip the is_already_declared (enum-merge) path's symbol-flag update, since that path reuses an existing outer binding.

Identifying members by their EnumMember symbol flag (rather than by name) naturally handles enum y { y = 123 }: the IIFE param's binding has overwritten the member's entry under that name, so its symbol is no longer reachable via get_bindings, and the param's FunctionScopedVariable symbol survives untouched.

Why the updates are queued and drained per scope

transform_ts_enum runs in enter_statement (before sibling references are visited) when the enum can't be fully removed — e.g. enum Mixed { A = "hello", B } (B's auto-increment after a string isn't statically evaluable), or export enum E { … }. Rewriting the symbol flags eagerly there would make the inliner later see FunctionScopedVariable for sibling Mixed.A / Direction.Up references, fail the is_const_enum / RegularEnum check, and skip inlining.

Queue the updates on TypeScriptEnum.pending_scoping_updates with each enum's enclosing_scope_id, and drain them in exit_statements for that scope. By the time exit_statements fires for the enclosing scope, the entire subtree (where any reference to the enum could live) has been visited depth-first, so every enter_expression that could inline an E.X access has already run.

The flush runs unconditionally because transform_ts_enum is invoked unconditionally for exported enums regardless of the optimize_* flags; the optimize-gated loop in exit_statements only deals with deferred-removable enums.

Impact

Pass count: 236 → 246 (oxc.snap.md) and 700 → 711 (babel.snap.md); ~19 fixtures move from semantic-mismatch failures to passing, plus 2 more (optimize-enums/auto-increment-after-string, optimize-enums/exported-not-removed) that the deferred-update approach unblocks. ~660 lines of snapshot errors removed.

Dunqing commented May 17, 2026

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


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.

@codspeed-hq

codspeed-hq Bot commented May 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix/transformer-enum-scoping-after-iife-lower (f250d22) with main (e6090e7)

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/transformer-enum-scoping-after-iife-lower branch 5 times, most recently from 3ac367a to 1c28e2d Compare May 18, 2026 00:34
@github-actions github-actions Bot added the A-semantic Area - Semantic label May 18, 2026
@Dunqing Dunqing force-pushed the fix/transformer-enum-scoping-after-iife-lower branch 3 times, most recently from f2a4208 to 8591185 Compare May 18, 2026 01:20
@Dunqing Dunqing force-pushed the fix/transformer-const-enum-value-ref-kept branch from da9fe2a to ae6439a Compare May 18, 2026 01:20
@Dunqing Dunqing force-pushed the fix/transformer-enum-scoping-after-iife-lower branch from 8591185 to f4778ae Compare May 18, 2026 01:45
@Dunqing Dunqing force-pushed the fix/transformer-const-enum-value-ref-kept branch from ae6439a to c723603 Compare May 18, 2026 01:45
@graphite-app graphite-app Bot changed the base branch from fix/transformer-const-enum-value-ref-kept to graphite-base/22502 May 18, 2026 02:19
@graphite-app graphite-app Bot force-pushed the graphite-base/22502 branch from c723603 to e6090e7 Compare May 18, 2026 02:24
@graphite-app graphite-app Bot force-pushed the fix/transformer-enum-scoping-after-iife-lower branch from f4778ae to fa62d6c Compare May 18, 2026 02:24
@graphite-app graphite-app Bot changed the base branch from graphite-base/22502 to main May 18, 2026 02:24
`transform_ts_enum` rewrites `enum E { … }` into `var/let E =
(function(E) { … })(E || {})` but left the semantic table describing
the *old* shape:

  * the enum's symbol still carried `RegularEnum`/`ConstEnum` flags,
    even though the binding now backs a plain `var`/`let`;
  * the function-expression body scope (re-used from the enum body)
    still had default scope flags instead of `Function`;
  * member names (`Up`, `Down`, …) remained bound in that scope despite
    living on the runtime enum object now, not as JS-level bindings.

`TransformerReturn` exposes this scoping table, so downstream consumers
— `oxc_minifier`, the mangler, NAPI clients — were reading a model that
no longer described the emitted code. The transform-checker conformance
suite had been snapshotting this as ~20 fixtures' worth of
"Bindings mismatch / Symbol flags mismatch / Scope flags mismatch"
errors (`optimize-enums/typeof-kept`, `exported-not-removed`, and the
new `const-enum-*-kept` fixtures from the parent branch).

Sync the scoping table inside `transform_ts_enum` once the lowering
decides what statement to emit: set the symbol's flags to
`FunctionScopedVariable`/`BlockScopedVariable` based on `var` vs `let`,
add `ScopeFlags::Function` to the body scope, and drop member bindings
from that scope. Skip the `is_already_declared` (enum-merge) path's
symbol-flag update, since that path reuses an existing outer binding.

Watch out for name collisions like `enum y { y = 123 }`: the IIFE param
`y` overwrites the member binding `y` under the same name. Skip
removal when the live binding now points at the param.

Pass count goes 236 → 244 (oxc.snap.md) and 700 → 711 (babel.snap.md):
+19 fixtures move from semantic-mismatch failures to passing.
@graphite-app graphite-app Bot force-pushed the fix/transformer-enum-scoping-after-iife-lower branch from fa62d6c to f250d22 Compare May 18, 2026 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant