feat(minifier): loop compressor passes (#6013)

crates/oxc_minifier/examples/minifier.rs

@@ -27,8 +27,9 @@ fn main() -> std::io::Result<()> {
     println!("{printed}");
 
     if twice {
-        let printed = minify(&printed, source_type, mangle);
-        println!("{printed}");
+        let printed2 = minify(&printed, source_type, mangle);
+        println!("{printed2}");
+        println!("same = {}", printed == printed2);
     }
 
     Ok(())

crates/oxc_minifier/src/ast_passes/collapse_variable_declarations.rs

@@ -9,25 +9,37 @@ use crate::{CompressOptions, CompressorPass};
 /// `var a; var b = 1; var c = 2` => `var a, b = 1; c = 2`
 pub struct CollapseVariableDeclarations {
     options: CompressOptions,
+
+    changed: bool,
 }
 
-impl<'a> CompressorPass<'a> for CollapseVariableDeclarations {}
+impl<'a> CompressorPass<'a> for CollapseVariableDeclarations {
+    fn changed(&self) -> bool {
+        self.changed
+    }
+
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
+        self.changed = false;
+        oxc_traverse::walk_program(self, program, ctx);
+    }
+}
 
 impl<'a> Traverse<'a> for CollapseVariableDeclarations {
     fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
-        if self.options.join_vars {
-            self.join_vars(stmts, ctx);
-        }
+        self.join_vars(stmts, ctx);
     }
 }
 
 impl<'a> CollapseVariableDeclarations {
     pub fn new(options: CompressOptions) -> Self {
-        Self { options }
+        Self { options, changed: false }
     }
 
     /// Join consecutive var statements
-    fn join_vars(&self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
+    fn join_vars(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
+        if self.options.join_vars {
+            return;
+        }
         // Collect all the consecutive ranges that contain joinable vars.
         // This is required because Rust prevents in-place vec mutation.
         let mut ranges = vec![];
@@ -83,6 +95,7 @@ impl<'a> CollapseVariableDeclarations {
             }
         }
         *stmts = new_stmts;
+        self.changed = true;
     }
 }
 

crates/oxc_minifier/src/ast_passes/exploit_assigns.rs

@@ -1,19 +1,31 @@
-use oxc_traverse::Traverse;
+use oxc_ast::ast::*;
+use oxc_traverse::{Traverse, TraverseCtx};
 
 use crate::CompressorPass;
 
 /// Tries to chain assignments together.
 ///
 /// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/ExploitAssigns.java>
-pub struct ExploitAssigns;
+pub struct ExploitAssigns {
+    changed: bool,
+}
+
+impl<'a> CompressorPass<'a> for ExploitAssigns {
+    fn changed(&self) -> bool {
+        self.changed
+    }
 
-impl<'a> CompressorPass<'a> for ExploitAssigns {}
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
+        self.changed = false;
+        oxc_traverse::walk_program(self, program, ctx);
+    }
+}
 
 impl<'a> Traverse<'a> for ExploitAssigns {}
 
 impl ExploitAssigns {
     pub fn new() -> Self {
-        Self {}
+        Self { changed: false }
     }
 }
 

crates/oxc_minifier/src/ast_passes/mod.rs

@@ -18,7 +18,7 @@ pub use statement_fusion::StatementFusion;
 
 use oxc_ast::ast::Program;
 use oxc_semantic::{ScopeTree, SymbolTable};
-use oxc_traverse::{walk_program, Traverse, TraverseCtx};
+use oxc_traverse::{Traverse, TraverseCtx};
 
 use crate::node_util::NodeUtil;
 
@@ -33,11 +33,7 @@ impl<'a> NodeUtil for TraverseCtx<'a> {
 }
 
 pub trait CompressorPass<'a>: Traverse<'a> {
-    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>)
-    where
-        Self: Traverse<'a>,
-        Self: Sized,
-    {
-        walk_program(self, program, ctx);
-    }
+    fn changed(&self) -> bool;
+
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>);
 }

crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs

@@ -22,10 +22,19 @@ use crate::{
 ///
 /// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>
 pub struct PeepholeFoldConstants {
-    evaluate: bool,
+    changed: bool,
 }
 
-impl<'a> CompressorPass<'a> for PeepholeFoldConstants {}
+impl<'a> CompressorPass<'a> for PeepholeFoldConstants {
+    fn changed(&self) -> bool {
+        self.changed
+    }
+
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
+        self.changed = false;
+        oxc_traverse::walk_program(self, program, ctx);
+    }
+}
 
 impl<'a> Traverse<'a> for PeepholeFoldConstants {
     fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
@@ -35,17 +44,12 @@ impl<'a> Traverse<'a> for PeepholeFoldConstants {
 
 impl<'a> PeepholeFoldConstants {
     pub fn new() -> Self {
-        Self { evaluate: false }
-    }
-
-    pub fn with_evaluate(mut self, yes: bool) -> Self {
-        self.evaluate = yes;
-        self
+        Self { changed: false }
     }
 
     // [optimizeSubtree](https://github.com/google/closure-compiler/blob/75335a5138dde05030747abfd3c852cd34ea7429/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L72)
     // TODO: tryReduceOperandsForOp
-    pub fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
+    pub fn fold_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
         if let Some(folded_expr) = match expr {
             Expression::BinaryExpression(e) => self.try_fold_binary_operator(e, ctx),
             Expression::LogicalExpression(e)
@@ -60,6 +64,7 @@ impl<'a> PeepholeFoldConstants {
             _ => None,
         } {
             *expr = folded_expr;
+            self.changed = true;
         };
     }
 
@@ -92,13 +97,7 @@ impl<'a> PeepholeFoldConstants {
                 &binary_expr.right,
                 ctx,
             ),
-            // NOTE: string concat folding breaks our current evaluation of Test262 tests. The
-            // minifier is tested by comparing output of running the minifier once and twice,
-            // respectively. Since Test262Error messages include string concats, the outputs
-            // don't match (even though the produced code is valid). Additionally, We'll likely
-            // want to add `evaluate` checks for all constant folding, not just additions, but
-            // we're adding this here until a decision is made.
-            BinaryOperator::Addition if self.evaluate => {
+            BinaryOperator::Addition => {
                 self.try_fold_addition(binary_expr.span, &binary_expr.left, &binary_expr.right, ctx)
             }
             _ => None,

crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs

@@ -10,9 +10,20 @@ use crate::CompressorPass;
 /// with `? :` and short-circuit binary operators.
 ///
 /// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java>
-pub struct PeepholeMinimizeConditions;
+pub struct PeepholeMinimizeConditions {
+    changed: bool,
+}
+
+impl<'a> CompressorPass<'a> for PeepholeMinimizeConditions {
+    fn changed(&self) -> bool {
+        self.changed
+    }
 
-impl<'a> CompressorPass<'a> for PeepholeMinimizeConditions {}
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
+        self.changed = false;
+        oxc_traverse::walk_program(self, program, ctx);
+    }
+}
 
 impl<'a> Traverse<'a> for PeepholeMinimizeConditions {
     fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
@@ -21,13 +32,14 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions {
             _ => None,
         } {
             *expr = folded_expr;
+            self.changed = true;
         };
     }
 }
 
 impl<'a> PeepholeMinimizeConditions {
     pub fn new() -> Self {
-        Self
+        Self { changed: false }
     }
 
     /// Try to minimize NOT nodes such as `!(x==y)`.

crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs

@@ -10,17 +10,30 @@ use crate::{keep_var::KeepVar, node_util::NodeUtil, tri::Tri, CompressorPass};
 /// Terser option: `dead_code: true`.
 ///
 /// See `KeepVar` at the end of this file for `var` hoisting logic.
-pub struct PeepholeRemoveDeadCode;
+pub struct PeepholeRemoveDeadCode {
+    changed: bool,
+}
+
+impl<'a> CompressorPass<'a> for PeepholeRemoveDeadCode {
+    fn changed(&self) -> bool {
+        self.changed
+    }
 
-impl<'a> CompressorPass<'a> for PeepholeRemoveDeadCode {}
+    fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
+        self.changed = false;
+        oxc_traverse::walk_program(self, program, ctx);
+    }
+}
 
 impl<'a> Traverse<'a> for PeepholeRemoveDeadCode {
     fn enter_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
-        Self::fold_if_statement(stmt, ctx);
+        self.fold_if_statement(stmt, ctx);
     }
 
     fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
-        stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
+        if stmts.iter().any(|stmt| matches!(stmt, Statement::EmptyStatement(_))) {
+            stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
+        }
         self.dead_code_elimination(stmts, ctx);
     }
 
@@ -30,13 +43,14 @@ impl<'a> Traverse<'a> for PeepholeRemoveDeadCode {
             _ => None,
         } {
             *expr = folded_expr;
+            self.changed = true;
         }
     }
 }
 
 impl<'a> PeepholeRemoveDeadCode {
     pub fn new() -> Self {
-        Self {}
+        Self { changed: false }
     }
 
     /// Removes dead code thats comes after `return` statements after inlining `if` statements
@@ -76,6 +90,7 @@ impl<'a> PeepholeRemoveDeadCode {
         }
 
         let mut i = 0;
+        let len = stmts.len();
         stmts.retain(|s| {
             i += 1;
             if i - 1 <= index {
@@ -88,25 +103,35 @@ impl<'a> PeepholeRemoveDeadCode {
             false
         });
 
+        let all_hoisted = keep_var.all_hoisted();
         if let Some(stmt) = keep_var.get_variable_declaration_statement() {
             stmts.push(stmt);
+            if !all_hoisted {
+                self.changed = true;
+            }
+        }
+
+        if stmts.len() != len {
+            self.changed = true;
         }
     }
 
-    fn fold_if_statement(stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
+    fn fold_if_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
         let Statement::IfStatement(if_stmt) = stmt else { return };
 
         // Descend and remove `else` blocks first.
         if let Some(alternate) = &mut if_stmt.alternate {
-            Self::fold_if_statement(alternate, ctx);
+            self.fold_if_statement(alternate, ctx);
             if matches!(alternate, Statement::EmptyStatement(_)) {
                 if_stmt.alternate = None;
+                self.changed = true;
             }
         }
 
         match ctx.get_boolean_value(&if_stmt.test) {
             Tri::True => {
                 *stmt = ctx.ast.move_statement(&mut if_stmt.consequent);
+                self.changed = true;
             }
             Tri::False => {
                 *stmt = if let Some(alternate) = &mut if_stmt.alternate {
@@ -119,6 +144,7 @@ impl<'a> PeepholeRemoveDeadCode {
                         .get_variable_declaration_statement()
                         .unwrap_or_else(|| ctx.ast.statement_empty(SPAN))
                 };
+                self.changed = true;
             }
             Tri::Unknown => {}
         }