fix(parser): set pure comment index after dedup check to handle lookahead/rewind#21570
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
Merging this PR will degrade performance by 3.5%
Performance Changes
Comparing Footnotes
|
95af6f4 to
2a0b901
Compare
|
Confirmed that this fix is correct, and I tested it locally in monitor-oxc, and it worked. So I am merging it without review. |
Merge activity
|
…head/rewind (#21570) Fix some idempotency tests that failed in https://github.com/oxc-project/monitor-oxc/actions/runs/24652862465/job/72079552802 ## Summary When `/* @__PURE__ */` appears before an object literal (e.g. `export const X = /* @__PURE__ */ { a: 1 }`), the parser's arrow-function lookahead causes the comment to be added to the `comments` vec during speculative parsing. After rewind, the same comment is encountered again — the dedup check skips the push, but `parse_annotation` had already set `pure_comment` to `comments.len()`, which now points to a non-existent index. This causes `mark_pure_comment_not_applied` to silently fail, breaking codegen idempotency. ### Fix - Move `pure_comment` / `has_no_side_effects_comment` assignment from `parse_annotation` to `add_comment`, **after** the dedup check - In the dedup branch, set the index to the existing comment (`len - 1`) - In the normal branch, set the index to where the comment will be pushed (`len`) - Extract shared logic into `set_annotation_flags` helper ## Test plan - Added parser test: `/* @__PURE__ */` before object literal is correctly marked `PureNotApplied` - Added parser test: `/* @__PURE__ */` before call expression remains `Pure` after lookahead - Added codegen idempotency test: parse→codegen→parse→codegen produces identical output - `cargo test -p oxc_parser --lib -- trivia_builder` — 9 tests pass - `cargo test -p oxc_codegen --test integration -- comments` — all pass
2a0b901 to
67d40f8
Compare

Fix some idempotency tests that failed in https://github.com/oxc-project/monitor-oxc/actions/runs/24652862465/job/72079552802
Summary
When
/* @__PURE__ */appears before an object literal (e.g.export const X = /* @__PURE__ */ { a: 1 }), the parser's arrow-function lookahead causes the comment to be added to thecommentsvec during speculative parsing. After rewind, the same comment is encountered again — the dedup check skips the push, butparse_annotationhad already setpure_commenttocomments.len(), which now points to a non-existent index. This causesmark_pure_comment_not_appliedto silently fail, breaking codegen idempotency.Fix
pure_comment/has_no_side_effects_commentassignment fromparse_annotationtoadd_comment, after the dedup checklen - 1)len)set_annotation_flagshelperTest plan
/* @__PURE__ */before object literal is correctly markedPureNotApplied/* @__PURE__ */before call expression remainsPureafter lookaheadcargo test -p oxc_parser --lib -- trivia_builder— 9 tests passcargo test -p oxc_codegen --test integration -- comments— all pass