Skip to content

fix(codegen): preserve in restriction through yield arguments - #26413

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/codegen-yield-forbid-in
Sep 7, 2026
Merged

graphite-app[bot] merged 1 commit into
mainfrom
codex/codegen-yield-forbid-in

Conversation

@camc314

@camc314 camc314 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Problem

A yield expression in a classic for loop initializer can lose the parentheses required around an in operator in its argument. This turns valid JavaScript into output that cannot be parsed, in both default and minified code generation.

For example, this input is valid:

function *g(o) {
  for (yield (1 in o); false;);
}

Previously, codegen emitted:

// Default output
function* g(o) {
  for (yield 1 in o; false;);
}

// Minified output
function*g(o){for(yield 1 in o;false;);}

The unparenthesized in is not allowed at this position in a classic for initializer. The same restriction applies to the argument of yield*, and can also be lost through nested yields or an assignment inside the yield argument.

Change

ForStatement already prints its initializer with Context::FORBID_IN. However, YieldExpression::gen_expr ignored its incoming context and always printed its argument with Context::empty().

Preserve FORBID_IN when printing the argument of an unwrapped yield expression. This lets the existing expression printers retain the required grouping:

// Default output
function* g(o) {
  for (yield (1 in o); false;);
}

// Minified output
function*g(o){for(yield(1 in o);false;);}

Only the relevant FORBID_IN flag is forwarded; the argument's existing precedence remains Precedence::Yield. Both yield and yield* use this path.

Parentheses around yield

When precedence requires parentheses around the entire yield expression, those parentheses already allow in in the argument. Clear the restriction in that case to avoid redundant inner grouping:

// Input
function *g(o) {
  for ((yield (1 in o)) + 1; false;);
}

// Default output
function* g(o) {
  for ((yield 1 in o) + 1; false;);
}

// Minified output
function*g(o){for((yield 1 in o)+1;false;);}

Outside a restricted initializer, ordinary yield expressions continue to omit unnecessary parentheses: yield (1 in o) prints as yield 1 in o.

Copilot AI lite review requested due to automatic review settings September 7, 2026 16:07
@camc314
camc314 requested a review from Dunqing as a code owner September 7, 2026 16:07
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T16:13:29.665476Z 7965dc7 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the A-codegen Area - Code Generation label Sep 7, 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.

🟢 Approval recommended

The change is narrowly scoped, matches the stated parsing constraint, and is backed by targeted integration tests for both default and minified codegen.

Pull request overview

Fixes a JavaScript codegen correctness bug where yield / yield* expressions inside classic for initializer clauses could drop required parentheses around an in operator in the yield argument, producing invalid output.

Changes:

  • Preserve Context::FORBID_IN when printing YieldExpression arguments (unless the yield itself is wrapped in parentheses by precedence).
  • Add integration coverage for yield / yield* + in interactions in classic for initializers, including nested-yield and assignment-in-argument cases.
File summaries
File Description
crates/oxc_codegen/src/gen.rs Forwards FORBID_IN into yield/yield* argument printing and clears it when the yield expression is wrapped, preventing invalid for init output and avoiding redundant parens.
crates/oxc_codegen/tests/integration/js.rs Adds integration tests covering in inside yield arguments in classic for initializers for both default and minified output, plus idempotency checks.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@codspeed

codspeed Bot commented Sep 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing codex/codegen-yield-forbid-in (0faa2f8) with main (10521b2)2

Open in CodSpeed

Footnotes

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

Comment thread crates/oxc_codegen/src/gen.rs Outdated
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Sep 7, 2026

camc314 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

## Problem

A `yield` expression in a classic `for` loop initializer can lose the parentheses required around an `in` operator in its argument. This turns valid JavaScript into output that cannot be parsed, in both default and minified code generation.

For example, this input is valid:

```js
function *g(o) {
  for (yield (1 in o); false;);
}
```

Previously, codegen emitted:

```js
// Default output
function* g(o) {
  for (yield 1 in o; false;);
}

// Minified output
function*g(o){for(yield 1 in o;false;);}
```

The unparenthesized `in` is not allowed at this position in a classic `for` initializer. The same restriction applies to the argument of `yield*`, and can also be lost through nested yields or an assignment inside the yield argument.

## Change

`ForStatement` already prints its initializer with `Context::FORBID_IN`. However, `YieldExpression::gen_expr` ignored its incoming context and always printed its argument with `Context::empty()`.

Preserve `FORBID_IN` when printing the argument of an unwrapped yield expression. This lets the existing expression printers retain the required grouping:

```js
// Default output
function* g(o) {
  for (yield (1 in o); false;);
}

// Minified output
function*g(o){for(yield(1 in o);false;);}
```

Only the relevant `FORBID_IN` flag is forwarded; the argument's existing precedence remains `Precedence::Yield`. Both `yield` and `yield*` use this path.

## Parentheses around yield

When precedence requires parentheses around the entire yield expression, those parentheses already allow `in` in the argument. Clear the restriction in that case to avoid redundant inner grouping:

```js
// Input
function *g(o) {
  for ((yield (1 in o)) + 1; false;);
}

// Default output
function* g(o) {
  for ((yield 1 in o) + 1; false;);
}

// Minified output
function*g(o){for((yield 1 in o)+1;false;);}
```

Outside a restricted initializer, ordinary yield expressions continue to omit unnecessary parentheses: `yield (1 in o)` prints as `yield 1 in o`.
@graphite-app
graphite-app Bot force-pushed the codex/codegen-yield-forbid-in branch from 0faa2f8 to ae6c386 Compare September 7, 2026 17:16
@graphite-app
graphite-app Bot merged commit ae6c386 into main Sep 7, 2026
33 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Sep 7, 2026
@graphite-app
graphite-app Bot deleted the codex/codegen-yield-forbid-in branch September 7, 2026 17:22
graphite-app Bot pushed a commit that referenced this pull request Sep 7, 2026
#26421)

`packages/codegen` can turn valid JavaScript into unparseable output when a `yield` expression appears in a classic `for` loop initializer and its argument contains an `in` operator.

For example, this input is valid:

```js
function* g(o) {
  for (yield (1 in o); false;);
}
```

The JavaScript printer previously dropped the required grouping:

```js
function* g(o) {
  for (yield 1 in o; false;);
}
```

An unparenthesized `in` is not allowed at this position in a classic `for` initializer. The same problem affects `yield*`, nested yields, and assignments inside the yield argument.

The printer now preserves the parentheses:

```js
function* g(o) {
  for (yield (1 in o); false;);
}
```

The `for` printer already marks its initializer with `CTX_FORBID_IN`, but `printYieldExpression` discarded that context and printed its argument with `CTX_NONE`. The yield dispatch now passes its incoming context to `printYieldExpression`, which forwards only `CTX_FORBID_IN` to the argument when the yield expression is unwrapped. The argument retains its existing `PREC_YIELD` precedence.

When precedence requires parentheses around the entire yield expression, those parentheses already permit `in` inside. The argument context is cleared in that case, avoiding redundant inner parentheses:

```js
// Input
function* g(o) {
  for ((yield (1 in o)) + 1; false;);
}

// Output
function* g(o) {
  for ((yield 1 in o) + 1; false;);
}
```

Both `yield` and `yield*` use this path. Outside a restricted initializer, ordinary yields continue to omit unnecessary grouping: `yield (1 in o)` prints as `yield 1 in o`.

The correction applies to both ordinary and source-map-enabled builds and brings the JavaScript printer in line with the Rust fix in #26413.
graphite-app Bot pushed a commit that referenced this pull request Sep 14, 2026
### 🚀 Features

- ca649e0 ecma: Define math constants as known globals and resolve their types (#26585) (Armano)
- 9ef028c codegen: Add `ascii_only` option (#25994) (Samuel Attard)
- 80a76a0 minifier: Negate binary comparison for `typeof x < 'u'` (#26367) (Armano)

### 🐛 Bug Fixes

- 8ca76da parser: Reject `accessor` modifiers on methods (#26617) (camc314)
- 1916f31 parser: Reject `readonly` modifier on constructors (#26612) (camc314)
- 1c42008 parser: Handle escaped let in for loops (#26583) (camc314)
- d21d5cf parser: Recognize annotated empty arrows in conditionals (#26537) (camc314)
- d7713ad parser: Classify Unicode line breaks in block comments (#26536) (camc314)
- 75cd919 transformer: Preserve receivers in private optional chains (#26535) (camc314)
- b32d25d parser: Reject escaped import-phase keywords (#26534) (camc314)
- 47b8311 parser: Recognize contextual binding names in type lookaheads (#26532) (camc314)
- d6b6705 parser: Require arrow separator in TypeScript function types (#26529) (camc314)
- e98beef parser: Disambiguate await using in for initializers (#26527) (camc314)
- 92afee6 parser: Allow parenthesized JSX comma expressions with preserve_parens=false (#26524) (camc314)
- 31508b1 parser: Reject return types on constructor overloads (#26523) (camc314)
- a091fc4 parser: Validate await context for await using declarations (#26495) (camc314)
- 5501e86 parser: Disallow in expressions in using for-loop initializers (#26490) (camc314)
- c8e5fa7 parser: Allow escaped type names in import and export specifiers (#26487) (camc314)
- 2dcee2f parser: Reject async modifiers on class fields (#26486) (camc314)
- 973d58e parser: Require comma after TypeScript this parameter (#26480) (camc314)
- 32d00c5 codegen: Preserve instantiation expression precedence (#26424) (camc314)
- cfa47ab parser: Allow `in` expressions in class static blocks (#26423) (camc314)
- 5986187 packages/codegen: Preserve private-in right operand precedence (#26420) (camc314)
- f8e6c6c packages/codegen: Preserve in restriction through yield arguments (#26421) (camc314)
- d61e3bf parser: Validate TS named tuple rest elements (#26419) (camc314)
- ae6c386 codegen: Preserve in restriction through yield arguments (#26413) (camc314)
- 42ac916 codegen: Preserve private-in right operand precedence (#26411) (camc314)
- 10521b2 parser: Allow escaped type default import bindings (#26409) (camc314)
- 72cb5e3 parser: Reject rest parameters in getters (#26400) (camc314)
- 6e15ad5 packages/codegen: Print matching quoted import names as identifiers (#26404) (camc314)
- bbbb4bc packages/codegen: Preserve private-in left operand precedence (#26403) (camc314)
- a111b5b packages/codegen: Print accessibility modifiers before abstract (#26402) (camc314)
- 4e76602 parser: Allow `in` in arrow block bodies within `for` initializers (#26395) (camc314)
- b20fc19 parser: Reject partially parenthesized mixed coalesce expressions (#26394) (camc314)

### ⚡ Performance

- 1f902a6 isolated_declarations: Key scope maps by `Ident` (#26380) (Dunqing)
- a242469 minfiier: Reduce allocs when creating indirect access (#26601) (Armano)
- 5b4787f minifier: Update chain expressions in place (#26544) (Armano)
- 0bc1661 minifier: Try merging before creating new expression statements (#26556) (Armano)
- c78d707 minifier: Process newly created stmt in handle_if_statement (#26541) (Armano)
- 029c84b minfier: Update expressions in place when substituting alternate syntax (#26460) (Armano)
- d198982 codegen: Outline postfix source mapping work (#26450) (camc314)
- 53f006e ecmascript: Format small integer literals with itoa (#26446) (camc314)
- 8bfb8c0 codegen: Avoid duplicate sourcemap name lookups (#26441) (camc314)

### 📚 Documentation

- 38533ac ast: Move type annotation span comment to span field (#26522) (camc314)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-codegen Area - Code Generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Sponsor
SponsoredKunjungi sekarang
Promo