Skip to content

Commit 94b2953

Browse files
sapphi-redsapphi-red
committed
fix(minifier): don't inline single use variables that are not literals to for statement initializers
1 parent b4e9d2c commit 94b2953

File tree

3 files changed

+85
-18
lines changed

3 files changed

+85
-18
lines changed

‎crates/oxc_minifier/src/peephole/minimize_statements.rs‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,12 @@ impl<'a> PeepholeOptimizations {
384384
if let Some(first_decl) = var_decl.declarations.first_mut()
385385
&& let Some(first_decl_init) = first_decl.init.as_mut()
386386
{
387-
let changed =
388-
Self::substitute_single_use_symbol_in_statement(first_decl_init, result, ctx);
387+
let changed = Self::substitute_single_use_symbol_in_statement(
388+
first_decl_init,
389+
result,
390+
ctx,
391+
false,
392+
);
389393
if changed {
390394
ctx.state.changed = true;
391395
}
@@ -433,8 +437,12 @@ impl<'a> PeepholeOptimizations {
433437

434438
ctx: &mut Ctx<'a, '_>,
435439
) {
436-
let changed =
437-
Self::substitute_single_use_symbol_in_statement(&mut expr_stmt.expression, result, ctx);
440+
let changed = Self::substitute_single_use_symbol_in_statement(
441+
&mut expr_stmt.expression,
442+
result,
443+
ctx,
444+
false,
445+
);
438446
if changed {
439447
ctx.state.changed = true;
440448
}
@@ -562,6 +570,7 @@ impl<'a> PeepholeOptimizations {
562570
&mut switch_stmt.discriminant,
563571
result,
564572
ctx,
573+
false,
565574
);
566575
if changed {
567576
ctx.state.changed = true;
@@ -588,7 +597,7 @@ impl<'a> PeepholeOptimizations {
588597
ctx: &mut Ctx<'a, '_>,
589598
) -> ControlFlow<()> {
590599
let changed =
591-
Self::substitute_single_use_symbol_in_statement(&mut if_stmt.test, result, ctx);
600+
Self::substitute_single_use_symbol_in_statement(&mut if_stmt.test, result, ctx, false);
592601
if changed {
593602
ctx.state.changed = true;
594603
}
@@ -747,8 +756,12 @@ impl<'a> PeepholeOptimizations {
747756
ctx: &mut Ctx<'a, '_>,
748757
) {
749758
if let Some(ret_argument_expr) = &mut ret_stmt.argument {
750-
let changed =
751-
Self::substitute_single_use_symbol_in_statement(ret_argument_expr, result, ctx);
759+
let changed = Self::substitute_single_use_symbol_in_statement(
760+
ret_argument_expr,
761+
result,
762+
ctx,
763+
false,
764+
);
752765
if changed {
753766
ctx.state.changed = true;
754767
}
@@ -799,8 +812,12 @@ impl<'a> PeepholeOptimizations {
799812

800813
ctx: &mut Ctx<'a, '_>,
801814
) {
802-
let changed =
803-
Self::substitute_single_use_symbol_in_statement(&mut throw_stmt.argument, result, ctx);
815+
let changed = Self::substitute_single_use_symbol_in_statement(
816+
&mut throw_stmt.argument,
817+
result,
818+
ctx,
819+
false,
820+
);
804821
if changed {
805822
ctx.state.changed = true;
806823
}
@@ -830,10 +847,12 @@ impl<'a> PeepholeOptimizations {
830847
if let Some(first_decl) = var_decl.declarations.first_mut()
831848
&& let Some(first_decl_init) = first_decl.init.as_mut()
832849
{
850+
let is_block_scoped_decl = !first_decl.kind.is_var();
833851
let changed = Self::substitute_single_use_symbol_in_statement(
834852
first_decl_init,
835853
result,
836854
ctx,
855+
is_block_scoped_decl,
837856
);
838857
if changed {
839858
ctx.state.changed = true;
@@ -843,7 +862,7 @@ impl<'a> PeepholeOptimizations {
843862
match_expression!(ForStatementInit) => {
844863
let init = init.to_expression_mut();
845864
let changed =
846-
Self::substitute_single_use_symbol_in_statement(init, result, ctx);
865+
Self::substitute_single_use_symbol_in_statement(init, result, ctx, false);
847866
if changed {
848867
ctx.state.changed = true;
849868
}
@@ -922,10 +941,12 @@ impl<'a> PeepholeOptimizations {
922941
// That is evaluated before the right hand side is evaluated. So, in that case, skip the single use substitution.
923942
if !matches!(&for_in_stmt.left, ForStatementLeft::VariableDeclaration(var_decl) if var_decl.has_init())
924943
{
944+
let is_block_scoped_decl = matches!(&for_in_stmt.left, ForStatementLeft::VariableDeclaration(var_decl) if !var_decl.kind.is_var());
925945
let changed = Self::substitute_single_use_symbol_in_statement(
926946
&mut for_in_stmt.right,
927947
result,
928948
ctx,
949+
is_block_scoped_decl,
929950
);
930951
if changed {
931952
ctx.state.changed = true;
@@ -1005,8 +1026,13 @@ impl<'a> PeepholeOptimizations {
10051026
result: &mut Vec<'a, Statement<'a>>,
10061027
ctx: &mut Ctx<'a, '_>,
10071028
) {
1008-
let changed =
1009-
Self::substitute_single_use_symbol_in_statement(&mut for_of_stmt.right, result, ctx);
1029+
let is_block_scoped_decl = matches!(&for_of_stmt.left, ForStatementLeft::VariableDeclaration(var_decl) if !var_decl.kind.is_var());
1030+
let changed = Self::substitute_single_use_symbol_in_statement(
1031+
&mut for_of_stmt.right,
1032+
result,
1033+
ctx,
1034+
is_block_scoped_decl,
1035+
);
10101036
if changed {
10111037
ctx.state.changed = true;
10121038
}
@@ -1100,6 +1126,7 @@ impl<'a> PeepholeOptimizations {
11001126
expr_in_stmt: &mut Expression<'a>,
11011127
stmts: &mut Vec<'a, Statement<'a>>,
11021128
ctx: &Ctx<'a, '_>,
1129+
non_scoped_literal_only: bool,
11031130
) -> bool {
11041131
// TODO: we should skip this compression when direct eval exists
11051132
// because the code inside eval may reference the variable
@@ -1139,6 +1166,9 @@ impl<'a> PeepholeOptimizations {
11391166
{
11401167
return true;
11411168
}
1169+
if non_scoped_literal_only && !prev_decl_init.is_literal_value(false, ctx) {
1170+
return true;
1171+
}
11421172
let replaced = Self::substitute_single_use_symbol_in_expression(
11431173
expr_in_stmt,
11441174
&prev_decl_id.name,

‎crates/oxc_minifier/tests/peephole/inline_single_use_variable.rs‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,17 +156,46 @@ fn test_inline_single_use_variable() {
156156
);
157157

158158
test(
159-
"function wrapper() { var x = foo; for (var i = x; i < 10; i++) console.log(i) }",
160-
"function wrapper() { for (var i = foo; i < 10; i++) console.log(i) }",
159+
"function wrapper() { var x = 1; for (var i = x; i < 10; i++) console.log(i) }",
160+
"function wrapper() { for (var i = 1; i < 10; i++) console.log(i) }",
161161
);
162162
test(
163-
"function wrapper() { var i, x = foo; for (i = x; i < 10; i++) console.log(i) }",
164-
"function wrapper() { var i; for (i = foo; i < 10; i++) console.log(i) }",
163+
"function wrapper() { var x = 1; for (let i = x; i < 10; i++) console.log(i) }",
164+
"function wrapper() { for (let i = 1; i < 10; i++) console.log(i) }",
165+
);
166+
test(
167+
"function wrapper() { var x = function () { return console.log(i), 1 }; for (let i = x(); i < 10; i++) console.log(i) }",
168+
"function wrapper() { var x = function () { return console.log(i), 1 }; for (let i = x(); i < 10; i++) console.log(i) }",
169+
);
170+
// this is fine because `let j` inside the block cannot be referenced from `var i = j`
171+
test(
172+
"function wrapper() { var x = j; for (var i = x; i < 10; i++) { let j; console.log(i) } }",
173+
"function wrapper() { for (var i = j; i < 10; i++) { let j; console.log(i) } }",
174+
);
175+
test(
176+
"function wrapper() { var i, x = 1; for (i = x; i < 10; i++) console.log(i) }",
177+
"function wrapper() { var i; for (i = 1; i < 10; i++) console.log(i) }",
178+
);
179+
test(
180+
"function wrapper() { var x = i; for (var i = x; i < 10; i++) console.log(i) }",
181+
"function wrapper() { for (var i = i; i < 10; i++) console.log(i) }",
182+
);
183+
test(
184+
"function wrapper() { var x = i; for (let i = x; i < 10; i++) console.log(i) }",
185+
"function wrapper() { var x = i; for (let i = x; i < 10; i++) console.log(i) }",
165186
);
166187
test(
167188
"function wrapper() { var x = {}; for (var a in x) console.log(a) }",
168189
"function wrapper() { for (var a in {}) console.log(a) }",
169190
);
191+
test(
192+
"function wrapper() { var x = a; for (var a in x) console.log(a) }",
193+
"function wrapper() { for (var a in a) console.log(a) }",
194+
);
195+
test(
196+
"function wrapper() { var x = a; for (let a in x) console.log(a) }",
197+
"function wrapper() { var x = a; for (let a in x) console.log(a) }",
198+
);
170199
test(
171200
"function wrapper() { var x = {}; for (var a = 0 in x) console.log(a) }",
172201
"function wrapper() { var x = {}; for (var a = 0 in x) console.log(a) }",
@@ -175,6 +204,14 @@ fn test_inline_single_use_variable() {
175204
"function wrapper() { var x = []; for (var a of x) console.log(a) }",
176205
"function wrapper() { for (var a of []) console.log(a) }",
177206
);
207+
test(
208+
"function wrapper() { var x = a; for (var a of x) console.log(a) }",
209+
"function wrapper() { for (var a of a) console.log(a) }",
210+
);
211+
test(
212+
"function wrapper() { var x = a; for (let a of x) console.log(a) }",
213+
"function wrapper() { var x = a; for (let a of x) console.log(a) }",
214+
);
178215
}
179216

180217
#[test]

‎tasks/track_memory_allocations/allocs_minifier.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
File | File size || Sys allocs | Sys reallocs || Arena allocs | Arena reallocs | Arena bytes
22
-------------------------------------------------------------------------------------------------------------------------------------------
3-
checker.ts | 2.92 MB || 84058 | 14190 || 153533 | 29395 | 5.615 MB
3+
checker.ts | 2.92 MB || 84073 | 14190 || 153691 | 29463 | 5.625 MB
44

55
cal.com.tsx | 1.06 MB || 40525 | 3033 || 37074 | 4733 | 1.654 MB
66

77
RadixUIAdoptionSection.jsx | 2.52 kB || 82 | 8 || 30 | 6 | 992 B
88

9-
pdf.mjs | 567.30 kB || 19572 | 2900 || 47384 | 7770 | 1.623 MB
9+
pdf.mjs | 567.30 kB || 19576 | 2900 || 47400 | 7781 | 1.624 MB
1010

1111
antd.js | 6.69 MB || 99854 | 13518 || 331725 | 70117 | 17.407 MB
1212

0 commit comments

Comments
 (0)