fix(minifier): preserve if-chain idempotency and correct side effects when alternate should not be moved (#25041)

do not rewrite if else where `alternate` could produce invalid code and
change behavior

```js
function g(a,b){if(a){}else if(b){return()=>typeof f}else function f(){}}
```

https://github.com/oxc-project/oxc/pull/24997/changes#r3659223987

> **<sub><sub>![P1
Badge](https://img.shields.io/badge/P1-orange?style=flat)</sub></sub>
Preserve Annex B branch scoping when wrapping terminated blocks**
> 
> In sloppy scripts, broadening this condition from direct jumps to any
terminated statement lets a block-returning branch trigger the synthetic
wrapper. For `function g(a,b){if(a){}else if(b){return()=>typeof f}else
function f(){}}`, the subsequent `handle_if_statement` pass detaches the
`else` into that wrapper, emitting the equivalent of
`if(!a){if(b)return()=>typeof f;function f(){}}`. The Annex B function
is then instantiated when the block is entered, so `g(false,true)()`
changes from `"undefined"` to `"function"`; avoid flattening
scope-sensitive alternates or otherwise preserve their original branch
scope.


based on changes from #24997

---------

Co-authored-by: Cameron <cameron.clark@hey.com>
This commit is contained in:
ArmanoandCameron authored and GitHub committed 2026-07-31 10:34:15 +08:00
1 parent e8caf5bb74
commit 6c9ee14ad7
8 files changed
+52 -26

No files matched your search

@@ -127,8 +127,7 @@ impl<'a> PeepholeOptimizations {
/// Wrap to avoid ambiguous else.
/// `if (foo) if (bar) baz else quaz` -> `if (foo) { if (bar) baz else quaz }`
fn wrap_to_avoid_ambiguous_else(if_stmt: &mut IfStatement<'a>, ctx: &mut TraverseCtx<'a>) {
if let Statement::IfStatement(if2) = &mut if_stmt.consequent
&& if2.consequent.is_jump_statement()
if let Statement::IfStatement(if2) = &if_stmt.consequent
&& if2.alternate.is_some()
{
let scope_id = ctx.create_child_scope_of_current(ScopeFlags::empty());
@@ -855,12 +855,9 @@ impl<'a> PeepholeOptimizations {
// if (a(() => b)) return; let b;
// if (a(() => b)) { let b; }
//
let mut can_move_branch_condition_outside_scope = true;
if let Some(alternate) = &if_stmt.alternate
&& Self::statement_cares_about_scope(alternate)
{
can_move_branch_condition_outside_scope = false;
}
let mut can_move_branch_condition_outside_scope =
!if_stmt.alternate.as_ref().is_some_and(Self::statement_cares_about_scope);
if let Some(stmts) = stmts.get(i + 1..) {
for stmt in stmts {
if Self::statement_cares_about_scope(stmt) {
@@ -906,11 +903,14 @@ impl<'a> PeepholeOptimizations {
}
}
if if_stmt.alternate.is_some() && if_stmt.consequent.is_terminated() {
if !if_stmt.alternate.as_ref().is_none_or(Self::statement_cares_about_scope)
&& if_stmt.consequent.is_terminated()
{
// "if (a) return b; else if (c) return d; else return e;" => "if (a) return b; if (c) return d; return e;"
result.push(Statement::IfStatement(if_stmt));
loop {
if let Some(Statement::IfStatement(if_stmt)) = result.last_mut()
&& !if_stmt.alternate.as_ref().is_none_or(Self::statement_cares_about_scope)
&& if_stmt.consequent.is_terminated()
&& let Some(stmt) = if_stmt.alternate.take()
{
@@ -41,9 +41,12 @@ impl<'a> PeepholeOptimizations {
if matches!(first, Statement::VariableDeclaration(decl) if !decl.kind.is_var())
|| matches!(first, Statement::ClassDeclaration(_))
|| matches!(first, Statement::FunctionDeclaration(_))
|| (matches!(first, Statement::IfStatement(decl) if decl.alternate.is_some())
&& matches!(ctx.parent(), Ancestor::IfStatementConsequent(_)))
{
return;
}
let new_stmt = s.body.remove(0);
ctx.replace_statement(stmt, new_stmt);
}
@@ -53,12 +53,15 @@ fn test_fold_one_child_blocks() {
test("if(e1){with(e2){if(e3){foo()}}}else{bar()}", "if(e1)with(e2)e3&&foo();else bar()");
// test("if(a||b){if(c||d){var x;}}", "if(a||b)if(c||d)var x");
test("if(x){ if(y){var x;}else{var z;} }", "if(x)if(y)var x;else var z");
test("if(x){ if(y){var x;}else{var z;} }", "if(x){if(y)var x;else var z}");
// NOTE - technically we can remove the blocks since both the parent
// and child have elses. But we don't since it causes ambiguities in
// some cases where not all descendent ifs having elses
test("if(x){ if(y){var x;}else{var z;} }else{var w}", "if(x)if(y)var x;else var z;else var w");
test(
"if(x){ if(y){var x;}else{var z;} }else{var w}",
"if(x){if(y)var x;else var z;}else var w",
);
test("if (x) {var x;}else { if (y) { var y;} }", "if(x)var x;else if(y)var y");
// Here's some of the ambiguous cases
@@ -49,6 +49,10 @@ fn test_function_return_optimization() {
"function f(){if(a()){if(b()){d();return;}else{return;}}else{return;} c();}",
"function f(){if(a()){if(b()){d();return}return}}",
); // function f(){a()&&b()&&d()}
test(
"function f(a,b,c){if(a){}else if(b){x();return}else if(c){y();return}z()}",
"function f(a,b,c){if(!a){if(b){x();return}if(c){y();return}}z()}",
);
test("function f(){if(a()){b();return;}else;}", "function f(){if(a()){b();return}}"); // function f(){a()&&b()}
test("function f(){if(a()){return;}else{return;} return;}", "function f(){a();}");
test("function f(){if(a()){return;}else{return;} b();}", "function f(){a()}");
@@ -83,6 +87,10 @@ fn test_function_return_optimization() {
"function f(){try{g:if(a()){throw 9;} return;}finally{return}}",
"function f(){try{g:if(a())throw 9; return}finally{return}}",
); // function f(){try{g:if(a())throw 9}finally{}}
test(
"function g(a,b){if(a){}else if(b){return()=>typeof f}else function f(){}}",
"function g(a,b){if(!a){if(b)return()=>typeof f;else function f(){}}}",
);
}
#[test]
@@ -70,4 +70,17 @@ fn test_minimize_if() {
return pos + 1;
}",
);
test(
"function f(){if(a)if(b)var x=1;else var y=2;return x+y}",
"function f(){if(a){if(b)var x=1;else var y=2}return x+y}",
);
test(
"function f(){if(a)if(b)if(c)var x=1;else var y=2;return x+y}",
"function f(){if(a&&b){if(c)var x=1;else var y=2}return x+y}",
);
test(
"function f(){if(!a){}else if(b)var x=1;else var y=2;return x+y}",
"function f(){if(a){if(b)var x=1;else var y=2}return x+y}",
);
}
+7 -7
View File
@@ -5,23 +5,23 @@ Original | minified | minified | gzip | gzip | Iterations | Fi
173.90 kB | 59.34 kB | 59.82 kB | 19.15 kB | 19.33 kB | 2 | moment.js
287.63 kB | 89.19 kB | 90.07 kB | 30.90 kB | 31.95 kB | 2 | jquery.js
287.63 kB | 89.20 kB | 90.07 kB | 30.90 kB | 31.95 kB | 2 | jquery.js
342.15 kB | 116.92 kB | 118.14 kB | 43.17 kB | 44.37 kB | 2 | vue.js
342.15 kB | 116.94 kB | 118.14 kB | 43.18 kB | 44.37 kB | 2 | vue.js
544.10 kB | 70.89 kB | 72.48 kB | 25.71 kB | 26.20 kB | 2 | lodash.js
555.77 kB | 267.19 kB | 270.13 kB | 87.99 kB | 90.80 kB | 2 | d3.js
555.77 kB | 267.20 kB | 270.13 kB | 87.99 kB | 90.80 kB | 2 | d3.js
1.01 MB | 439.20 kB | 458.89 kB | 122.01 kB | 126.71 kB | 2 | bundle.min.js
1.01 MB | 439.21 kB | 458.89 kB | 122.01 kB | 126.71 kB | 2 | bundle.min.js
1.25 MB | 642.41 kB | 646.76 kB | 159.35 kB | 163.73 kB | 2 | three.js
1.25 MB | 642.43 kB | 646.76 kB | 159.35 kB | 163.73 kB | 2 | three.js
2.14 MB | 710.73 kB | 724.14 kB | 160.33 kB | 181.07 kB | 2 | victory.js
2.14 MB | 710.75 kB | 724.14 kB | 160.33 kB | 181.07 kB | 2 | victory.js
3.20 MB | 1.00 MB | 1.01 MB | 322.54 kB | 331.56 kB | 2 | echarts.js
6.69 MB | 2.12 MB | 2.31 MB | 453.79 kB | 488.28 kB | 5 | antd.js
10.95 MB | 3.33 MB | 3.49 MB | 852.63 kB | 915.50 kB | 4 | typescript.js
10.95 MB | 3.33 MB | 3.49 MB | 852.64 kB | 915.50 kB | 4 | typescript.js
@@ -5,9 +5,9 @@ checker.ts:
sys deallocs: 152
sys alloc bytes: 14996600 # 15.00 MB
sys peak growth: 10169560 # 10.17 MB
arena allocs: 148201
arena allocs: 148493
arena reallocs: 28468
arena size: 19202816 # 19.20 MB
arena size: 19208192 # 19.21 MB
App.tsx:
file size: 415342 # 415.34 kB
@@ -16,7 +16,7 @@ App.tsx:
sys deallocs: 63
sys alloc bytes: 2128555 # 2.13 MB
sys peak growth: 1612427 # 1.61 MB
arena allocs: 16027
arena allocs: 16046
arena reallocs: 2721
arena size: 2888280 # 2.89 MB
@@ -38,7 +38,7 @@ pdf.mjs:
sys deallocs: 2467
sys alloc bytes: 5334247 # 5.33 MB
sys peak growth: 3693355 # 3.69 MB
arena allocs: 45873
arena allocs: 45881
arena reallocs: 7718
arena size: 6326168 # 6.33 MB
@@ -49,9 +49,9 @@ antd.js:
sys deallocs: 1038
sys alloc bytes: 29976031 # 29.98 MB
sys peak growth: 19934725 # 19.93 MB
arena allocs: 371607
arena allocs: 371736
arena reallocs: 76289
arena size: 49109008 # 49.11 MB
arena size: 49111224 # 49.11 MB
binder.ts:
file size: 193077 # 193.08 kB
@@ -60,7 +60,7 @@ binder.ts:
sys deallocs: 33
sys alloc bytes: 963008 # 963.01 kB
sys peak growth: 657404 # 657.40 kB
arena allocs: 7026
arena allocs: 7032
arena reallocs: 842
arena size: 1167904 # 1.17 MB
@@ -71,7 +71,7 @@ kitchen-sink.tsx:
sys deallocs: 1858
sys alloc bytes: 5307512 # 5.31 MB
sys peak growth: 3707425 # 3.71 MB
arena allocs: 66761
arena allocs: 66806
arena reallocs: 12252
arena size: 9425560 # 9.43 MB