From 702c049ebc3bb873d4c2be8eeb247f34bb551962 Mon Sep 17 00:00:00 2001 From: 7086cmd <54303040+7086cmd@users.noreply.github.com> Date: Sat, 12 Oct 2024 07:59:44 +0000 Subject: [PATCH] refactor(minifier): move compress block to dce (#6468) --- .../ast_passes/peephole_remove_dead_code.rs | 56 ++++++++++++++++++- .../peephole_substitute_alternate_syntax.rs | 22 +------- .../tests/ast_passes/dead_code_elimination.rs | 50 ++++++++--------- 3 files changed, 80 insertions(+), 48 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 f62ed6a008ab7..0d0aa37acd267 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 @@ -42,6 +42,10 @@ impl<'a> Traverse<'a> for PeepholeRemoveDeadCode { } } + fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { + self.compress_block(stmt, ctx); + } + fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { if stmts.iter().any(|stmt| matches!(stmt, Statement::EmptyStatement(_))) { stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_))); @@ -128,6 +132,27 @@ impl<'a> PeepholeRemoveDeadCode { } } + /// Remove block from single line blocks + /// `{ block } -> block` + fn compress_block(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { + if let Statement::BlockStatement(block) = stmt { + // Avoid compressing `if (x) { var x = 1 }` to `if (x) var x = 1` due to different + // semantics according to AnnexB, which lead to different semantics. + if block.body.len() == 1 && !block.body[0].is_declaration() { + *stmt = block.body.remove(0); + self.compress_block(stmt, ctx); + self.changed = true; + return; + } + if block.body.len() == 0 + && (ctx.parent().is_block_statement() || ctx.parent().is_program()) + { + // Remove the block if it is empty and the parent is a block statement. + *stmt = ctx.ast.statement_empty(SPAN); + } + } + } + fn try_fold_if( &mut self, if_stmt: &mut IfStatement<'a>, @@ -264,6 +289,34 @@ mod test { test(js, expected); } + #[test] + fn test_fold_block() { + fold("{{foo()}}", "foo()"); + fold("{foo();{}}", "foo()"); + fold("{{foo()}{}}", "foo()"); + // fold("{{foo()}{bar()}}", "foo();bar()"); + fold("{if(false)foo(); {bar()}}", "bar()"); + fold("{if(false)if(false)if(false)foo(); {bar()}}", "bar()"); + + fold("{'hi'}", ""); + // fold("{x==3}", ""); + // fold("{`hello ${foo}`}", ""); + // fold("{ (function(){x++}) }", ""); + // fold_same("function f(){return;}"); + // fold("function f(){return 3;}", "function f(){return 3}"); + // fold_same("function f(){if(x)return; x=3; return; }"); + // fold("{x=3;;;y=2;;;}", "x=3;y=2"); + + // Cases to test for empty block. + // fold("while(x()){x}", "while(x());"); + // fold("while(x()){x()}", "while(x())x()"); + // fold("for(x=0;x<100;x++){x}", "for(x=0;x<100;x++);"); + // fold("for(x in y){x}", "for(x in y);"); + // fold("for (x of y) {x}", "for(x of y);"); + // fold_same("for (let x = 1; x <10; x++ ) {}"); + // fold_same("for (var x = 1; x <10; x++ ) {}"); + } + #[test] #[ignore] fn test_remove_no_op_labelled_statement() { @@ -300,9 +353,6 @@ mod test { fold("for(;false;) { foo(); continue }", ""); // fold("l1:for(;false;) { }", ""); - - // TODO handle single block statement - fold_same("for(;a;) { foo(); }"); } #[test] diff --git a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs index 92596163c17ae..68a0e2e4bf94e 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs @@ -33,11 +33,6 @@ impl<'a> CompressorPass<'a> for PeepholeSubstituteAlternateSyntax { } impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax { - fn enter_statement(&mut self, stmt: &mut Statement<'a>, _ctx: &mut TraverseCtx<'a>) { - self.compress_block(stmt); - // self.compress_while(stmt); - } - fn exit_return_statement( &mut self, stmt: &mut ReturnStatement<'a>, @@ -161,20 +156,6 @@ impl<'a> PeepholeSubstituteAlternateSyntax { /* Statements */ - /// Remove block from single line blocks - /// `{ block } -> block` - fn compress_block(&mut self, stmt: &mut Statement<'a>) { - if let Statement::BlockStatement(block) = stmt { - // Avoid compressing `if (x) { var x = 1 }` to `if (x) var x = 1` due to different - // semantics according to AnnexB, which lead to different semantics. - if block.body.len() == 1 && !block.body[0].is_declaration() { - *stmt = block.body.remove(0); - self.compress_block(stmt); - self.changed = true; - } - } - } - // /// Transforms `while(expr)` to `for(;expr;)` // fn compress_while(&mut self, stmt: &mut Statement<'a>) { // let Statement::WhileStatement(while_stmt) = stmt else { return }; @@ -531,7 +512,8 @@ mod test { test("function f(){return void 0;}", "function f(){return}"); test("function f(){return void foo();}", "function f(){return void foo()}"); test("function f(){return undefined;}", "function f(){return}"); - test("function f(){if(a()){return undefined;}}", "function f(){if(a())return}"); + // Here we handle the block in dce. + test("function f(){if(a()){return undefined;}}", "function f(){if(a()){return}}"); } #[test] diff --git a/crates/oxc_minifier/tests/ast_passes/dead_code_elimination.rs b/crates/oxc_minifier/tests/ast_passes/dead_code_elimination.rs index e030cd6997cf8..6b5e777fc29d8 100644 --- a/crates/oxc_minifier/tests/ast_passes/dead_code_elimination.rs +++ b/crates/oxc_minifier/tests/ast_passes/dead_code_elimination.rs @@ -18,57 +18,57 @@ fn test_same(source_text: &str) { #[test] fn dce_if_statement() { - test("if (true) { foo }", "{ foo }"); - test("if (true) { foo } else { bar }", "{ foo }"); - test("if (false) { foo } else { bar }", "{ bar }"); + test("if (true) { foo }", "foo"); + test("if (true) { foo } else { bar }", "foo"); + test("if (false) { foo } else { bar }", "bar"); - test("if (xxx) { foo } else if (false) { bar }", "if (xxx) { foo }"); - test("if (xxx) { foo } else if (false) { bar } else { baz }", "if (xxx) { foo } else { baz }"); - test("if (xxx) { foo } else if (false) { bar } else if (false) { baz }", "if (xxx) { foo }"); + test("if (xxx) { foo } else if (false) { bar }", "if (xxx) foo"); + test("if (xxx) { foo } else if (false) { bar } else { baz }", "if (xxx) foo; else baz"); + test("if (xxx) { foo } else if (false) { bar } else if (false) { baz }", "if (xxx) foo"); test( "if (xxx) { foo } else if (false) { bar } else if (false) { baz } else { quaz }", - "if (xxx) { foo } else { quaz }", + "if (xxx) foo; else quaz", ); test( "if (xxx) { foo } else if (true) { bar } else if (false) { baz }", - "if (xxx) { foo } else { bar }", + "if (xxx) foo; else bar", ); test( "if (xxx) { foo } else if (false) { bar } else if (true) { baz }", - "if (xxx) { foo } else { baz }", + "if (xxx) foo; else baz", ); test( "if (xxx) { foo } else if (true) { bar } else if (true) { baz }", - "if (xxx) { foo } else { bar }", + "if (xxx) foo; else bar", ); test( "if (xxx) { foo } else if (false) { var a; var b; } else if (false) { var c; var d; }", - "if (xxx) { foo } else var c, d;", + "if (xxx) foo; else var c, d;", ); - test("if (!false) { foo }", "{ foo }"); - test("if (!true) { foo } else { bar }", "{ bar }"); + test("if (!false) { foo }", "foo"); + test("if (!true) { foo } else { bar }", "bar"); - test("if (!false && xxx) { foo }", "if (xxx) { foo; }"); - test("if (!true && yyy) { foo } else { bar }", "{ bar }"); + test("if (!false && xxx) { foo }", "if (xxx) foo"); + test("if (!true && yyy) { foo } else { bar }", "bar"); - test("if (true || xxx) { foo }", "{ foo }"); - test("if (false || xxx) { foo }", "if (xxx) { foo }"); + test("if (true || xxx) { foo }", "foo"); + test("if (false || xxx) { foo }", "if (xxx) foo"); - test("if ('production' == 'production') { foo } else { bar }", "{ foo }"); - test("if ('development' == 'production') { foo } else { bar }", "{ bar }"); + test("if ('production' == 'production') { foo } else { bar }", "foo"); + test("if ('development' == 'production') { foo } else { bar }", "bar"); - test("if ('production' === 'production') { foo } else { bar }", "{ foo }"); - test("if ('development' === 'production') { foo } else { bar }", "{ bar }"); + test("if ('production' === 'production') { foo } else { bar }", "foo"); + test("if ('development' === 'production') { foo } else { bar }", "bar"); // Shadowed `undefined` as a variable should not be erased. // This is a rollup test. test_same("function foo(undefined) { if (!undefined) { } }"); test("function foo() { if (undefined) { bar } }", "function foo() { }"); - test_same("function foo() { { bar } }"); + test("function foo() { { bar } }", "function foo() { bar }"); - test("if (true) { foo; } if (true) { foo; }", "{ foo; } { foo; }"); + test("if (true) { foo; } if (true) { foo; }", "foo; foo;"); test( " @@ -83,11 +83,11 @@ fn dce_if_statement() { // nested expression test( "const a = { fn: function() { if (true) { foo; } } }", - "const a = { fn: function() { { foo; } } }", + "const a = { fn: function() { foo; } }", ); // parenthesized - test("if (!!(false)) { REMOVE; } else { KEEP; }", "{ KEEP }"); + test("if (!!(false)) { REMOVE; } else { KEEP; }", "KEEP"); // typeof test("if (typeof 1 !== 'number') { REMOVE; }", "");