diff --git a/crates/swc_ecma_minifier/src/compress/optimize/collapse_vars.rs b/crates/swc_ecma_minifier/src/compress/optimize/collapse_vars.rs index 7c5dbd4f232..2ca785ba47e 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/collapse_vars.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/collapse_vars.rs @@ -68,7 +68,7 @@ where left.id.span.ctxt ); - self.lits.insert(left.to_id(), value); + self.vars.lits.insert(left.to_id(), value); } } } diff --git a/crates/swc_ecma_minifier/src/compress/optimize/hoist_props.rs b/crates/swc_ecma_minifier/src/compress/optimize/hoist_props.rs index e6edb94a585..3d6d0408c6a 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/hoist_props.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/hoist_props.rs @@ -21,6 +21,10 @@ where } if let Pat::Ident(name) = &mut n.name { + if self.options.top_retain.contains(&name.id.sym) { + return; + } + // If a variable is initialized multiple time, we currently don't do anything // smart. if !self diff --git a/crates/swc_ecma_minifier/src/compress/optimize/iife.rs b/crates/swc_ecma_minifier/src/compress/optimize/iife.rs index 3d3b204c073..b82a0af7456 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/iife.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/iife.rs @@ -9,7 +9,10 @@ use swc_ecma_utils::{ }; use swc_ecma_visit::VisitMutWith; -use super::{util::MultiReplacer, Optimizer}; +use super::{ + util::{MultiReplacer, MultiReplacerMode}, + Optimizer, +}; use crate::{ compress::optimize::{util::Remapper, Ctx}, debug::dump, @@ -262,7 +265,7 @@ where n.visit_mut_with(&mut MultiReplacer::new( &mut vars, false, - false, + MultiReplacerMode::Normal, &mut self.changed, )); } @@ -437,18 +440,9 @@ where exprs.push(arg.expr.take()); } } - body.visit_mut_with(&mut MultiReplacer::new( - &mut self.simple_functions, - true, - true, - &mut self.changed, - )); - body.visit_mut_with(&mut MultiReplacer::new( - &mut self.vars_for_inlining, - false, - false, - &mut self.changed, - )); + if self.vars.inline_with_multi_replacer(body) { + self.changed = true; + } body.visit_mut_with(&mut Remapper { vars: remap }); exprs.push(body.take()); @@ -549,10 +543,6 @@ where } } - fn has_pending_inline_for(&self, id: &Id) -> bool { - self.lits.contains_key(id) || self.vars_for_inlining.contains_key(id) - } - fn is_return_arg_simple_enough_for_iife_eval(&self, e: &Expr) -> bool { match e { Expr::Lit(..) | Expr::Ident(..) => true, @@ -632,7 +622,7 @@ where .. }) => true, Pat::Ident(id) => { - if self.has_pending_inline_for(&id.to_id()) { + if self.vars.has_pending_inline_for(&id.to_id()) { return true; } @@ -729,18 +719,9 @@ where } } - body.visit_mut_with(&mut MultiReplacer::new( - &mut self.simple_functions, - true, - true, - &mut self.changed, - )); - body.visit_mut_with(&mut MultiReplacer::new( - &mut self.vars_for_inlining, - false, - false, - &mut self.changed, - )); + if self.vars.inline_with_multi_replacer(body) { + self.changed = true; + } body.visit_mut_with(&mut Remapper { vars: remap }); { diff --git a/crates/swc_ecma_minifier/src/compress/optimize/inline.rs b/crates/swc_ecma_minifier/src/compress/optimize/inline.rs index af4cb0bb46f..da9d5b55da5 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/inline.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/inline.rs @@ -47,7 +47,7 @@ where // We will inline if possible. if let Pat::Ident(i) = &var.name { - if i.id.sym == *"arguments" { + if i.id.sym == js_word!("arguments") { return; } if self.options.top_retain.contains(&i.id.sym) { @@ -185,7 +185,7 @@ where var.span = var.span.apply_mark(self.marks.non_top_level); } - self.lits.insert(i.to_id(), init.take()); + self.vars.lits.insert(i.to_id(), init.take()); var.name.take(); } else if self.options.inline != 0 || self.options.reduce_vars { @@ -197,7 +197,7 @@ where self.mode.store(i.to_id(), &*init); - self.lits.insert(i.to_id(), init.clone()); + self.vars.lits.insert(i.to_id(), init.clone()); } return; } @@ -294,7 +294,7 @@ where i.id ); self.changed = true; - self.vars_for_inlining.insert(i.to_id(), init.take()); + self.vars.vars_for_inlining.insert(i.to_id(), init.take()); } } } @@ -479,7 +479,7 @@ where return; } - self.simple_functions.insert( + self.vars.simple_functions.insert( i.to_id(), match decl { Decl::Fn(f) => Box::new(Expr::Fn(FnExpr { @@ -559,7 +559,7 @@ where } }; - self.vars_for_inlining.insert(i.to_id(), e); + self.vars.vars_for_inlining.insert(i.to_id(), e); } else { if cfg!(feature = "debug") { tracing::trace!("inline: [x] Usage: {:?}", usage); @@ -573,11 +573,12 @@ where if let Expr::Ident(i) = e { // if let Some(value) = self + .vars .lits .get(&i.to_id()) .or_else(|| { if self.ctx.is_callee { - self.simple_functions.get(&i.to_id()) + self.vars.simple_functions.get(&i.to_id()) } else { None } @@ -615,7 +616,7 @@ where } // Check without cloning - if let Some(value) = self.vars_for_inlining.get(&i.to_id()) { + if let Some(value) = self.vars.vars_for_inlining.get(&i.to_id()) { if self.ctx.is_exact_lhs_of_assign && !is_valid_for_lhs(value) { return; } @@ -627,7 +628,7 @@ where } } - if let Some(value) = self.vars_for_inlining.remove(&i.to_id()) { + if let Some(value) = self.vars.vars_for_inlining.remove(&i.to_id()) { self.changed = true; tracing::debug!( "inline: Replacing '{}{:?}' with an expression", diff --git a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs index 940ca1b7151..af595dfcfbb 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs @@ -18,7 +18,10 @@ use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith, VisitWith}; use tracing::{span, Level}; use Value::Known; -use self::{unused::PropertyAccessOpts, util::MultiReplacer}; +use self::{ + unused::PropertyAccessOpts, + util::{MultiReplacer, MultiReplacerMode}, +}; use super::util::{drop_invalid_stmts, is_fine_for_if_cons}; use crate::{ analyzer::{ProgramData, UsageAnalyzer}, @@ -71,9 +74,7 @@ where options, prepend_stmts: Default::default(), append_stmts: Default::default(), - lits: Default::default(), - simple_functions: Default::default(), - vars_for_inlining: Default::default(), + vars: Default::default(), vars_for_prop_hoisting: Default::default(), simple_props: Default::default(), _simple_array_values: Default::default(), @@ -192,16 +193,7 @@ struct Optimizer<'a, M> { /// Statements appended to the current statement. append_stmts: SynthesizedStmts, - /// Cheap to clone. - /// - /// Used for inlining. - lits: AHashMap>, - - /// Used for copying functions. - /// - /// We use this to distinguish [Callee::Expr] from other [Expr]s. - simple_functions: FxHashMap>, - vars_for_inlining: FxHashMap>, + vars: Vars, /// Used for `hoist_props`. vars_for_prop_hoisting: FxHashMap>, @@ -228,6 +220,48 @@ struct Optimizer<'a, M> { functions: FxHashMap, } +#[derive(Default)] +struct Vars { + /// Cheap to clone. + /// + /// Used for inlining. + lits: AHashMap>, + + /// Used for copying functions. + /// + /// We use this to distinguish [Callee::Expr] from other [Expr]s. + simple_functions: FxHashMap>, + vars_for_inlining: FxHashMap>, +} + +impl Vars { + fn has_pending_inline_for(&self, id: &Id) -> bool { + self.lits.contains_key(id) || self.vars_for_inlining.contains_key(id) + } + + /// Returns true if something is changed. + fn inline_with_multi_replacer(&mut self, n: &mut N) -> bool + where + N: for<'aa> VisitMutWith>, + { + let mut changed = false; + n.visit_mut_with(&mut MultiReplacer::new( + &mut self.simple_functions, + true, + MultiReplacerMode::OnlyCallee, + &mut changed, + )); + n.visit_mut_with(&mut MultiReplacer::new( + &mut self.vars_for_inlining, + false, + MultiReplacerMode::Normal, + &mut changed, + )); + + changed + } +} + impl Repeated for Optimizer<'_, M> { fn changed(&self) -> bool { self.changed @@ -1935,8 +1969,8 @@ where if !self.options.negate_iife { // I(kdy1) don't know why this check if required, but there are two test cases // with `options.expressions` as only difference. - if !self.options.expr { - need_ignore_return_value |= self.negate_iife_in_cond(&mut n.expr); + if !self.options.expr && self.negate_iife_in_cond(&mut n.expr) { + need_ignore_return_value = true } } @@ -2229,18 +2263,9 @@ where }; self.with_ctx(ctx).handle_stmt_likes(stmts); - stmts.visit_mut_with(&mut MultiReplacer::new( - &mut self.simple_functions, - true, - true, - &mut self.changed, - )); - stmts.visit_mut_with(&mut MultiReplacer::new( - &mut self.vars_for_inlining, - false, - false, - &mut self.changed, - )); + if self.vars.inline_with_multi_replacer(stmts) { + self.changed = true; + } drop_invalid_stmts(stmts); } @@ -2313,8 +2338,7 @@ where n.visit_mut_children_with(self); if let Prop::Shorthand(i) = n { - if self.lits.contains_key(&i.to_id()) || self.vars_for_inlining.contains_key(&i.to_id()) - { + if self.vars.has_pending_inline_for(&i.to_id()) { let mut e = Box::new(Expr::Ident(i.clone())); e.visit_mut_with(self); diff --git a/crates/swc_ecma_minifier/src/compress/optimize/util.rs b/crates/swc_ecma_minifier/src/compress/optimize/util.rs index 9d3001f8987..e11b7a052be 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/util.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/util.rs @@ -190,23 +190,31 @@ pub(crate) struct MultiReplacer<'a> { vars: &'a mut FxHashMap>, changed: bool, clone: bool, - only_callee: bool, + mode: MultiReplacerMode, worked: &'a mut bool, } +#[repr(u8)] +#[derive(Debug, Clone, Copy)] + +pub enum MultiReplacerMode { + Normal, + OnlyCallee, +} + impl<'a> MultiReplacer<'a> { /// `worked` will be changed to `true` if any replacement is done pub fn new( vars: &'a mut FxHashMap>, clone: bool, - only_callee: bool, + mode: MultiReplacerMode, worked: &'a mut bool, ) -> Self { MultiReplacer { vars, changed: false, clone, - only_callee, + mode, worked, } } @@ -226,7 +234,7 @@ impl VisitMut for MultiReplacer<'_> { fn visit_mut_callee(&mut self, e: &mut Callee) { e.visit_mut_children_with(self); - if self.only_callee { + if matches!(self.mode, MultiReplacerMode::OnlyCallee) { if let Callee::Expr(e) = e { if let Expr::Ident(i) = &**e { if let Some(new) = self.var(&i.to_id()) { @@ -244,7 +252,7 @@ impl VisitMut for MultiReplacer<'_> { fn visit_mut_expr(&mut self, e: &mut Expr) { e.visit_mut_children_with(self); - if !self.only_callee { + if matches!(self.mode, MultiReplacerMode::Normal) { if let Expr::Ident(i) = e { if let Some(new) = self.var(&i.to_id()) { debug!("multi-replacer: Replaced `{}`", i); diff --git a/crates/swc_ecma_minifier/tests/TODO.txt b/crates/swc_ecma_minifier/tests/TODO.txt index 3017c66abcc..ca9f3623f3e 100644 --- a/crates/swc_ecma_minifier/tests/TODO.txt +++ b/crates/swc_ecma_minifier/tests/TODO.txt @@ -334,7 +334,6 @@ hoist_props/contains_this_2/input.js hoist_props/direct_access_1/input.js hoist_props/hoist_class/input.js hoist_props/hoist_class_with_new/input.js -hoist_props/issue_2473_3/input.js hoist_props/issue_2473_4/input.js hoist_props/issue_2508_1/input.js hoist_props/issue_2508_2/input.js diff --git a/crates/swc_ecma_minifier/tests/golden.txt b/crates/swc_ecma_minifier/tests/golden.txt index 9831fe9ae28..13920acf9dd 100644 --- a/crates/swc_ecma_minifier/tests/golden.txt +++ b/crates/swc_ecma_minifier/tests/golden.txt @@ -629,6 +629,7 @@ hoist_props/issue_2377_3/input.js hoist_props/issue_2462/input.js hoist_props/issue_2473_1/input.js hoist_props/issue_2473_2/input.js +hoist_props/issue_2473_3/input.js hoist_props/issue_2508_3/input.js hoist_props/issue_2508_4/input.js hoist_props/issue_3071_1/input.js