From e0832f8e184a33c826e712cee945f6a0b8ca8b35 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 7 Aug 2024 13:26:29 +0000 Subject: [PATCH] refactor(minifier): use `oxc_traverse` for AST passes (#4725) --- Cargo.lock | 1 + crates/oxc_minifier/Cargo.toml | 1 + .../oxc_minifier/src/ast_passes/collapse.rs | 17 ++-- .../src/ast_passes/fold_constants.rs | 18 ++-- crates/oxc_minifier/src/ast_passes/mod.rs | 13 +++ .../src/ast_passes/remove_dead_code.rs | 20 ++-- .../src/ast_passes/remove_syntax.rs | 25 ++--- .../ast_passes/substitute_alternate_syntax.rs | 93 +++++++++++++------ crates/oxc_minifier/src/compressor.rs | 56 +++++++---- crates/oxc_minifier/src/lib.rs | 2 +- crates/oxc_traverse/src/context/mod.rs | 2 +- crates/oxc_traverse/src/lib.rs | 12 ++- 12 files changed, 164 insertions(+), 96 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 579897c64..f98e867f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1591,6 +1591,7 @@ dependencies = [ "oxc_semantic", "oxc_span", "oxc_syntax", + "oxc_traverse", "pico-args", ] diff --git a/crates/oxc_minifier/Cargo.toml b/crates/oxc_minifier/Cargo.toml index c6decb219..495a24541 100644 --- a/crates/oxc_minifier/Cargo.toml +++ b/crates/oxc_minifier/Cargo.toml @@ -30,6 +30,7 @@ oxc_parser = { workspace = true } oxc_diagnostics = { workspace = true } oxc_codegen = { workspace = true } oxc_mangler = { workspace = true } +oxc_traverse = { workspace = true } num-bigint = { workspace = true } num-traits = { workspace = true } diff --git a/crates/oxc_minifier/src/ast_passes/collapse.rs b/crates/oxc_minifier/src/ast_passes/collapse.rs index 48f5fdb6a..b59a3814b 100644 --- a/crates/oxc_minifier/src/ast_passes/collapse.rs +++ b/crates/oxc_minifier/src/ast_passes/collapse.rs @@ -1,7 +1,8 @@ use oxc_allocator::Vec; -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut}; +use oxc_ast::{ast::*, AstBuilder}; +use oxc_traverse::{Traverse, TraverseCtx}; -use crate::CompressOptions; +use crate::{CompressOptions, CompressorPass}; /// Collapse variable declarations (TODO: and assignments). /// @@ -12,13 +13,13 @@ pub struct Collapse<'a> { options: CompressOptions, } -impl<'a> VisitMut<'a> for Collapse<'a> { - fn visit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>) { +impl<'a> CompressorPass<'a> for Collapse<'a> {} + +impl<'a> Traverse<'a> for Collapse<'a> { + fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) { if self.options.join_vars { self.join_vars(stmts); } - - walk_mut::walk_statements(self, stmts); } } @@ -27,10 +28,6 @@ impl<'a> Collapse<'a> { Self { ast, options } } - pub fn build(&mut self, program: &mut Program<'a>) { - self.visit_program(program); - } - /// Join consecutive var statements fn join_vars(&mut self, stmts: &mut Vec<'a, Statement<'a>>) { // Collect all the consecutive ranges that contain joinable vars. diff --git a/crates/oxc_minifier/src/ast_passes/fold_constants.rs b/crates/oxc_minifier/src/ast_passes/fold_constants.rs index 4c7f5e661..04bb356b3 100644 --- a/crates/oxc_minifier/src/ast_passes/fold_constants.rs +++ b/crates/oxc_minifier/src/ast_passes/fold_constants.rs @@ -6,12 +6,13 @@ use std::{cmp::Ordering, mem}; use num_bigint::BigInt; -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, Visit, VisitMut}; +use oxc_ast::{ast::*, AstBuilder, Visit}; use oxc_span::{GetSpan, Span, SPAN}; use oxc_syntax::{ number::NumberBase, operator::{BinaryOperator, LogicalOperator, UnaryOperator}, }; +use oxc_traverse::{Traverse, TraverseCtx}; use crate::{ ast_util::{ @@ -22,6 +23,7 @@ use crate::{ keep_var::KeepVar, tri::Tri, ty::Ty, + CompressorPass, }; pub struct FoldConstants<'a> { @@ -29,14 +31,14 @@ pub struct FoldConstants<'a> { evaluate: bool, } -impl<'a> VisitMut<'a> for FoldConstants<'a> { - fn visit_statement(&mut self, stmt: &mut Statement<'a>) { - walk_mut::walk_statement(self, stmt); +impl<'a> CompressorPass<'a> for FoldConstants<'a> {} + +impl<'a> Traverse<'a> for FoldConstants<'a> { + fn exit_statement(&mut self, stmt: &mut Statement<'a>, _ctx: &mut TraverseCtx<'a>) { self.fold_condition(stmt); } - fn visit_expression(&mut self, expr: &mut Expression<'a>) { - walk_mut::walk_expression(self, expr); + fn exit_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) { self.fold_expression(expr); self.fold_conditional_expression(expr); } @@ -52,10 +54,6 @@ impl<'a> FoldConstants<'a> { self } - pub fn build(&mut self, program: &mut Program<'a>) { - self.visit_program(program); - } - fn fold_expression_and_get_boolean_value(&mut self, expr: &mut Expression<'a>) -> Option { self.fold_expression(expr); get_boolean_value(expr) diff --git a/crates/oxc_minifier/src/ast_passes/mod.rs b/crates/oxc_minifier/src/ast_passes/mod.rs index 62d57e564..20d29dde1 100644 --- a/crates/oxc_minifier/src/ast_passes/mod.rs +++ b/crates/oxc_minifier/src/ast_passes/mod.rs @@ -9,3 +9,16 @@ pub use fold_constants::FoldConstants; pub use remove_dead_code::RemoveDeadCode; pub use remove_syntax::RemoveSyntax; pub use substitute_alternate_syntax::SubstituteAlternateSyntax; + +use oxc_ast::ast::Program; +use oxc_traverse::{walk_program, Traverse, TraverseCtx}; + +pub trait CompressorPass<'a> { + fn build(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) + where + Self: Traverse<'a>, + Self: Sized, + { + walk_program(self, program, ctx); + } +} diff --git a/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs index b700bbaf2..a7aaacdf3 100644 --- a/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs @@ -1,7 +1,8 @@ use oxc_allocator::Vec; -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, Visit, VisitMut}; +use oxc_ast::{ast::*, AstBuilder, Visit}; +use oxc_traverse::{Traverse, TraverseCtx}; -use crate::keep_var::KeepVar; +use crate::{keep_var::KeepVar, CompressorPass}; /// Remove Dead Code from the AST. /// @@ -12,15 +13,12 @@ pub struct RemoveDeadCode<'a> { ast: AstBuilder<'a>, } -impl<'a> VisitMut<'a> for RemoveDeadCode<'a> { - fn visit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>) { +impl<'a> CompressorPass<'a> for RemoveDeadCode<'a> {} + +impl<'a> Traverse<'a> for RemoveDeadCode<'a> { + fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) { stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_))); self.dead_code_elimination(stmts); - walk_mut::walk_statements(self, stmts); - } - - fn visit_expression(&mut self, expr: &mut Expression<'a>) { - walk_mut::walk_expression(self, expr); } } @@ -29,10 +27,6 @@ impl<'a> RemoveDeadCode<'a> { Self { ast } } - pub fn build(&mut self, program: &mut Program<'a>) { - self.visit_program(program); - } - /// Removes dead code thats comes after `return` statements after inlining `if` statements fn dead_code_elimination(&mut self, stmts: &mut Vec<'a, Statement<'a>>) { // Remove code after `return` and `throw` statements diff --git a/crates/oxc_minifier/src/ast_passes/remove_syntax.rs b/crates/oxc_minifier/src/ast_passes/remove_syntax.rs index 019da99db..abe22fbe2 100644 --- a/crates/oxc_minifier/src/ast_passes/remove_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/remove_syntax.rs @@ -1,7 +1,8 @@ use oxc_allocator::Vec; -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut}; +use oxc_ast::{ast::*, AstBuilder}; +use oxc_traverse::{Traverse, TraverseCtx}; -use crate::CompressOptions; +use crate::{CompressOptions, CompressorPass}; /// Remove syntax from the AST. /// @@ -13,23 +14,27 @@ pub struct RemoveSyntax<'a> { options: CompressOptions, } -impl<'a> VisitMut<'a> for RemoveSyntax<'a> { - fn visit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>) { +impl<'a> CompressorPass<'a> for RemoveSyntax<'a> {} + +impl<'a> Traverse<'a> for RemoveSyntax<'a> { + fn enter_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) { stmts.retain(|stmt| { !(matches!(stmt, Statement::EmptyStatement(_)) || self.drop_debugger(stmt) || self.drop_console(stmt)) }); - walk_mut::walk_statements(self, stmts); } - fn visit_expression(&mut self, expr: &mut Expression<'a>) { + fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) { self.strip_parenthesized_expression(expr); self.compress_console(expr); - walk_mut::walk_expression(self, expr); } - fn visit_arrow_function_expression(&mut self, expr: &mut ArrowFunctionExpression<'a>) { + fn exit_arrow_function_expression( + &mut self, + expr: &mut ArrowFunctionExpression<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { self.recover_arrow_expression_after_drop_console(expr); } } @@ -39,10 +44,6 @@ impl<'a> RemoveSyntax<'a> { Self { ast, options } } - pub fn build(&mut self, program: &mut Program<'a>) { - self.visit_program(program); - } - fn strip_parenthesized_expression(&self, expr: &mut Expression<'a>) { if let Expression::ParenthesizedExpression(paren_expr) = expr { *expr = self.ast.move_expression(&mut paren_expr.expression); diff --git a/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs index 9d1cb9974..f62916b0f 100644 --- a/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs @@ -1,11 +1,12 @@ -use oxc_ast::{ast::*, visit::walk_mut, AstBuilder, VisitMut}; +use oxc_ast::{ast::*, AstBuilder}; use oxc_span::SPAN; use oxc_syntax::{ number::NumberBase, operator::{BinaryOperator, UnaryOperator}, }; +use oxc_traverse::{Ancestor, Traverse, TraverseCtx}; -use crate::CompressOptions; +use crate::{CompressOptions, CompressorPass}; /// A peephole optimization that minimizes code by simplifying conditional /// expressions, replacing IFs with HOOKs, replacing object constructors @@ -13,52 +14,80 @@ use crate::CompressOptions; pub struct SubstituteAlternateSyntax<'a> { ast: AstBuilder<'a>, options: CompressOptions, + in_define_export: bool, } -impl<'a> VisitMut<'a> for SubstituteAlternateSyntax<'a> { - fn visit_statement(&mut self, stmt: &mut Statement<'a>) { +impl<'a> CompressorPass<'a> for SubstituteAlternateSyntax<'a> {} + +impl<'a> Traverse<'a> for SubstituteAlternateSyntax<'a> { + fn enter_statement(&mut self, stmt: &mut Statement<'a>, _ctx: &mut TraverseCtx<'a>) { self.compress_block(stmt); // self.compress_while(stmt); - walk_mut::walk_statement(self, stmt); } - fn visit_return_statement(&mut self, stmt: &mut ReturnStatement<'a>) { - walk_mut::walk_return_statement(self, stmt); + fn exit_return_statement( + &mut self, + stmt: &mut ReturnStatement<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { // We may fold `void 1` to `void 0`, so compress it after visiting Self::compress_return_statement(stmt); } - fn visit_variable_declaration(&mut self, decl: &mut VariableDeclaration<'a>) { + fn enter_variable_declaration( + &mut self, + decl: &mut VariableDeclaration<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { for declarator in decl.declarations.iter_mut() { - self.visit_variable_declarator(declarator); Self::compress_variable_declarator(declarator); } } - fn visit_expression(&mut self, expr: &mut Expression<'a>) { - // Bail cjs `Object.defineProperty(exports, ...)` - if Self::is_object_define_property_exports(expr) { - return; + /// Set `in_define_export` flag if this is a top-level statement of form: + /// ```js + /// Object.defineProperty(exports, 'Foo', { + /// enumerable: true, + /// get: function() { return Foo_1.Foo; } + /// }); + /// ``` + fn enter_call_expression( + &mut self, + call_expr: &mut CallExpression<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + // Check if this call expression is a top level `ExpressionStatement`. + // NB: 1 = global, 2 = Program, 3 = ExpressionStatement + if ctx.ancestors_depth() == 3 + && matches!(ctx.parent(), Ancestor::ExpressionStatementExpression(_)) + && Self::is_object_define_property_exports(call_expr) + { + self.in_define_export = true; } - walk_mut::walk_expression(self, expr); + } + + fn exit_call_expression(&mut self, _expr: &mut CallExpression<'a>, _ctx: &mut TraverseCtx<'a>) { + self.in_define_export = false; + } + + fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) { if !self.compress_undefined(expr) { self.compress_boolean(expr); } } - fn visit_binary_expression(&mut self, expr: &mut BinaryExpression<'a>) { - walk_mut::walk_binary_expression(self, expr); + fn exit_binary_expression( + &mut self, + expr: &mut BinaryExpression<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { self.compress_typeof_undefined(expr); } } impl<'a> SubstituteAlternateSyntax<'a> { pub fn new(ast: AstBuilder<'a>, options: CompressOptions) -> Self { - Self { ast, options } - } - - pub fn build(&mut self, program: &mut Program<'a>) { - self.visit_program(program); + Self { ast, options, in_define_export: false } } /* Utilities */ @@ -77,13 +106,22 @@ impl<'a> SubstituteAlternateSyntax<'a> { } /// Test `Object.defineProperty(exports, ...)` - fn is_object_define_property_exports(expr: &Expression<'a>) -> bool { - let Expression::CallExpression(call_expr) = expr else { return false }; + fn is_object_define_property_exports(call_expr: &CallExpression<'a>) -> bool { let Some(Argument::Identifier(ident)) = call_expr.arguments.first() else { return false }; if ident.name != "exports" { return false; } - call_expr.callee.is_specific_member_access("Object", "defineProperty") + + // Use tighter check than `call_expr.callee.is_specific_member_access("Object", "defineProperty")` + // because we're looking for `Object.defineProperty` specifically, not e.g. `Object['defineProperty']` + if let Expression::StaticMemberExpression(callee) = &call_expr.callee { + if let Expression::Identifier(id) = &callee.object { + if id.name == "Object" && callee.property.name == "defineProperty" { + return true; + } + } + } + false } /* Statements */ @@ -115,11 +153,12 @@ impl<'a> SubstituteAlternateSyntax<'a> { /* Expressions */ - /// Transforms boolean expression `true` => `!0` `false` => `!1` - /// Enabled by `compress.booleans` + /// Transforms boolean expression `true` => `!0` `false` => `!1`. + /// Enabled by `compress.booleans`. + /// Do not compress `true` in `Object.defineProperty(exports, 'Foo', {enumerable: true, ...})`. fn compress_boolean(&mut self, expr: &mut Expression<'a>) -> bool { let Expression::BooleanLiteral(lit) = expr else { return false }; - if self.options.booleans { + if self.options.booleans && !self.in_define_export { let num = self.ast.expression_numeric_literal( SPAN, if lit.value { 0.0 } else { 1.0 }, diff --git a/crates/oxc_minifier/src/compressor.rs b/crates/oxc_minifier/src/compressor.rs index c3dd921fd..5fddbaee1 100644 --- a/crates/oxc_minifier/src/compressor.rs +++ b/crates/oxc_minifier/src/compressor.rs @@ -1,61 +1,77 @@ use oxc_allocator::Allocator; -use oxc_ast::{ast::*, AstBuilder}; +use oxc_ast::ast::*; +use oxc_semantic::{ScopeTree, SemanticBuilder, SymbolTable}; +use oxc_traverse::TraverseCtx; use crate::{ ast_passes::{ Collapse, FoldConstants, RemoveDeadCode, RemoveSyntax, SubstituteAlternateSyntax, }, - CompressOptions, + CompressOptions, CompressorPass, }; pub struct Compressor<'a> { - ast: AstBuilder<'a>, + allocator: &'a Allocator, options: CompressOptions, } impl<'a> Compressor<'a> { pub fn new(allocator: &'a Allocator, options: CompressOptions) -> Self { - let ast = AstBuilder::new(allocator); - Self { ast, options } + Self { allocator, options } } pub fn build(self, program: &mut Program<'a>) { + let (symbols, scopes) = SemanticBuilder::new("", program.source_type) + .build(program) + .semantic + .into_symbol_table_and_scope_tree(); + self.build_with_symbols_and_scopes(symbols, scopes, program); + } + + pub fn build_with_symbols_and_scopes( + self, + symbols: SymbolTable, + scopes: ScopeTree, + program: &mut Program<'a>, + ) { + let mut ctx = TraverseCtx::new(scopes, symbols, self.allocator); + // Run separate AST passes // TODO: inline variables - self.remove_syntax(program); - self.fold_constants(program); - self.remove_dead_code(program); + self.remove_syntax(program, &mut ctx); + self.fold_constants(program, &mut ctx); + self.remove_dead_code(program, &mut ctx); // TODO: StatementFusion - self.substitute_alternate_syntax(program); - self.collapse(program); + self.substitute_alternate_syntax(program, &mut ctx); + self.collapse(program, &mut ctx); } - fn remove_syntax(&self, program: &mut Program<'a>) { + fn remove_syntax(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { if self.options.remove_syntax { - RemoveSyntax::new(self.ast, self.options).build(program); + RemoveSyntax::new(ctx.ast, self.options).build(program, ctx); } } - fn fold_constants(&self, program: &mut Program<'a>) { + fn fold_constants(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { if self.options.fold_constants { - FoldConstants::new(self.ast).with_evaluate(self.options.evaluate).build(program); + FoldConstants::new(ctx.ast).with_evaluate(self.options.evaluate).build(program, ctx); } } - fn substitute_alternate_syntax(&self, program: &mut Program<'a>) { + fn substitute_alternate_syntax(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { if self.options.substitute_alternate_syntax { - SubstituteAlternateSyntax::new(self.ast, self.options).build(program); + SubstituteAlternateSyntax::new(ctx.ast, self.options).build(program, ctx); } } - fn remove_dead_code(&self, program: &mut Program<'a>) { + fn remove_dead_code(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { if self.options.remove_dead_code { - RemoveDeadCode::new(self.ast).build(program); + RemoveDeadCode::new(ctx.ast).build(program, ctx); } } - fn collapse(&self, program: &mut Program<'a>) { + fn collapse(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { if self.options.collapse { - Collapse::new(self.ast, self.options).build(program); + Collapse::new(ctx.ast, self.options).build(program, ctx); } } } diff --git a/crates/oxc_minifier/src/lib.rs b/crates/oxc_minifier/src/lib.rs index 25de7d70e..148df0c46 100644 --- a/crates/oxc_minifier/src/lib.rs +++ b/crates/oxc_minifier/src/lib.rs @@ -16,7 +16,7 @@ use oxc_ast::ast::Program; use oxc_mangler::{Mangler, ManglerBuilder}; pub use crate::{ - ast_passes::{RemoveDeadCode, RemoveSyntax}, + ast_passes::{CompressorPass, RemoveDeadCode, RemoveSyntax}, compressor::Compressor, options::CompressOptions, plugins::{ReplaceGlobalDefines, ReplaceGlobalDefinesConfig}, diff --git a/crates/oxc_traverse/src/context/mod.rs b/crates/oxc_traverse/src/context/mod.rs index 38ec44824..f99e57bb1 100644 --- a/crates/oxc_traverse/src/context/mod.rs +++ b/crates/oxc_traverse/src/context/mod.rs @@ -112,7 +112,7 @@ pub struct TraverseCtx<'a> { // Public methods impl<'a> TraverseCtx<'a> { /// Create new traversal context. - pub(crate) fn new(scopes: ScopeTree, symbols: SymbolTable, allocator: &'a Allocator) -> Self { + pub fn new(scopes: ScopeTree, symbols: SymbolTable, allocator: &'a Allocator) -> Self { let ancestry = TraverseAncestry::new(); let scoping = TraverseScoping::new(scopes, symbols); let ast = AstBuilder::new(allocator); diff --git a/crates/oxc_traverse/src/lib.rs b/crates/oxc_traverse/src/lib.rs index 5dbd1d12f..e7b8a75f0 100644 --- a/crates/oxc_traverse/src/lib.rs +++ b/crates/oxc_traverse/src/lib.rs @@ -147,8 +147,16 @@ pub fn traverse_mut<'a, Tr: Traverse<'a>>( scopes: ScopeTree, ) -> (SymbolTable, ScopeTree) { let mut ctx = TraverseCtx::new(scopes, symbols, allocator); - // SAFETY: Walk functions are constructed to avoid unsoundness - unsafe { walk::walk_program(traverser, program as *mut Program, &mut ctx) }; + walk_program(traverser, program, &mut ctx); debug_assert!(ctx.ancestors_depth() == 1); ctx.scoping.into_symbol_table_and_scope_tree() } + +pub fn walk_program<'a, Tr: Traverse<'a>>( + traverser: &mut Tr, + program: &mut Program<'a>, + ctx: &mut TraverseCtx<'a>, +) { + // SAFETY: Walk functions are constructed to avoid unsoundness + unsafe { walk::walk_program(traverser, program as *mut Program, ctx) }; +}