Skip to content

feat(minifier): invert early-return guards with statement bodies#24111

Draft
Dunqing wants to merge 1 commit into
mainfrom
minifier-guard-inversion
Draft

feat(minifier): invert early-return guards with statement bodies#24111
Dunqing wants to merge 1 commit into
mainfrom
minifier-guard-inversion

Conversation

@Dunqing

@Dunqing Dunqing commented Jul 3, 2026

Copy link
Copy Markdown
Member

What

Flatten an early-return guard that has statements in its body:

if (c) { X; return; }
REST

becomes if (c) { X } else { REST }, and the existing passes then fold it further (into a ternary when both sides are expressions). We already did this when the guard body was a single bare return; now it also works with statements before the return.

How

A new arm in handle_if_statement (minimize_statements.rs): when a function-body if has a block ending in a bare return, drop that return and move the trailing statements into the else. The rest is done by passes we already have (try_minimize_if, sequence merging, block flattening). Bails when the moved statements declare let/const/class/function (same scope guard as the existing path). The fold runs in-pass: the drained tail is re-minimized immediately, so a chain of guards collapses in one compress iteration instead of one iteration per guard (this is what fixed the earlier CodSpeed regression — App.tsx is back to 2 iterations, same as main). Recursion depth is capped at 100 so a generated file with an extreme flat guard chain cannot overflow worker-thread stacks; past the cap the rest is left less folded but correct. Gated behind the sequences option, which DCE mode turns off, so tree-shaking output does not change.

This also implements the old ignored test test_function_return_optimization2 ("TODO: Function return optimization not yet implemented") and un-ignores it. Its expectations came from closure-compiler and were stale; they were rewritten to oxc's actual output and each one was checked against the source for equivalence.

Example

// input
function f(x) {
  if (x) {
    foo();
    return;
  }
  bar();
  baz();
}
// before
function f(e){if(e){foo();return}bar(),baz()}

// after
function f(e){e?foo():(bar(),baz())}

Found by re-minifying oxc's minsize output with terser -c: echarts.js has 105 of these guards. All minsize fixtures stay equal or smaller.

Implemented with Claude Code, reviewed and verified by me.

@github-actions github-actions Bot added the A-minifier Area - Minifier label Jul 3, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 52 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing minifier-guard-inversion (315e7f1) with main (b521b9b)

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.

@Dunqing
Dunqing changed the base branch from main to graphite-base/24111 July 3, 2026 09:20
@Dunqing
Dunqing force-pushed the minifier-guard-inversion branch from 5fd9286 to abad106 Compare July 3, 2026 09:20
@Dunqing
Dunqing changed the base branch from graphite-base/24111 to minifier-ternary-bool-fold July 3, 2026 09:20

Dunqing commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

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 force-pushed the minifier-guard-inversion branch from abad106 to caca822 Compare July 3, 2026 09:56
@Dunqing
Dunqing force-pushed the minifier-guard-inversion branch from caca822 to a77d6c4 Compare July 6, 2026 07:36
@Dunqing
Dunqing force-pushed the minifier-ternary-bool-fold branch from 3260236 to cf589f0 Compare July 6, 2026 07:36
@graphite-app
graphite-app Bot changed the base branch from minifier-ternary-bool-fold to graphite-base/24111 July 6, 2026 08:26
@graphite-app
graphite-app Bot force-pushed the graphite-base/24111 branch from cf589f0 to 2d9b0b3 Compare July 6, 2026 08:30
@graphite-app
graphite-app Bot force-pushed the minifier-guard-inversion branch from a77d6c4 to 9e679b3 Compare July 6, 2026 08:30
@graphite-app
graphite-app Bot changed the base branch from graphite-base/24111 to main July 6, 2026 08:31
@graphite-app
graphite-app Bot force-pushed the minifier-guard-inversion branch from 9e679b3 to f39d74e Compare July 6, 2026 08:31
`optimize_implicit_jump` already flips a tail guard whose body is a single
bare `return`, but not one whose body is a block ending in `return`, since
`is_jump_statement` only unwraps single-statement blocks. Extend the
function-body-tail case to also accept `if (c) { X; return } REST`, rewriting
it to `if (c) { X } else { REST }`: the bare `return` makes REST reachable
only when `c` is false, so it moves into a new `else`.

The rewrite keeps the test polarity (a non-empty consequent gives a real
`else` to fill). Like the jump path, it re-minimizes the `else` body and re-runs
`try_minimize_if` inline, so both branches collapse into a conditional/logical
expression in the same pass once they reduce to expression statements
(`if (x) { foo(); return } bar()` => `x ? foo() : bar()`). Because the
re-minimize itself contains this arm, a whole chain of guards folds
innermost-first in a single pass — `if(a){x();return} if(b){y();return} z()`
becomes `a ? x() : b ? y() : z()` in one iteration rather than one guard per
iteration, which avoids an extra global fixed-point pass and the throwaway
intermediate block a deferred fold would allocate.

That single-pass fold recurses once per guard, and a flat guard chain is not
bounded by parse nesting, so `GUARD_INVERSION_MAX_DEPTH` caps the recursion
(threaded through `minimize_statements`); past it the arm falls back to the
deferred wrap so a pathologically long generated chain can't overflow the
stack. The cap is far above any real chain, so real fixtures fold identically.

Gated behind `options().sequences` like the jump path it extends, so DCE mode
(sequences off) is unaffected. REST and any existing alternate move into a
block, so the same `statement_cares_about_scope` guard bails when they declare
bindings the test could observe (a hoisted `function` the test calls, a
surviving lexical binding). Single-statement branches skip the wrapping block.
All minsize fixtures stay equal-or-smaller.
@Dunqing
Dunqing force-pushed the minifier-guard-inversion branch from f39d74e to 315e7f1 Compare July 7, 2026 14:58
@Dunqing

Dunqing commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

@armano2 Since you are working on statement-related tasks, I am wondering whether you are trying to work on this or if it has been on your mind. If yes, I will close this PR and leave it as is; if not, I will continue this since the output notably decreased.

test("function f(){if(a()){b();if(c())return;}}", "function f(){if(a()){b();if(c());}}");
test("function f(){if(x)return; x=3; return; }", "function f(){if(x); else x=3}");
test("function f(){if(true){a();return;}else;b();}", "function f(){if(true){a();}else{b();}}");
test("function f(){if(a()){b();if(c())return;}}", "function f(){a()&&(b(),c())}");

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.

can you merge this with main? most of those test changes are not relevant 🐱

&& matches!(&if_stmt.consequent, Statement::BlockStatement(block)
if block.body.last().is_some_and(|s|
matches!(s, Statement::ReturnStatement(r) if r.argument.is_none())))
{

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.

i do not like that we assume return here, this should also work for continue if parent is loop

@Dunqing

Dunqing commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

Thank you for reviewing. This PR is outdated; I will rewrite it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants