Skip to content

fix(transformer/es2018/for-await): hoist for-await generated bindings#22355

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/coverage-4ef2e1
May 12, 2026
Merged

fix(transformer/es2018/for-await): hoist for-await generated bindings#22355
graphite-app[bot] merged 1 commit into
mainfrom
codex/coverage-4ef2e1

Conversation

@camc314

@camc314 camc314 commented May 12, 2026

Copy link
Copy Markdown
Contributor

for await lowering emits several temporary bindings as var, but the transformer was registering those generated symbols in the current block scope instead of the nearest hoist scope.

For example, this input:

async function* f(iter) {
  {
    for await (const value of iter) {
      value;
    }
  }
}

is lowered into code shaped like this:

async function* f(iter) {
  {
    var _iteratorAbruptCompletion = false;
    var _didIteratorError = false;
    var _iteratorError;

    try {
      for (
        var _iterator = _asyncIterator(iter), _step;
        //  ^^^^^^^^^                         ^^^^^
        //  These are `var` declarations, so semantically they belong to
        //  the function/hoist scope, not the inner block scope.
        _iteratorAbruptCompletion = !(_step = await _iterator.next()).done;
        _iteratorAbruptCompletion = false
      ) {
        const value = _step.value;
        value;
      }
    } catch (err) {
      _didIteratorError = true;
      _iteratorError = err;
    }
  }
}

Before this PR, the AST transform generated the right JavaScript, but its semantic metadata recorded _iterator, _step, _didIteratorError, _iteratorError, and _iteratorAbruptCompletion in the current block scope:

That made the transformed semantic data disagree with semantic data rebuilt from the emitted AST.

Copilot AI review requested due to automatic review settings May 12, 2026 14:18
@github-actions github-actions Bot added the A-transformer Area - Transformer / Transpiler label May 12, 2026

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

Fixes incorrect scoping of generated var bindings produced by the for await (...) transform so that they are hoisted into the correct “var/hoist” scope (function or program), eliminating semantic binding/symbol-scope mismatches in coverage runs.

Changes:

  • Generate the internal step binding in ctx.current_hoist_scope_id() instead of the current (potentially block) scope.
  • Generate the internal iterator bookkeeping bindings (_iterator, _step, _didIteratorError, etc.) in ctx.current_hoist_scope_id() to match var hoisting semantics.
  • Update semantic coverage snapshots to reflect the resolved mismatches (improved “Positive Passed” counts).

Reviewed changes

Copilot reviewed 1 out of 3 changed files in this pull request and generated no comments.

File Description
crates/oxc_transformer/src/es2018/async_generator_functions/for_await.rs Hoists for-await transform-generated var bindings into the correct hoist scope to match JS var semantics.
tasks/coverage/snapshots/semantic_test262.snap Snapshot update showing removal of prior for-await binding/scope mismatches and improved pass count.
tasks/coverage/snapshots/semantic_babel.snap Snapshot update showing removal of a prior for-await-related binding/scope mismatch and improved pass count.

@camc314 camc314 changed the title fix(transformer/transform): hoist for-await generated bindings fix(transformer/es2018/for-await): hoist for-await generated bindings May 12, 2026
@codspeed-hq

codspeed-hq Bot commented May 12, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing codex/coverage-4ef2e1 (38e1470) with main (41fcdcf)

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.

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 12, 2026

camc314 commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

…#22355)

`for await` lowering emits several temporary bindings as `var`, but the transformer was registering those generated symbols in the current block scope instead of the nearest hoist scope.

For example, this input:

```js
async function* f(iter) {
  {
    for await (const value of iter) {
      value;
    }
  }
}
```

is lowered into code shaped like this:

```js
async function* f(iter) {
  {
    var _iteratorAbruptCompletion = false;
    var _didIteratorError = false;
    var _iteratorError;

    try {
      for (
        var _iterator = _asyncIterator(iter), _step;
        //  ^^^^^^^^^                         ^^^^^
        //  These are `var` declarations, so semantically they belong to
        //  the function/hoist scope, not the inner block scope.
        _iteratorAbruptCompletion = !(_step = await _iterator.next()).done;
        _iteratorAbruptCompletion = false
      ) {
        const value = _step.value;
        value;
      }
    } catch (err) {
      _didIteratorError = true;
      _iteratorError = err;
    }
  }
}
```

Before this PR, the AST transform generated the right JavaScript, but its semantic metadata recorded `_iterator`, `_step`, `_didIteratorError`, `_iteratorError`, and `_iteratorAbruptCompletion` in the current block scope:

That made the transformed semantic data disagree with semantic data rebuilt from the emitted AST.
@graphite-app graphite-app Bot force-pushed the codex/coverage-4ef2e1 branch from 38e1470 to 67ab1c9 Compare May 12, 2026 17:37
@graphite-app graphite-app Bot merged commit 67ab1c9 into main May 12, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 12, 2026
@graphite-app graphite-app Bot deleted the codex/coverage-4ef2e1 branch May 12, 2026 17:41
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-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants