fix(codegen): preserve legal comments orphaned by upstream passes by Dunqing · Pull Request #21575 · oxc-project/oxc

Dunqing · 2026-04-20T14:03:41Z

Closes #19750.

Problem

Legal comments (//!, /*!, @license, @preserve) anchor to AST nodes via attached_to. When DCE removes the node, codegen's Inline mode silently drops the comment.

Fix

Codegen-side. Two helpers in oxc_codegen:

print_legal_orphans_before(end) — drains pending legal-comment orphans and emits them at the current cursor in source order.
has_legal_orphans_before(end) — used by block emitters to keep an empty body multi-line so it can host the flush.

Wired into every statement-list site (Program / FunctionBody / BlockStatement / StaticBlock / SwitchCase / TSModuleBlock) via a single print_stmts_with_orphan_flush helper. Orphans land at the next surviving anchor in the same scope, preserving lexical order.

A sorted side index legal_comment_keys: Vec<u32> keeps the per-call cost at O(log K) (K = legal-comment count) rather than O(M) over the full comments map. This avoids the 19% codegen regression on react.development.js an earlier iteration of this PR introduced.

Behaviour

Statement-level orphans match esbuild's inline mode: anchor surviving → original spot; anchor removed with sibling → next sibling in scope; anchor removed with no sibling → enclosing block (or program scope-end as last resort). Order against surviving legal comments preserved. Never silently dropped.

Out of scope: comments anchored to non-statement nodes (array elements, object properties, class members, expression positions). They still survive via the nearest enclosing statement-scope flush but won't always land at their exact original position. Closing those gaps would need an AST-level change (legal comments as first-class statements).

Tests

preserve_legal_comment_when_dce_removes_anchor covers all four legal styles, no-DCE no-op, multi-orphan order, order against surviving legals, all-removed scope-end fallback, BlockStatement / FunctionBody / SwitchCase scopes, and the single-consequent SwitchCase regression flagged by Codex review.

All minifier + codegen tests pass. CodSpeed at baseline.

Dunqing · 2026-04-20T14:03:57Z

fix(codegen): preserve legal comments orphaned by upstream passes #21575 👈 (View in Graphite)
main

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.

codspeed-hq · 2026-04-20T14:09:03Z

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks¹

_{Comparing fix/minifier-preserve-legal-comments-on-inline (722ff71) with main (aaabde4)}

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 · 2026-04-29T05:03:11Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b3c98cabe

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Dunqing · 2026-04-29T15:28:35Z

Merge activity

Apr 29, 3:28 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 30, 12:40 AM UTC: Dunqing added this pull request to the Graphite merge queue.
Apr 30, 12:49 AM UTC: Merged by the Graphite merge queue.

@license

After the refactor of oxc-project#21575's omnibus `preserve_legal_comment_when_dce_removes_anchor`, two of the additional tests in this branch became mechanism-redundant: * `preserve_legal_comment_when_dce_removes_one_of_multiple_declarators` exercised the multi-declarator AST-mutation path, but the codegen-side orphan flush is identical to the single-declarator case the parent already pins. * `preserve_legal_comments_when_dce_removes_anchors_at_multiple_levels` asserted the inner orphan stays scoped when both inner and outer anchors are removed. The parent already pins the per-scope flush keeping the inner orphan inside the function with one orphan; adding an outer orphan exercises the same mechanism. The remaining three tests (`/* @license */` standalone, and the two `"use strict"`-removal tests for the legal-comment subset of oxc-project#19748) each cover behaviour the parent omnibus doesn't reach.

@license

…21951) What --- Three tests on top of #21575: In `dead_code_elimination.rs`: - `/* @license */` standalone — the only legal-comment marker the omnibus `preserve_legal_comment_when_dce_removes_anchor` doesn't pin. In `normalize.rs`, covering the legal-comment subset of #19748 that #21575 also closes: - Banner above a removed program-level `"use strict"`. - Inner-function `"use strict"` removal preserving its inner legal comment. The placement quirk for the legal-comment subset of #19748 is pinned separately in #21943.

@license

…21951) What --- Three tests on top of #21575: In `dead_code_elimination.rs`: - `/* @license */` standalone — the only legal-comment marker the omnibus `preserve_legal_comment_when_dce_removes_anchor` doesn't pin. In `normalize.rs`, covering the legal-comment subset of #19748 that #21575 also closes: - Banner above a removed program-level `"use strict"`. - Inner-function `"use strict"` removal preserving its inner legal comment. The placement quirk for the legal-comment subset of #19748 is pinned separately in #21943.

…1575) Closes #19750. ## Problem Legal comments (`//!`, `/*!`, `@license`, `@preserve`) anchor to AST nodes via `attached_to`. When DCE removes the node, codegen's `Inline` mode silently drops the comment. ## Fix Codegen-side. Two helpers in `oxc_codegen`: - `print_legal_orphans_before(end)` — drains pending legal-comment orphans and emits them at the current cursor in source order. - `has_legal_orphans_before(end)` — used by block emitters to keep an empty body multi-line so it can host the flush. Wired into every statement-list site (Program / FunctionBody / BlockStatement / StaticBlock / SwitchCase / TSModuleBlock) via a single `print_stmts_with_orphan_flush` helper. Orphans land at the next surviving anchor in the same scope, preserving lexical order. A sorted side index `legal_comment_keys: Vec<u32>` keeps the per-call cost at `O(log K)` (K = legal-comment count) rather than `O(M)` over the full comments map. This avoids the 19% codegen regression on `react.development.js` an earlier iteration of this PR introduced. ## Behaviour Statement-level orphans match esbuild's `inline` mode: anchor surviving → original spot; anchor removed with sibling → next sibling in scope; anchor removed with no sibling → enclosing block (or program scope-end as last resort). Order against surviving legal comments preserved. Never silently dropped. Out of scope: comments anchored to non-statement nodes (array elements, object properties, class members, expression positions). They still survive via the nearest enclosing statement-scope flush but won't always land at their exact original position. Closing those gaps would need an AST-level change (legal comments as first-class statements). ## Tests `preserve_legal_comment_when_dce_removes_anchor` covers all four legal styles, no-DCE no-op, multi-orphan order, order against surviving legals, all-removed scope-end fallback, BlockStatement / FunctionBody / SwitchCase scopes, and the single-consequent SwitchCase regression flagged by Codex review. All minifier + codegen tests pass. CodSpeed at baseline.

### 💥 BREAKING CHANGES - 0ffbe0d allocator: [**BREAKING**] Remove `Allocator::end_ptr` method (#21871) (overlookmotel) ### 🚀 Features - 9593ec8 transformer/jsx: Add jsxDEV source metadata for fragments (#21932) (Ido Rosenthal) ### 🐛 Bug Fixes - 429deac napi/parser: Export `visitorKeys` from `wasm` entrypoint (#21996) (NullVoxPopuli) - e852911 codegen: Preserve legal comments orphaned by upstream passes (#21575) (Dunqing) - e3399ec transformer/class-properties: Preserve RHS in logical-assignment to static private field (#21950) (Dunqing) - c59c199 transformer/typescript: Emit class fields for parameter properties (#21831) (Dunqing) - aaabde4 parser: Attach legal comments to following token (#21670) (Dunqing) ### ⚡ Performance - 0bf0cb9 allocator: Per-platform `Arena::new_fixed_size` implementations (#22088) (overlookmotel) ### 📚 Documentation - 62ec410 allocator: Correct doc comment for `Allocator::from_raw_parts` (#22093) (overlookmotel) - 3e152c6 allocator: Correct typos in comments (#22092) (overlookmotel) - e220855 allocator: Correct doc comment for `Allocator::set_cursor_ptr` (#21866) (overlookmotel) --------- Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com> Co-authored-by: Cameron Clark <cameron.clark@hey.com>

Dunqing force-pushed the fix/minifier-preserve-legal-comments-on-inline branch 4 times, most recently from d2b85d3 to bd8e456 Compare April 27, 2026 05:39

Dunqing changed the title ~~fix(minifier): preserve legal comments when inlining removes a statement~~ fix(codegen): preserve legal comments orphaned by upstream passes Apr 28, 2026

Dunqing force-pushed the fix/minifier-preserve-legal-comments-on-inline branch 4 times, most recently from 288398f to 6b3c98c Compare April 29, 2026 02:58

Dunqing force-pushed the fix/minifier-preserve-legal-comments-on-inline branch from 6b3c98c to 6d697da Compare April 29, 2026 05:07

chatgpt-codex-connector Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/oxc_codegen/src/gen.rs Outdated

gthb mentioned this pull request Apr 29, 2026

test(minifier): pin remaining banner placement quirk for #19748 #21943

Closed

Dunqing force-pushed the fix/minifier-preserve-legal-comments-on-inline branch 8 times, most recently from 670a1d5 to 722ff71 Compare April 29, 2026 15:13

Dunqing marked this pull request as ready for review April 29, 2026 15:14

Dunqing added the 0-merge Merge with Graphite Merge Queue label Apr 29, 2026

gthb mentioned this pull request Apr 29, 2026

test(minifier): extend legal-comment coverage for #19750 and #19748 #21951

Merged

Dunqing force-pushed the fix/minifier-preserve-legal-comments-on-inline branch from 5f0dcb8 to 392c909 Compare April 30, 2026 00:36

graphite-app Bot force-pushed the fix/minifier-preserve-legal-comments-on-inline branch from 392c909 to e852911 Compare April 30, 2026 00:44

graphite-app Bot merged commit e852911 into main Apr 30, 2026
27 checks passed

graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 30, 2026

graphite-app Bot deleted the fix/minifier-preserve-legal-comments-on-inline branch April 30, 2026 00:49

oxc-guard Bot mentioned this pull request May 4, 2026

release(crates): oxc v0.129.0 #22108

Merged

This was referenced May 7, 2026

fix(codegen): emit call end mapping at ) position, not past it #22199

Merged

[Bug] Legal comments are not respected when adjacent statements are eliminated rolldown/rolldown#7257

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(codegen): preserve legal comments orphaned by upstream passes#21575

fix(codegen): preserve legal comments orphaned by upstream passes#21575
graphite-app[bot] merged 1 commit intomainfrom
fix/minifier-preserve-legal-comments-on-inline

Dunqing commented Apr 20, 2026 •

edited

Loading

Uh oh!

Dunqing commented Apr 20, 2026

Uh oh!

codspeed-hq Bot commented Apr 20, 2026 •

edited

Loading

Uh oh!

Dunqing commented Apr 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Dunqing commented Apr 29, 2026 •

edited by graphite-app Bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Dunqing commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Behaviour

Tests

Uh oh!

Dunqing commented Apr 20, 2026

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq Bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Dunqing commented Apr 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Dunqing commented Apr 29, 2026 • edited by graphite-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Dunqing commented Apr 20, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 20, 2026 •

edited

Loading

Dunqing commented Apr 29, 2026 •

edited by graphite-app Bot

Loading