Skip to content

chore(tasks): enable enum_eval for transformer-fed semantic builders#22253

Merged
graphite-app[bot] merged 1 commit into
mainfrom
chore-tasks-enable-enum-eval-for-transformer
May 9, 2026
Merged

chore(tasks): enable enum_eval for transformer-fed semantic builders#22253
graphite-app[bot] merged 1 commit into
mainfrom
chore-tasks-enable-enum-eval-for-transformer

Conversation

@Dunqing
Copy link
Copy Markdown
Member

@Dunqing Dunqing commented May 8, 2026

Pre-requisite for #22252.

Why

#22252 adds a debug_assert! in the transformer that requires Scoping to be built with SemanticBuilder::with_enum_eval(true) whenever a TSEnumDeclaration reaches the transform pass. The internal Compiler (crates/oxc/src/compiler.rs) and the playground already enable it; the CI run on #22252 surfaced a few internal callers that don't.

Land that fix on its own first, so the assert PR doesn't trip CI on these tasks.

Sites updated

Every SemanticBuilder site that feeds Transformer::build_with_scoping:

  • tasks/track_memory_allocations/src/lib.rs — both call sites (cargo allocs).
  • tasks/benchmark/benches/transformer.rs
  • tasks/benchmark/benches/pipeline.rs
  • tasks/benchmark/benches/minifier.rs
  • tasks/benchmark/benches/codegen.rs

Other semantic builders in these files that feed only the minifier / codegen / compressor are left alone — they don't go through enum lowering.

Test plan

  • cargo check -p oxc_track_memory_allocations — clean
  • cargo check --benches -p oxc_benchmark — clean

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 8, 2026

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing chore-tasks-enable-enum-eval-for-transformer (b8ca72b) with main (bc54fd4)

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.

@Dunqing Dunqing marked this pull request as ready for review May 8, 2026 09:45
Copy link
Copy Markdown
Member Author

Dunqing commented May 8, 2026


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 added the 0-merge Merge with Graphite Merge Queue label May 9, 2026
Copy link
Copy Markdown
Member Author

Dunqing commented May 9, 2026

Merge activity

#22253)

Pre-requisite for #22252.

## Why

#22252 adds a `debug_assert!` in the transformer that requires `Scoping` to be built with `SemanticBuilder::with_enum_eval(true)` whenever a `TSEnumDeclaration` reaches the transform pass. The internal `Compiler` (`crates/oxc/src/compiler.rs`) and the playground already enable it; the CI run on #22252 surfaced a few internal callers that don't.

Land that fix on its own first, so the assert PR doesn't trip CI on these tasks.

## Sites updated

Every `SemanticBuilder` site that feeds `Transformer::build_with_scoping`:

- `tasks/track_memory_allocations/src/lib.rs` — both call sites (`cargo allocs`).
- `tasks/benchmark/benches/transformer.rs`
- `tasks/benchmark/benches/pipeline.rs`
- `tasks/benchmark/benches/minifier.rs`
- `tasks/benchmark/benches/codegen.rs`

Other semantic builders in these files that feed only the minifier / codegen / compressor are left alone — they don't go through enum lowering.

## Test plan

- [x] `cargo check -p oxc_track_memory_allocations` — clean
- [x] `cargo check --benches -p oxc_benchmark` — clean
@graphite-app graphite-app Bot force-pushed the chore-tasks-enable-enum-eval-for-transformer branch from b8ca72b to 6749b02 Compare May 9, 2026 00:11
@graphite-app graphite-app Bot merged commit 6749b02 into main May 9, 2026
28 checks passed
@graphite-app graphite-app Bot deleted the chore-tasks-enable-enum-eval-for-transformer branch May 9, 2026 00:15
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 9, 2026
graphite-app Bot pushed a commit that referenced this pull request May 10, 2026
…mantic (#22252)

Closes #21667.

Stacked on #22253 (which lands the prerequisite `with_enum_eval(true)` calls in `cargo allocs` and the four transformer benches that this assert would otherwise trip).

## Problem

The TS enum lowering reads pre-computed member values from `Scoping`. Those values are only populated when `SemanticBuilder::with_enum_eval(true)` is set; without it, every `get_enum_member_value` call returns `None`, so string-valued members are treated as opaque expressions and the transform emits incorrect reverse mappings.

Example from #21667:

```ts
enum Theme {
  Light = "Light",
  Dark = "Dark",
  Default = Theme.Light,
}
```

When the transformer runs on a `Scoping` built without `enum_eval`, it does not recognise `Theme.Light` as a string and produces:

```js
Theme[Theme["Default"] = Theme.Light] = "Default";
```

— i.e. a reverse-mapping assignment for a string member, which is wrong (and runtime-observable).

## Fix

Add a `debug_assert!` at the entry of `transform_ts_enum` (after the `decl.declare` early return) verifying that the enum's body scope is registered in `Scoping`.

The check uses `get_enum_body_scopes` as the signal: `add_enum_body_scope` has exactly one caller (`evaluate_enum_members`), which is gated entirely by `enum_eval`. No new state on `Scoping` / `EnumData`.

The internal `Compiler` (`crates/oxc/src/compiler.rs`) and the playground (`napi/playground/src/lib.rs`) already enable `enum_eval`. The benches and `cargo allocs` are fixed in the parent PR. This guard catches the misuse for any other external caller of `Transformer::build_with_scoping`.

## Test plan

- [x] `cargo test -p oxc_transformer` — new integration tests in `tests/integrations/enum_eval.rs` cover both the positive case and the `#[should_panic]` case (debug-only)
- [x] `cargo run -p oxc_transform_conformance` — no snapshot changes (the conformance runner goes through `CompilerInterface`, which already enables `enum_eval`)
- [x] `cargo clippy -p oxc_transformer --tests` — clean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants