refactor(es/minifier): Merge code for multi-replacer (#4269)

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);
         }
     }
 }

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

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(
+                        if self.vars.inline_with_multi_replacer(body) {
-                            &mut self.simple_functions,
+                            self.changed = true;
-                            true,
+                        }
-                            true,
-                            &mut self.changed,
-                        ));
-                        body.visit_mut_with(&mut MultiReplacer::new(
-                            &mut self.vars_for_inlining,
-                            false,
-                            false,
-                            &mut self.changed,
-                        ));
                         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(
+        if self.vars.inline_with_multi_replacer(body) {
-            &mut self.simple_functions,
+            self.changed = true;
-            true,
+        }
-            true,
-            &mut self.changed,
-        ));
-        body.visit_mut_with(&mut MultiReplacer::new(
-            &mut self.vars_for_inlining,
-            false,
-            false,
-            &mut self.changed,
-        ));
         body.visit_mut_with(&mut Remapper { vars: remap });
 
         {

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",

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(),
+        vars: Default::default(),
-        simple_functions: Default::default(),
-        vars_for_inlining: 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.
+    vars: Vars,
-    ///
-    /// Used for inlining.
-    lits: AHashMap<Id, Box<Expr>>,
-
-    /// Used for copying functions.
-    ///
-    /// We use this to distinguish [Callee::Expr] from other [Expr]s.
-    simple_functions: FxHashMap<Id, Box<Expr>>,
-    vars_for_inlining: FxHashMap<Id, Box<Expr>>,
 
     /// Used for `hoist_props`.
     vars_for_prop_hoisting: FxHashMap<Id, Box<Expr>>,
@@ -228,6 +220,48 @@ struct Optimizer<'a, M> {
     functions: FxHashMap<Id, FnMetadata>,
 }
 
+#[derive(Default)]
+struct Vars {
+    /// Cheap to clone.
+    ///
+    /// Used for inlining.
+    lits: AHashMap<Id, Box<Expr>>,
+
+    /// Used for copying functions.
+    ///
+    /// We use this to distinguish [Callee::Expr] from other [Expr]s.
+    simple_functions: FxHashMap<Id, Box<Expr>>,
+    vars_for_inlining: FxHashMap<Id, Box<Expr>>,
+}
+
+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<N>(&mut self, n: &mut N) -> bool
+    where
+        N: for<'aa> VisitMutWith<MultiReplacer<'aa>>,
+    {
+        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<M> 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 {
+            if !self.options.expr && self.negate_iife_in_cond(&mut n.expr) {
-                need_ignore_return_value |= 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(
+        if self.vars.inline_with_multi_replacer(stmts) {
-            &mut self.simple_functions,
+            self.changed = true;
-            true,
+        }
-            true,
-            &mut self.changed,
-        ));
-        stmts.visit_mut_with(&mut MultiReplacer::new(
-            &mut self.vars_for_inlining,
-            false,
-            false,
-            &mut self.changed,
-        ));
 
         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);
 

crates/swc_ecma_minifier/src/compress/optimize/util.rs

@@ -190,23 +190,31 @@ pub(crate) struct MultiReplacer<'a> {
     vars: &'a mut FxHashMap<Id, Box<Expr>>,
     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<Id, Box<Expr>>,
         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);

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

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