Skip to content

Commit

Permalink
feat(es/minifier): Optimize switch with side effect and termination t…
Browse files Browse the repository at this point in the history
…ests (#9677)

**Description:**

This is learnt from terser/terser#1044
**Key point:** all the cases (statements) after the last side-effect and
non-terminated one are useless.

There are also some points on whether a statement is side-effect free:
- **Var declaration:** I think var declaration always contains side
effect.
- **Function declaration:** In non-strict mode, function declaration is
same as var declaration. In strict mode, function is declared in
enclosing block scope, so it's side-effect free. But there is a bad case
when we call the function before it is declared in the switch case:
```js
"use strict";
const k = (() => {
    let x = 1;
    switch (x) {
        case y():
        case x: {
            async function y() {
                console.log(123);
            }
        }
    }
    return x;
})();
```

**This is an existing bug.** I'm not sure whether we should fix it since
it is a very bad code and I also find some similar bad cases with
terser.

~I'm also not sure it's appropriate to introduce context variable
`in_strict` for `StmtExt:: may_have_side_effects`, because `in_strict`
could change from `false` to `true`. But this is a **conservative**
mistake and does not break the program. Maybe we should use visitor for
context-aware side effect checker in the future.~

**Related issue:**

 - Closes #8953
  • Loading branch information
CPunisher authored Oct 29, 2024
1 parent 2bbd1e8 commit 7344a63
Show file tree
Hide file tree
Showing 36 changed files with 307 additions and 197 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-planes-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
swc_ecma_utils: major
---

feat(es/minifier): Optimize switch and replace empty test with side effect and termination tests
Original file line number Diff line number Diff line change
@@ -1,6 +1,2 @@
//// [stringLiteralsWithSwitchStatements03.ts]
var z;
switch(void 0){
case randBool() ? "foo" : "baz":
case z || "baz":
}
randBool();
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ switch((M || (M = {})).fn = function(x) {
case void 0 === x ? "undefined" : _type_of(x):
case void 0 === M ? "undefined" : _type_of(M):
case M.fn(1):
default:
}
var x, M, C = function C() {
_class_call_check(this, C);
Expand Down
14 changes: 10 additions & 4 deletions crates/swc_ecma_minifier/src/compress/optimize/bools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ impl Optimizer<'_> {
expr: &mut Expr,
is_ret_val_ignored: bool,
) -> bool {
let cost = negate_cost(&self.expr_ctx, expr, is_ret_val_ignored, is_ret_val_ignored);
let cost = negate_cost(
&self.ctx.expr_ctx,
expr,
is_ret_val_ignored,
is_ret_val_ignored,
);
if cost >= 0 {
return false;
}
Expand Down Expand Up @@ -72,11 +77,12 @@ impl Optimizer<'_> {

let ctx = Ctx {
in_bool_ctx: true,
..self.ctx
..self.ctx.clone()
};

self.with_ctx(ctx).negate(&mut e.left, false);
self.with_ctx(ctx).negate(&mut e.right, is_ret_val_ignored);
self.with_ctx(ctx.clone()).negate(&mut e.left, false);
self.with_ctx(ctx.clone())
.negate(&mut e.right, is_ret_val_ignored);

dump_change_detail!("{} => {}", start, dump(&*e, false));

Expand Down
10 changes: 5 additions & 5 deletions crates/swc_ecma_minifier/src/compress/optimize/conditionals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ impl Optimizer<'_> {
_ => {}
}

if negate_cost(&self.expr_ctx, &stmt.test, true, false) < 0 {
if negate_cost(&self.ctx.expr_ctx, &stmt.test, true, false) < 0 {
report_change!("if_return: Negating `cond` of an if statement which has cons and alt");
let ctx = Ctx {
in_bool_ctx: true,
..self.ctx
..self.ctx.clone()
};
self.with_ctx(ctx).negate(&mut stmt.test, false);
swap(alt, &mut *stmt.cons);
Expand Down Expand Up @@ -63,7 +63,7 @@ impl Optimizer<'_> {
_ => return,
};

if !cond.cons.may_have_side_effects(&self.expr_ctx) {
if !cond.cons.may_have_side_effects(&self.ctx.expr_ctx) {
self.changed = true;
report_change!("conditionals: `cond ? useless : alt` => `cond || alt`");
*e = BinExpr {
Expand All @@ -76,7 +76,7 @@ impl Optimizer<'_> {
return;
}

if !cond.alt.may_have_side_effects(&self.expr_ctx) {
if !cond.alt.may_have_side_effects(&self.ctx.expr_ctx) {
self.changed = true;
report_change!("conditionals: `cond ? cons : useless` => `cond && cons`");
*e = BinExpr {
Expand Down Expand Up @@ -878,7 +878,7 @@ impl Optimizer<'_> {
) = (&*cons, &*alt)
{
// I don't know why, but terser behaves differently
negate(&self.expr_ctx, &mut test, true, false);
negate(&self.ctx.expr_ctx, &mut test, true, false);

swap(&mut cons, &mut alt);
}
Expand Down
16 changes: 8 additions & 8 deletions crates/swc_ecma_minifier/src/compress/optimize/evaluate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl Optimizer<'_> {
//

for arg in &*args {
if arg.spread.is_some() || arg.expr.may_have_side_effects(&self.expr_ctx) {
if arg.spread.is_some() || arg.expr.may_have_side_effects(&self.ctx.expr_ctx) {
return;
}
}
Expand Down Expand Up @@ -233,7 +233,7 @@ impl Optimizer<'_> {
return;
}

if let Known(char_code) = args[0].expr.as_pure_number(&self.expr_ctx) {
if let Known(char_code) = args[0].expr.as_pure_number(&self.ctx.expr_ctx) {
let v = char_code.floor() as u32;

if let Some(v) = char::from_u32(v) {
Expand Down Expand Up @@ -341,7 +341,7 @@ impl Optimizer<'_> {
}

if let Expr::Call(..) = e {
if let Some(value) = eval_as_number(&self.expr_ctx, e) {
if let Some(value) = eval_as_number(&self.ctx.expr_ctx, e) {
self.changed = true;
report_change!("evaluate: Evaluated an expression as `{}`", value);

Expand All @@ -367,8 +367,8 @@ impl Optimizer<'_> {

match e {
Expr::Bin(bin @ BinExpr { op: op!("**"), .. }) => {
let l = bin.left.as_pure_number(&self.expr_ctx);
let r = bin.right.as_pure_number(&self.expr_ctx);
let l = bin.left.as_pure_number(&self.ctx.expr_ctx);
let r = bin.right.as_pure_number(&self.ctx.expr_ctx);

if let Known(l) = l {
if let Known(r) = r {
Expand All @@ -395,9 +395,9 @@ impl Optimizer<'_> {
}

Expr::Bin(bin @ BinExpr { op: op!("/"), .. }) => {
let ln = bin.left.as_pure_number(&self.expr_ctx);
let ln = bin.left.as_pure_number(&self.ctx.expr_ctx);

let rn = bin.right.as_pure_number(&self.expr_ctx);
let rn = bin.right.as_pure_number(&self.ctx.expr_ctx);
if let (Known(ln), Known(rn)) = (ln, rn) {
// Prefer `0/0` over NaN.
if ln == 0.0 && rn == 0.0 {
Expand Down Expand Up @@ -477,7 +477,7 @@ impl Optimizer<'_> {
}
// Remove rhs of lhs if possible.

let v = left.right.as_pure_bool(&self.expr_ctx);
let v = left.right.as_pure_bool(&self.ctx.expr_ctx);
if let Known(v) = v {
// As we used as_pure_bool, we can drop it.
if v && e.op == op!("&&") {
Expand Down
8 changes: 4 additions & 4 deletions crates/swc_ecma_minifier/src/compress/optimize/if_return.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use swc_common::{util::take::Take, Spanned, DUMMY_SP};
use swc_ecma_ast::*;
use swc_ecma_transforms_optimization::debug_assert_valid;
use swc_ecma_utils::{StmtExt, StmtLike};
use swc_ecma_utils::StmtLike;
use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};

use super::Optimizer;
Expand Down Expand Up @@ -116,7 +116,7 @@ impl Optimizer<'_> {
// for stmt in stmts.iter_mut() {
// let ctx = Ctx {
// is_nested_if_return_merging: true,
// ..self.ctx
// ..self.ctx.clone()
// };
// self.with_ctx(ctx).merge_nested_if_returns(stmt, terminate);
// }
Expand Down Expand Up @@ -396,7 +396,7 @@ impl Optimizer<'_> {
&& seq
.exprs
.last()
.map(|v| is_pure_undefined(&self.expr_ctx, v))
.map(|v| is_pure_undefined(&self.ctx.expr_ctx, v))
.unwrap_or(true) =>
{
let expr = self.ignore_return_value(&mut cur);
Expand Down Expand Up @@ -570,7 +570,7 @@ fn always_terminates_with_return_arg(s: &Stmt) -> bool {
fn can_merge_as_if_return(s: &Stmt) -> bool {
fn cost(s: &Stmt) -> Option<isize> {
if let Stmt::Block(..) = s {
if !s.terminates() {
if !swc_ecma_utils::StmtExt::terminates(s) {
return None;
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/swc_ecma_minifier/src/compress/optimize/iife.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ impl Optimizer<'_> {
let ctx = Ctx {
in_fn_like: true,
top_level: false,
..self.ctx
..self.ctx.clone()
};
let mut optimizer = self.with_ctx(ctx);
match find_body(callee) {
Expand Down
10 changes: 5 additions & 5 deletions crates/swc_ecma_minifier/src/compress/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl Optimizer<'_> {
if ref_count == 0 {
self.mode.store(ident.to_id(), &*init);

if init.may_have_side_effects(&self.expr_ctx) {
if init.may_have_side_effects(&self.ctx.expr_ctx) {
// TODO: Inline partially
return;
}
Expand Down Expand Up @@ -494,7 +494,7 @@ impl Optimizer<'_> {
}
}

if init.may_have_side_effects(&self.expr_ctx) {
if init.may_have_side_effects(&self.ctx.expr_ctx) {
return;
}

Expand Down Expand Up @@ -531,13 +531,13 @@ impl Optimizer<'_> {
if body.stmts.len() == 1 {
match &body.stmts[0] {
Stmt::Expr(ExprStmt { expr, .. })
if expr.size(self.expr_ctx.unresolved_ctxt) < cost_limit =>
if expr.size(self.ctx.expr_ctx.unresolved_ctxt) < cost_limit =>
{
return true
}

Stmt::Return(ReturnStmt { arg: Some(arg), .. })
if arg.size(self.expr_ctx.unresolved_ctxt) < cost_limit =>
if arg.size(self.ctx.expr_ctx.unresolved_ctxt) < cost_limit =>
{
return true
}
Expand Down Expand Up @@ -719,7 +719,7 @@ impl Optimizer<'_> {
})
{
if let Decl::Class(ClassDecl { class, .. }) = decl {
if class_has_side_effect(&self.expr_ctx, class) {
if class_has_side_effect(&self.ctx.expr_ctx, class) {
return;
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/swc_ecma_minifier/src/compress/optimize/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl Optimizer<'_> {

match stmt {
Stmt::While(w) => {
let (purity, val) = w.test.cast_to_bool(&self.expr_ctx);
let (purity, val) = w.test.cast_to_bool(&self.ctx.expr_ctx);
if let Known(false) = val {
if purity.is_pure() {
let changed = UnreachableHandler::preserve_vars(stmt);
Expand All @@ -86,7 +86,7 @@ impl Optimizer<'_> {
}
Stmt::For(f) => {
if let Some(test) = &mut f.test {
let (purity, val) = test.cast_to_bool(&self.expr_ctx);
let (purity, val) = test.cast_to_bool(&self.ctx.expr_ctx);
if let Known(false) = val {
let changed = UnreachableHandler::preserve_vars(&mut f.body);
self.changed |= changed;
Expand Down
Loading

0 comments on commit 7344a63

Please sign in to comment.