fix(es/minifier): Prepend/append correctly (#3367)

swc_ecma_minifier: - Add some assertions for injections. - Fix prepend/append logic of statements.
swc-project · Jan 26, 2022 · 703972d · 703972d · github-actions · Jan 26, 2022
1 parent 14e87d3
commit 703972d
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 22 deletions.
diff --git a/crates/swc/tests/tsc-references/arrowFunctionExpressions_es5.2.minified.js b/crates/swc/tests/tsc-references/arrowFunctionExpressions_es5.2.minified.js
@@ -24,10 +24,6 @@ var MyClass = function() {
         }
     ], _defineProperties((Constructor = MyClass).prototype, protoProps), staticProps && _defineProperties(Constructor, staticProps), MyClass;
 }();
-(m = 3, function() {
-    return function(n) {
-        return 3 + n;
-    };
-})()(4), (function() {
+(function() {
     return 0;
 })().toExponential();
diff --git a/crates/swc/tests/tsc-references/exportCodeGen_es5.2.minified.js b/crates/swc/tests/tsc-references/exportCodeGen_es5.2.minified.js
@@ -1,8 +1,8 @@
-var A1, D, E, F;
+var A, D, E, F;
 function _classCallCheck(instance, Constructor) {
     if (!(instance instanceof Constructor)) throw new TypeError("Cannot call a class as a function");
 }
-(A = A1 || (A1 = {})).x = 12, (D || (D = {})).yes = function() {
+(A || (A = {})).x = 12, (D || (D = {})).yes = function() {
     return !0;
 }, (function(E1) {
     (Color = Color = E1.Color || (E1.Color = {}))[Color.Red = 0] = "Red", E1.fn = function() {};

diff --git a/.../tsc-references/genericCallToOverloadedMethodWithOverloadedArguments_es2015.2.minified.js b/.../tsc-references/genericCallToOverloadedMethodWithOverloadedArguments_es2015.2.minified.js
@@ -1,2 +1,2 @@
-var m11, m21, m31, m41, m51, m6;
-m1 = m11 || (m11 = {}), (void 0).then(testFunction), m2 = m21 || (m21 = {}), (void 0).then(testFunction), m3 = m31 || (m31 = {}), (void 0).then(testFunction), m4 = m41 || (m41 = {}), (void 0).then(testFunction), m5 = m51 || (m51 = {}), (void 0).then(testFunction), m6 || (m6 = {}), (void 0).then(testFunction);
+var m1, m2, m3, m4, m5, m6;
+m1 || (m1 = {}), (void 0).then(testFunction), m2 || (m2 = {}), (void 0).then(testFunction), m3 || (m3 = {}), (void 0).then(testFunction), m4 || (m4 = {}), (void 0).then(testFunction), m5 || (m5 = {}), (void 0).then(testFunction), m6 || (m6 = {}), (void 0).then(testFunction);
diff --git a/...sts/tsc-references/genericCallToOverloadedMethodWithOverloadedArguments_es5.2.minified.js b/...sts/tsc-references/genericCallToOverloadedMethodWithOverloadedArguments_es5.2.minified.js
@@ -1,2 +1,2 @@
-var m11, m21, m31, m41, m51, m6;
-m1 = m11 || (m11 = {}), (void 0).then(testFunction), m2 = m21 || (m21 = {}), (void 0).then(testFunction), m3 = m31 || (m31 = {}), (void 0).then(testFunction), m4 = m41 || (m41 = {}), (void 0).then(testFunction), m5 = m51 || (m51 = {}), (void 0).then(testFunction), m6 || (m6 = {}), (void 0).then(testFunction);
+var m1, m2, m3, m4, m5, m6;
+m1 || (m1 = {}), (void 0).then(testFunction), m2 || (m2 = {}), (void 0).then(testFunction), m3 || (m3 = {}), (void 0).then(testFunction), m4 || (m4 = {}), (void 0).then(testFunction), m5 || (m5 = {}), (void 0).then(testFunction), m6 || (m6 = {}), (void 0).then(testFunction);
diff --git a/crates/swc_ecma_minifier/src/compress/optimize/iife.rs b/crates/swc_ecma_minifier/src/compress/optimize/iife.rs
@@ -137,6 +137,7 @@ where
     ///     })(x);
     /// })(7);
     /// ```
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, e)))]
     pub(super) fn inline_args_of_iife(&mut self, e: &mut CallExpr) {
         if self.options.inline == 0 {
             return;
@@ -250,6 +251,7 @@ where
         }
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n, vars)))]
     pub(super) fn inline_vars_in_node<N>(&mut self, n: &mut N, vars: AHashMap<Id, Box<Expr>>)
     where
         N: VisitMutWith<Self>,
@@ -616,6 +618,9 @@ where
                 .collect::<Vec<_>>();
 
             if !vars.is_empty() {
+                if cfg!(feature = "debug") {
+                    tracing::debug!("iife: Creating variables: {:?}", vars);
+                }
                 self.prepend_stmts.push(Stmt::Decl(Decl::Var(VarDecl {
                     span: DUMMY_SP.apply_mark(self.marks.non_top_level),
                     kind: VarDeclKind::Var,

diff --git a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs
@@ -188,9 +188,9 @@ struct Optimizer<'a, M> {
     options: &'a CompressOptions,
 
     /// Statements prepended to the current statement.
-    prepend_stmts: Vec<Stmt>,
+    prepend_stmts: SynthesizedStmts,
     /// Statements appended to the current statement.
-    append_stmts: Vec<Stmt>,
+    append_stmts: SynthesizedStmts,
 
     /// Cheap to clone.
     ///
@@ -239,6 +239,7 @@ impl<M> Optimizer<'_, M>
 where
     M: Mode,
 {
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, stmts)))]
     fn handle_stmt_likes<T>(&mut self, stmts: &mut Vec<T>)
     where
         T: StmtLike + ModuleItemLike + ModuleItemExt + VisitMutWith<Self>,
@@ -251,8 +252,8 @@ where
             }
         }
         let mut use_asm = false;
-        // let prepend_stmts = self.prepend_stmts.take();
-        // let append_stmts = self.append_stmts.take();
+        let prepend_stmts = self.prepend_stmts.take();
+        let append_stmts = self.append_stmts.take();
 
         {
             let mut child_ctx = Ctx { ..self.ctx };
@@ -289,6 +290,8 @@ where
                     stmt.visit_mut_with(&mut *self.with_ctx(child_ctx));
                 }
 
+                new.extend(self.prepend_stmts.drain(..).map(T::from_stmt));
+
                 match stmt.try_into_stmt() {
                     Ok(Stmt::Block(s)) if s.span.has_mark(self.marks.fake_block) => {
                         new.extend(s.stmts.into_iter().map(T::from_stmt));
@@ -301,8 +304,7 @@ where
                     }
                 }
 
-                // new.extend(self.prepend_stmts.drain(..).map(T::from_stmt));
-                // new.extend(self.append_stmts.drain(..).map(T::from_stmt));
+                new.extend(self.append_stmts.drain(..).map(T::from_stmt));
             }
             *stmts = new;
         }
@@ -360,8 +362,8 @@ where
         drop_invalid_stmts(stmts);
 
         // debug_assert_eq!(self.prepend_stmts, vec![]);
-        // self.prepend_stmts = prepend_stmts;
-        // self.append_stmts = append_stmts;
+        self.prepend_stmts = prepend_stmts;
+        self.append_stmts = append_stmts;
     }
 
     /// `a = a + 1` => `a += 1`.
@@ -1535,6 +1537,7 @@ where
 {
     noop_visit_mut_type!();
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n,)))]
     fn visit_mut_arrow_expr(&mut self, n: &mut ArrowExpr) {
         let prepend = self.prepend_stmts.take();
 
@@ -1546,7 +1549,7 @@ where
         n.visit_mut_children_with(&mut *self.with_ctx(ctx));
 
         if !self.prepend_stmts.is_empty() {
-            let mut stmts = self.prepend_stmts.take();
+            let mut stmts = self.prepend_stmts.take().take_stmts();
             match &mut n.body {
                 BlockStmtOrExpr::BlockStmt(v) => {
                     prepend_stmts(&mut v.stmts, stmts.into_iter());
@@ -1575,6 +1578,7 @@ where
         }
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, e)))]
     fn visit_mut_assign_expr(&mut self, e: &mut AssignExpr) {
         {
             let ctx = Ctx {
@@ -1594,6 +1598,7 @@ where
         self.compress_bin_assignment_to_right(e);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_assign_pat_prop(&mut self, n: &mut AssignPatProp) {
         n.visit_mut_children_with(self);
 
@@ -1604,6 +1609,7 @@ where
         }
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_bin_expr(&mut self, n: &mut BinExpr) {
         {
             let ctx = Ctx {
@@ -1631,6 +1637,7 @@ where
         }
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_block_stmt(&mut self, n: &mut BlockStmt) {
         let ctx = Ctx {
             stmt_labelled: false,
@@ -1654,6 +1661,7 @@ where
         }
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, e)))]
     fn visit_mut_call_expr(&mut self, e: &mut CallExpr) {
         let is_this_undefined = match &e.callee {
             Callee::Super(_) | Callee::Import(_) => false,
@@ -1703,6 +1711,7 @@ where
         self.inline_args_of_iife(e);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_class(&mut self, n: &mut Class) {
         n.decorators.visit_mut_with(self);
 
@@ -1731,6 +1740,7 @@ where
         e.visit_mut_children_with(self);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, decl)))]
     fn visit_mut_decl(&mut self, decl: &mut Decl) {
         decl.visit_mut_children_with(self);
 
@@ -1739,6 +1749,7 @@ where
         self.store_decl_for_inlining(decl);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_default_decl(&mut self, n: &mut DefaultDecl) {
         match n {
             DefaultDecl::Class(_) => {}
@@ -1753,6 +1764,7 @@ where
         n.visit_mut_children_with(self);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_export_decl(&mut self, n: &mut ExportDecl) {
         if let Decl::Fn(f) = &mut n.decl {
             // I don't know why, but terser removes parameters from an exported function if
@@ -1777,6 +1789,7 @@ where
         n.visit_mut_children_with(&mut *self.with_ctx(ctx));
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, e)))]
     fn visit_mut_expr(&mut self, e: &mut Expr) {
         let ctx = Ctx {
             is_exported: false,
@@ -1848,6 +1861,7 @@ where
         }
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_expr_stmt(&mut self, n: &mut ExprStmt) {
         n.visit_mut_children_with(self);
 
@@ -1977,6 +1991,7 @@ where
         self.with_ctx(ctx).drop_if_break(s);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, n)))]
     fn visit_mut_function(&mut self, n: &mut Function) {
         {
             let ctx = Ctx {
@@ -2267,6 +2282,7 @@ where
         self.lift_seqs_of_assign(n);
     }
 
+    #[cfg_attr(feature = "debug", tracing::instrument(skip(self, s)))]
     fn visit_mut_stmt(&mut self, s: &mut Stmt) {
         let old_prepend = self.prepend_stmts.take();
         let old_append = self.append_stmts.take();
@@ -2338,10 +2354,10 @@ where
                 span: span.apply_mark(self.marks.fake_block),
                 stmts: self
                     .prepend_stmts
-                    .take()
+                    .take_stmts()
                     .into_iter()
                     .chain(once(s.take()))
-                    .chain(self.append_stmts.take().into_iter())
+                    .chain(self.append_stmts.take_stmts().into_iter())
                     .filter(|s| match s {
                         Stmt::Empty(..) => false,
                         Stmt::Decl(Decl::Var(v)) => !v.decls.is_empty(),
@@ -2358,6 +2374,8 @@ where
         self.prepend_stmts = old_prepend;
         self.append_stmts = old_append;
 
+        let len = self.prepend_stmts.len();
+
         if cfg!(feature = "debug") && self.debug_infinite_loop {
             let text = dump(&*s, false);
 
@@ -2366,6 +2384,8 @@ where
             }
         }
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         if let Stmt::Expr(ExprStmt { expr, .. }) = s {
             if is_pure_undefined(expr) {
                 *s = Stmt::Empty(EmptyStmt { span: DUMMY_SP });
@@ -2402,6 +2422,8 @@ where
             }
         }
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         match s {
             // We use var decl with no declarator to indicate we dropped an decl.
             Stmt::Decl(Decl::Var(VarDecl { decls, .. })) if decls.is_empty() => {
@@ -2411,20 +2433,34 @@ where
             _ => {}
         }
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         if cfg!(debug_assertions) {
             s.visit_with(&mut AssertValid);
         }
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         self.compress_if_without_alt(s);
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         self.compress_if_stmt_as_cond(s);
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         self.compress_if_stmt_as_expr(s);
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         self.optimize_const_switches(s);
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         self.optimize_switches(s);
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         if cfg!(feature = "debug") && self.debug_infinite_loop {
             let text = dump(&*s, false);
 
@@ -2433,9 +2469,13 @@ where
             }
         }
 
+        debug_assert_eq!(self.prepend_stmts.len(), len);
+
         if cfg!(debug_assertions) {
             s.visit_with(&mut AssertValid);
         }
+
+        debug_assert_eq!(self.prepend_stmts.len(), len);
     }
 
     fn visit_mut_stmts(&mut self, stmts: &mut Vec<Stmt>) {
@@ -2743,3 +2783,40 @@ fn is_left_access_to_arguments(l: &PatOrExpr) -> bool {
         },
     }
 }
+
+#[derive(Debug, Default, PartialEq)]
+struct SynthesizedStmts(Vec<Stmt>);
+
+impl SynthesizedStmts {
+    fn take_stmts(&mut self) -> Vec<Stmt> {
+        take(&mut self.0)
+    }
+}
+
+impl std::ops::Deref for SynthesizedStmts {
+    type Target = Vec<Stmt>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl std::ops::DerefMut for SynthesizedStmts {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl Take for SynthesizedStmts {
+    fn dummy() -> Self {
+        Self(Take::dummy())
+    }
+}
+
+impl Drop for SynthesizedStmts {
+    fn drop(&mut self) {
+        if !self.0.is_empty() {
+            panic!("We should not drop synthesized stmts");
+        }
+    }
+}
Benchmark suite	Current: `703972d`	Previous: `ac2bb9b`	Ratio
`full_es2015`	`227227449` ns/iter (`± 21785755`)	`168508742` ns/iter (`± 14377732`)	`1.35`
`full_es2016`	`246081883` ns/iter (`± 17362878`)	`174846463` ns/iter (`± 18316763`)	`1.41`
`full_es2017`	`242279023` ns/iter (`± 16094027`)	`170760902` ns/iter (`± 16472025`)	`1.42`
`full_es2018`	`240469738` ns/iter (`± 20441378`)	`171273620` ns/iter (`± 17291228`)	`1.40`
`full_es2019`	`241294783` ns/iter (`± 19663616`)	`169813817` ns/iter (`± 19731378`)	`1.42`
`full_es2020`	`204232444` ns/iter (`± 14953471`)	`156542729` ns/iter (`± 12899006`)	`1.30`
`full_es3`	`294298864` ns/iter (`± 21397607`)	`222253049` ns/iter (`± 20920857`)	`1.32`
`full_es5`	`305781933` ns/iter (`± 25772766`)	`213942890` ns/iter (`± 25815550`)	`1.43`
`parser`	`948125` ns/iter (`± 219447`)	`654749` ns/iter (`± 143468`)	`1.45`