Skip to content

fix(transformer/explicit-resource-management): align lexical binding scopes#22320

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/erm-scope-bindings
May 14, 2026
Merged

fix(transformer/explicit-resource-management): align lexical binding scopes#22320
graphite-app[bot] merged 1 commit into
mainfrom
codex/erm-scope-bindings

Conversation

@camc314

@camc314 camc314 commented May 11, 2026

Copy link
Copy Markdown
Contributor

Part of #9699

@camc314 camc314 self-assigned this May 11, 2026
@github-actions github-actions Bot added A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler labels May 11, 2026
@codspeed-hq

codspeed-hq Bot commented May 11, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing codex/erm-scope-bindings (8df56e4) with main (afc0a83)

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.

@camc314 camc314 changed the title fix(transformer): align ERM lexical binding scopes fix(transformer/explicit-resource-management): align lexical binding scopes May 11, 2026
@camc314 camc314 marked this pull request as ready for review May 11, 2026 11:06
Copilot AI review requested due to automatic review settings May 11, 2026 11:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns semantic scope/binding metadata with the AST shape produced by the ES2026 explicit resource management (using/await using) transform, reducing semantic mismatches in conformance/coverage runs (part of #9699).

Changes:

  • Adjust the explicit_resource_management function-body transform to move block-scoped bindings into the newly inserted try-body scope.
  • Add Scoping::move_bindings_if to support bulk moving bindings between scopes while updating symbol scope ownership.
  • Update conformance/coverage snapshots reflecting improved pass counts and fewer scope/binding mismatches for the transform.

Reviewed changes

Copilot reviewed 4 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/oxc_transformer/src/es2026/explicit_resource_management.rs Moves relevant bindings from the function-body scope into the generated try block scope to match the transformed AST.
crates/oxc_semantic/src/scoping.rs Introduces move_bindings_if to move bindings between scopes and update symbol scope IDs.
tasks/transform_conformance/snapshots/oxc.snap.md Snapshot updated for improved explicit-resource-management conformance results.
tasks/transform_conformance/snapshots/babel.snap.md Snapshot updated for improved explicit-resource-management conformance results.
tasks/coverage/snapshots/semantic_typescript.snap Snapshot updated showing fewer semantic mismatches / improved pass count.
tasks/coverage/snapshots/semantic_test262.snap Snapshot updated showing fewer semantic mismatches / improved pass count.
tasks/coverage/snapshots/semantic_babel.snap Snapshot updated showing fewer semantic mismatches / improved pass count.

Comment thread crates/oxc_transformer/src/es2026/explicit_resource_management.rs Outdated
Comment thread crates/oxc_semantic/src/scoping.rs Outdated
@camc314 camc314 assigned overlookmotel and Dunqing and unassigned camc314 May 11, 2026
@Dunqing Dunqing changed the base branch from main to graphite-base/22320 May 14, 2026 07:58
@Dunqing Dunqing force-pushed the codex/erm-scope-bindings branch from 8df56e4 to 4bd4297 Compare May 14, 2026 07:58
@Dunqing Dunqing changed the base branch from graphite-base/22320 to refactor/semantic-move-binding-by-symbol-id May 14, 2026 07:58

Dunqing commented May 14, 2026

Copy link
Copy Markdown
Member

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 14, 2026

Dunqing commented May 14, 2026

Copy link
Copy Markdown
Member

Merge activity

@graphite-app graphite-app Bot changed the base branch from refactor/semantic-move-binding-by-symbol-id to graphite-base/22320 May 14, 2026 09:08
graphite-app Bot pushed a commit that referenced this pull request May 14, 2026
## Summary

Adds `Scoping::move_binding_by_symbol_id(from, to, symbol_id)` that bundles `move_binding` + `set_symbol_scope_id`, looking up the symbol's name internally.

Every existing call to `move_binding` is paired with `set_symbol_scope_id` to keep `symbol_scope_ids` consistent with the binding map. Folding them into one call also lets callers move a binding without holding an arena-lifetime `Ident` across the `&mut Scoping` access — useful when the only name source is `Scoping`'s own binding map (the next step in the ERM scope-binding fix in #22320).

The 6 existing pair sites are migrated:

- `oxc_semantic/src/binder.rs` (var-hoist Annex B)
- `oxc_transformer/src/common/arrow_function_converter.rs` (`adjust_binding_scope`)
- `oxc_transformer/src/es2017/async_to_generator.rs` (`BindingMover`)
- `oxc_transformer/src/es2018/object_rest_spread.rs` (for-init bindings)
- `oxc_transformer/src/es2026/explicit_resource_management.rs` x3 (for-of init, static block, try statement)

## Notes

- The helper updates `symbol_scope_ids` unconditionally — matching the pre-existing pair behavior, which always set the scope id even when `move_binding` was a silent no-op.
- The internal `remove_entry(&name)` lookup uses `Ident`'s precomputed hash via `IdentBuildHasher::write_u64`, skipping the byte-by-byte hash path that `name.as_str()` would take.
- No new tests: each migrated call site is exercised by transformer Babel/conformance and `semantic_*` coverage; snapshots are unchanged.
@graphite-app graphite-app Bot force-pushed the graphite-base/22320 branch from 16a055b to fb4d98b Compare May 14, 2026 09:13
@graphite-app graphite-app Bot force-pushed the codex/erm-scope-bindings branch from 4bd4297 to e7ab200 Compare May 14, 2026 09:13
@graphite-app graphite-app Bot changed the base branch from graphite-base/22320 to main May 14, 2026 09:14
@graphite-app graphite-app Bot force-pushed the codex/erm-scope-bindings branch from e7ab200 to 79822b3 Compare May 14, 2026 09:14
@graphite-app graphite-app Bot force-pushed the codex/erm-scope-bindings branch from 79822b3 to dda33de Compare May 14, 2026 09:18
@graphite-app graphite-app Bot merged commit dda33de into main May 14, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 14, 2026
@graphite-app graphite-app Bot deleted the codex/erm-scope-bindings branch May 14, 2026 09:22
overlookmotel added a commit that referenced this pull request May 15, 2026
### 🚀 Features

- bc91a17 codegen: Expose `Codegen::with_source_type` method (#22432)
(camc314)

### 🐛 Bug Fixes

- 5ac7e79 minifier: Drop unused-var-init pure IIFEs and preserve
annotation for downstream (#22349) (Dunqing)
- 4ab57eb allocator: Fixed-size allocators use `VirtualAlloc` on Windows
(#22124) (overlookmotel)
- 66d77eb allocator: Fix segfault on Linux MUSL with fixed-size
allocators (#22388) (overlookmotel)
- b8fbc1f transformer/object-rest-spread: Correct scope id when moving
bindings (#22419) (camc314)
- 18edc2c codegen: Keep `Object.defineProperty` property name as plain
string in minify (#22400) (Dunqing)
- dda33de transformer/explicit-resource-management: Align lexical
binding scopes (#22320) (camc314)
- 8e79de8 transformer: Preserve for-await statement bodies (#22361)
(camc314)
- 0cba210 transformer/class: Replace `new.target` in static blocks
(#22360) (camc314)
- 67ab1c9 transformer/es2018/for-await: Hoist for-await generated
bindings (#22355) (camc314)
- c3ceb4a transformer/object-rest-spread: Use hoisted scope for `for-of`
temp refs (#22347) (camc314)

### ⚡ Performance

- 73a9043 allocator/bitset: Avoid temp heap `String` allocation (#22403)
(camc314)
- 8b2f4f9 transformer/object-rest-spread: Collect `Vec<SymbolId` over
`Vec<BindingIdentifier>` (#22418) (camc314)
- 83679ea parser: Split TriviaBuilder::handle_token hot/cold paths
(#22415) (Boshen)
- 2c7d781 codegen: Inline identifier-name accessors (#22411) (Boshen)
- 618bc76 diagnostics: Inline `OxcDiagnosticInner` to avoid heap
allocation (#22406) (Boshen)
- 0b4e158 parser: Reserve cap `2` for sequence expressions vec (#22374)
(camc314)
- 5f3bdd0 codegen: Add `#[inline]` to `code`, `code_len` (#22373)
(camc314)

Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
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.

4 participants