mirror of
https://github.com/aphrody-labs/oxc.git
synced 2026-10-15 06:39:17 +00:00
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></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:
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}",
|
||||
);
|
||||
}
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user