feat(minifier): invert early-return guards with statement bodies#24111
feat(minifier): invert early-return guards with statement bodies#24111Dunqing wants to merge 1 commit into
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
5fd9286 to
abad106
Compare
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. |
abad106 to
caca822
Compare
caca822 to
a77d6c4
Compare
3260236 to
cf589f0
Compare
cf589f0 to
2d9b0b3
Compare
a77d6c4 to
9e679b3
Compare
9e679b3 to
f39d74e
Compare
`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.
f39d74e to
315e7f1
Compare
|
@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())}"); |
There was a problem hiding this comment.
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()))) | ||
| { |
There was a problem hiding this comment.
i do not like that we assume return here, this should also work for continue if parent is loop
|
Thank you for reviewing. This PR is outdated; I will rewrite it later. |

What
Flatten an early-return guard that has statements in its body:
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 barereturn; now it also works with statements before thereturn.How
A new arm in
handle_if_statement(minimize_statements.rs): when a function-bodyifhas a block ending in a barereturn, drop thatreturnand move the trailing statements into theelse. The rest is done by passes we already have (try_minimize_if, sequence merging, block flattening). Bails when the moved statements declarelet/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 thesequencesoption, 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
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.