From d4b3a08975ac7bd40c1cb420fe0f639c789a2aea Mon Sep 17 00:00:00 2001 From: magic-akari Date: Mon, 14 Oct 2024 17:14:01 +0800 Subject: [PATCH 1/7] Fix(minifier): Preserve init variable declarations when removing `for` statements during DCE --- .../ast_passes/peephole_remove_dead_code.rs | 3 +++ crates/oxc_minifier/src/keep_var.rs | 22 ++++++++++--------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index 6c81df5be80b7..a313d666c9ccb 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -202,6 +202,9 @@ impl<'a, 'b> PeepholeRemoveDeadCode { // Remove the entire `for` statement. // Check vars in statement let mut keep_var = KeepVar::new(ctx.ast); + if let Some(ForStatementInit::VariableDeclaration(var_init)) = &for_stmt.init { + keep_var.visit_variable_declaration(var_init); + } keep_var.visit_statement(&for_stmt.body); Some( keep_var diff --git a/crates/oxc_minifier/src/keep_var.rs b/crates/oxc_minifier/src/keep_var.rs index 6652e1e14d538..21006d9169304 100644 --- a/crates/oxc_minifier/src/keep_var.rs +++ b/crates/oxc_minifier/src/keep_var.rs @@ -34,19 +34,21 @@ impl<'a> Visit<'a> for KeepVar<'a> { // match_module_declaration!(Statement) => { // visitor.visit_module_declaration(it.to_module_declaration()) // } - Statement::VariableDeclaration(decl) => { - if decl.kind.is_var() { - decl.bound_names(&mut |ident| { - self.vars.push((ident.name.clone(), ident.span)); - }); - if decl.has_init() { - self.all_hoisted = false; - } - } - } + Statement::VariableDeclaration(decl) => self.visit_variable_declaration(decl), _ => {} } } + + fn visit_variable_declaration(&mut self, it: &VariableDeclaration<'a>) { + if it.kind.is_var() { + it.bound_names(&mut |ident| { + self.vars.push((ident.name.clone(), ident.span)); + }); + if it.has_init() { + self.all_hoisted = false; + } + } + } } impl<'a> KeepVar<'a> { From 8e7ab43ec08d7da8d059f441dcd63f4f3e51671e Mon Sep 17 00:00:00 2001 From: magic-akari Date: Mon, 14 Oct 2024 19:39:35 +0800 Subject: [PATCH 2/7] fix --- crates/oxc_ast/src/ast_builder_impl.rs | 11 +++++++ .../ast_passes/peephole_remove_dead_code.rs | 31 ++++++++++++++----- crates/oxc_minifier/src/keep_var.rs | 10 ++++-- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/crates/oxc_ast/src/ast_builder_impl.rs b/crates/oxc_ast/src/ast_builder_impl.rs index 69c1e636e4f53..8f7375a59da8a 100644 --- a/crates/oxc_ast/src/ast_builder_impl.rs +++ b/crates/oxc_ast/src/ast_builder_impl.rs @@ -112,6 +112,17 @@ impl<'a> AstBuilder<'a> { mem::replace(decl, empty_decl) } + #[inline] + pub fn move_variable_declaration(self, decl: &mut VariableDeclaration<'a>) -> VariableDeclaration<'a> { + let empty_decl = self.variable_declaration( + Span::default(), + VariableDeclarationKind::Var, + self.vec(), + false, + ); + mem::replace(decl, empty_decl) + } + #[inline] pub fn move_vec(self, vec: &mut Vec<'a, T>) -> Vec<'a, T> { mem::replace(vec, self.vec()) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index a313d666c9ccb..cf02c6ce8b695 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -202,15 +202,27 @@ impl<'a, 'b> PeepholeRemoveDeadCode { // Remove the entire `for` statement. // Check vars in statement let mut keep_var = KeepVar::new(ctx.ast); - if let Some(ForStatementInit::VariableDeclaration(var_init)) = &for_stmt.init { - keep_var.visit_variable_declaration(var_init); - } keep_var.visit_statement(&for_stmt.body); - Some( - keep_var - .get_variable_declaration_statement() - .unwrap_or_else(|| ctx.ast.statement_empty(SPAN)), - ) + + let mut var_decl = keep_var.get_variable_declaration(); + + if let Some(ForStatementInit::VariableDeclaration(var_init)) = &mut for_stmt.init { + if var_init.kind.is_var() { + if let Some(var_decl) = &mut var_decl { + var_decl + .declarations + .splice(0..0, ctx.ast.move_vec(&mut var_init.declarations)); + } else { + var_decl = Some(ctx.ast.move_variable_declaration(var_init)); + } + } + } + + var_decl + .map(|var_decl| { + ctx.ast.statement_declaration(ctx.ast.declaration_from_variable(var_decl)) + }) + .or_else(|| Some(ctx.ast.statement_empty(SPAN))) } Some(true) => { // Remove the test expression. @@ -353,6 +365,9 @@ mod test { // Make sure it plays nice with minimizing fold("for(;false;) { foo(); continue }", ""); + fold("for (var { c, x: [d] } = {}; 0;);", "var { c, x: [d] } = {};"); + fold("for (var se = [1, 2]; false;);", "var se = [1, 2];"); + // fold("l1:for(;false;) { }", ""); } diff --git a/crates/oxc_minifier/src/keep_var.rs b/crates/oxc_minifier/src/keep_var.rs index 21006d9169304..b66c0f3c861ca 100644 --- a/crates/oxc_minifier/src/keep_var.rs +++ b/crates/oxc_minifier/src/keep_var.rs @@ -60,7 +60,7 @@ impl<'a> KeepVar<'a> { self.all_hoisted } - pub fn get_variable_declaration_statement(self) -> Option> { + pub fn get_variable_declaration(self) -> Option> { if self.vars.is_empty() { return None; } @@ -73,7 +73,13 @@ impl<'a> KeepVar<'a> { })); let decl = self.ast.variable_declaration(SPAN, kind, decls, false); - let stmt = self.ast.statement_declaration(self.ast.declaration_from_variable(decl)); + Some(decl) + } + + pub fn get_variable_declaration_statement(self) -> Option> { + let stmt = self.ast.statement_declaration( + self.ast.declaration_from_variable(self.get_variable_declaration()?), + ); Some(stmt) } } From be347ea646eb260d4f76e5d66cba6c5dfb94c5ce Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 11:40:28 +0000 Subject: [PATCH 3/7] [autofix.ci] apply automated fixes --- crates/oxc_ast/src/ast_builder_impl.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/oxc_ast/src/ast_builder_impl.rs b/crates/oxc_ast/src/ast_builder_impl.rs index 8f7375a59da8a..ac4d12b6cad3c 100644 --- a/crates/oxc_ast/src/ast_builder_impl.rs +++ b/crates/oxc_ast/src/ast_builder_impl.rs @@ -113,7 +113,10 @@ impl<'a> AstBuilder<'a> { } #[inline] - pub fn move_variable_declaration(self, decl: &mut VariableDeclaration<'a>) -> VariableDeclaration<'a> { + pub fn move_variable_declaration( + self, + decl: &mut VariableDeclaration<'a>, + ) -> VariableDeclaration<'a> { let empty_decl = self.variable_declaration( Span::default(), VariableDeclarationKind::Var, From ff35fefd1b7869c44173111b61735f45c31a237a Mon Sep 17 00:00:00 2001 From: magic-akari Date: Mon, 14 Oct 2024 19:45:36 +0800 Subject: [PATCH 4/7] chore: add more test cases --- crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index cf02c6ce8b695..d3a36c98cc122 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -367,6 +367,7 @@ mod test { fold("for (var { c, x: [d] } = {}; 0;);", "var { c, x: [d] } = {};"); fold("for (var se = [1, 2]; false;);", "var se = [1, 2];"); + fold("for (var se = [1, 2]; false;) { var a = 0; }", "var se = [1, 2], a;"); // fold("l1:for(;false;) { }", ""); } From ac85e9c2dd2164eb0010d6fdea72df6005fa462a Mon Sep 17 00:00:00 2001 From: magic-akari Date: Mon, 14 Oct 2024 20:08:12 +0800 Subject: [PATCH 5/7] fix: block scope of for loop --- .../src/ast_passes/peephole_remove_dead_code.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index d3a36c98cc122..bca18769f3eaa 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -215,6 +215,10 @@ impl<'a, 'b> PeepholeRemoveDeadCode { } else { var_decl = Some(ctx.ast.move_variable_declaration(var_init)); } + } else { + // block scope + // for(const/let x = 1; false; ) { ... } + var_decl = None; } } @@ -368,6 +372,8 @@ mod test { fold("for (var { c, x: [d] } = {}; 0;);", "var { c, x: [d] } = {};"); fold("for (var se = [1, 2]; false;);", "var se = [1, 2];"); fold("for (var se = [1, 2]; false;) { var a = 0; }", "var se = [1, 2], a;"); + + fold("for (let se = [1, 2]; false;) { var a = 0; }", ""); // fold("l1:for(;false;) { }", ""); } From ae4aac0016a2ed98fca61e5cefed8b66d496e531 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 12:09:06 +0000 Subject: [PATCH 6/7] [autofix.ci] apply automated fixes --- crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index bca18769f3eaa..c97c0f314b1a0 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -372,7 +372,7 @@ mod test { fold("for (var { c, x: [d] } = {}; 0;);", "var { c, x: [d] } = {};"); fold("for (var se = [1, 2]; false;);", "var se = [1, 2];"); fold("for (var se = [1, 2]; false;) { var a = 0; }", "var se = [1, 2], a;"); - + fold("for (let se = [1, 2]; false;) { var a = 0; }", ""); // fold("l1:for(;false;) { }", ""); From a39cd49417139cb8e157f9c6ff115ddd9ee8925a Mon Sep 17 00:00:00 2001 From: magic-akari Date: Mon, 14 Oct 2024 20:12:21 +0800 Subject: [PATCH 7/7] Revert "fix: block scope of for loop" This reverts commit ac85e9c2dd2164eb0010d6fdea72df6005fa462a. --- .../src/ast_passes/peephole_remove_dead_code.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index c97c0f314b1a0..d3a36c98cc122 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -215,10 +215,6 @@ impl<'a, 'b> PeepholeRemoveDeadCode { } else { var_decl = Some(ctx.ast.move_variable_declaration(var_init)); } - } else { - // block scope - // for(const/let x = 1; false; ) { ... } - var_decl = None; } } @@ -373,8 +369,6 @@ mod test { fold("for (var se = [1, 2]; false;);", "var se = [1, 2];"); fold("for (var se = [1, 2]; false;) { var a = 0; }", "var se = [1, 2], a;"); - fold("for (let se = [1, 2]; false;) { var a = 0; }", ""); - // fold("l1:for(;false;) { }", ""); }