Skip to content

Commit

Permalink
refactor(minifier): change minimize conditionals into a loop (#8413)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Boshen
Boshen committed Jan 10, 2025
1 parent baaec60 commit fb2acd8
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 53 deletions.
79 changes: 41 additions & 38 deletions crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions {
};

if let Some(expr) = expr {
self.try_fold_expr_in_boolean_context(expr, Ctx(ctx));
Self::try_fold_expr_in_boolean_context(expr, Ctx(ctx));
}

if let Some(folded_stmt) = match stmt {
Expand All @@ -75,14 +75,29 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions {
}

fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
if let Expression::ConditionalExpression(e) = expr {
self.try_fold_expr_in_boolean_context(&mut e.test, Ctx(ctx));
loop {
let mut changed = false;
if let Expression::ConditionalExpression(logical_expr) = expr {
if let Some(e) = Self::try_minimize_conditional(logical_expr, ctx) {
*expr = e;
changed = true;
}
}
if let Expression::ConditionalExpression(logical_expr) = expr {
if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, Ctx(ctx)) {
changed = true;
}
}
if changed {
self.changed = true;
} else {
break;
}
}

if let Some(folded_expr) = match expr {
Expression::UnaryExpression(e) => Self::try_minimize_not(e, ctx),
Expression::BinaryExpression(e) => Self::try_minimize_binary(e, ctx),
Expression::ConditionalExpression(e) => Self::try_minimize_conditional(e, ctx),
_ => None,
} {
*expr = folded_expr;
Expand Down Expand Up @@ -288,17 +303,17 @@ impl<'a> PeepholeMinimizeConditions {
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
// `(a, b) ? c : d` -> `a, b ? c : d`
if let Expression::SequenceExpression(sequence_expr) = &expr.test {
if let Expression::SequenceExpression(sequence_expr) = &mut expr.test {
if sequence_expr.expressions.len() > 1 {
let span = expr.span();
let mut sequence = ctx.ast.move_expression(&mut expr.test);
let Expression::SequenceExpression(ref mut sequence_expr) = &mut sequence else {
unreachable!()
};
let test = sequence_expr.expressions.pop().expect("sequence_expr.expressions");
expr.test = test;
let test = sequence_expr.expressions.pop().unwrap();
sequence_expr.expressions.push(ctx.ast.expression_conditional(
SPAN,
ctx.ast.move_expression(&mut expr.test),
span,
test,
ctx.ast.move_expression(&mut expr.consequent),
ctx.ast.move_expression(&mut expr.alternate),
));
Expand Down Expand Up @@ -382,16 +397,15 @@ impl<'a> PeepholeMinimizeConditions {
(&expr.test, &expr.consequent)
{
if test_ident.name == consequent_ident.name {
let ident = ctx.ast.move_expression(&mut expr.test);

return Some(ctx.ast.expression_logical(
expr.span,
ident,
ctx.ast.move_expression(&mut expr.test),
LogicalOperator::Or,
ctx.ast.move_expression(&mut expr.alternate),
));
}
}

// `a ? b : a` -> `a && b`
if let (Expression::Identifier(test_ident), Expression::Identifier(alternate_ident)) =
(&expr.test, &expr.alternate)
Expand Down Expand Up @@ -653,29 +667,14 @@ impl<'a> PeepholeMinimizeConditions {
/// Simplify syntax when we know it's used inside a boolean context, e.g. `if (boolean_context) {}`.
///
/// <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2059>
fn try_fold_expr_in_boolean_context(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) {
loop {
let changed = self.try_fold_expr_in_boolean_context_impl(e, ctx);
if changed {
self.changed = true;
} else {
return;
}
}
}

fn try_fold_expr_in_boolean_context_impl(
&mut self,
expr: &mut Expression<'a>,
ctx: Ctx<'a, '_>,
) -> bool {
fn try_fold_expr_in_boolean_context(expr: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
match expr {
// "!!a" => "a"
Expression::UnaryExpression(u1) if u1.operator.is_not() => {
if let Expression::UnaryExpression(u2) = &mut u1.argument {
if u2.operator.is_not() {
let mut e = ctx.ast.move_expression(&mut u2.argument);
self.try_fold_expr_in_boolean_context(&mut e, ctx);
Self::try_fold_expr_in_boolean_context(&mut e, ctx);
*expr = e;
return true;
}
Expand All @@ -701,8 +700,8 @@ impl<'a> PeepholeMinimizeConditions {
}
// "if (!!a && !!b)" => "if (a && b)"
Expression::LogicalExpression(e) if e.operator == LogicalOperator::And => {
self.try_fold_expr_in_boolean_context(&mut e.left, ctx);
self.try_fold_expr_in_boolean_context(&mut e.right, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.left, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.right, ctx);
// "if (anything && truthyNoSideEffects)" => "if (anything)"
if ctx.get_side_free_boolean_value(&e.right) == Some(true) {
*expr = ctx.ast.move_expression(&mut e.left);
Expand All @@ -711,8 +710,8 @@ impl<'a> PeepholeMinimizeConditions {
}
// "if (!!a ||!!b)" => "if (a || b)"
Expression::LogicalExpression(e) if e.operator == LogicalOperator::Or => {
self.try_fold_expr_in_boolean_context(&mut e.left, ctx);
self.try_fold_expr_in_boolean_context(&mut e.right, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.left, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.right, ctx);
// "if (anything || falsyNoSideEffects)" => "if (anything)"
if ctx.get_side_free_boolean_value(&e.right) == Some(false) {
*expr = ctx.ast.move_expression(&mut e.left);
Expand All @@ -721,8 +720,8 @@ impl<'a> PeepholeMinimizeConditions {
}
Expression::ConditionalExpression(e) => {
// "if (a ? !!b : !!c)" => "if (a ? b : c)"
self.try_fold_expr_in_boolean_context(&mut e.consequent, ctx);
self.try_fold_expr_in_boolean_context(&mut e.alternate, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.consequent, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.alternate, ctx);
if let Some(boolean) = ctx.get_side_free_boolean_value(&e.consequent) {
let right = ctx.ast.move_expression(&mut e.alternate);
let left = ctx.ast.move_expression(&mut e.test);
Expand Down Expand Up @@ -1836,8 +1835,8 @@ mod test {

#[test]
fn test_coercion_substitution_not() {
test("var x = {}; var y = !(x != null) ? 1 : 2;", "var x = {}; var y = x != null ? 2 : 1;");
test("var x = 1; var y = !(x != 0) ? 1 : 2; ", "var x = 1; var y = x != 0 ? 2 : 1; ");
test("var x = {}; var y = !(x != null) ? 1 : 2;", "var x = {}; var y = x == null ? 1 : 2;");
test("var x = 1; var y = !(x != 0) ? 1 : 2; ", "var x = 1; var y = x == 0 ? 1 : 2; ");
}

#[test]
Expand Down Expand Up @@ -1973,7 +1972,7 @@ mod test {
}

#[test]
fn minimize_conditional_exprs_esbuild() {
fn minimize_conditional_exprs() {
test("(a, b) ? c : d", "a, b ? c : d");
test("!a ? b : c", "a ? c : b");
// test("/* @__PURE__ */ a() ? b : b", "b");
Expand All @@ -1994,6 +1993,10 @@ mod test {
test("a() != null ? a() : b", "a() == null ? b : a()");
// test("a != null ? a : b", "a ?? b");
// test("a != null ? a.b.c[d](e) : undefined", "a?.b.c[d](e)");
test("cmp !== 0 ? cmp : (bar, cmp);", "cmp === 0 && bar, cmp;");
test("cmp === 0 ? cmp : (bar, cmp);", "cmp === 0 || bar, cmp;");
test("cmp !== 0 ? (bar, cmp) : cmp;", "cmp === 0 || bar, cmp;");
test("cmp === 0 ? (bar, cmp) : cmp;", "cmp === 0 && bar, cmp;");
}

#[test]
Expand Down
26 changes: 13 additions & 13 deletions crates/oxc_minifier/tests/ast_passes/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@ fn test_same(source_text: &str) {

// Oxc Integration Tests

#[test] // https://github.com/oxc-project/oxc/issues/4341
fn tagged_template() {
test_same("(1, o.f)()");
test_same("(1, o.f)``");
test_same("(!0 && o.f)()");
test_same("(!0 && o.f)``");
test("(!0 ? o.f : !1)()", "(0 ? !1: o.f)()");
test("(!0 ? o.f : !1)``", "(0 ? !1: o.f)``");

test("foo(true && o.f)", "foo(o.f)");
test("foo(true ? o.f : false)", "foo(o.f)");
}

#[test]
fn cjs() {
// Bail `cjs-module-lexer`.
Expand Down Expand Up @@ -56,16 +69,3 @@ fn cjs() {
});",
);
}

#[test] // https://github.com/oxc-project/oxc/issues/4341
fn tagged_template() {
test_same("(1, o.f)()");
test_same("(1, o.f)``");
test_same("(!0 && o.f)()");
test_same("(!0 && o.f)``");
test("(!0 ? o.f : !1)()", "(0 ? !1: o.f)()");
test("(!0 ? o.f : !1)``", "(0 ? !1: o.f)``");

test("foo(true && o.f)", "foo(o.f)");
test("foo(true ? o.f : false)", "foo(o.f)");
}
4 changes: 2 additions & 2 deletions tasks/minsize/minsize.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Original | minified | minified | gzip | gzip | Fixture

555.77 kB | 273.21 kB | 270.13 kB | 90.92 kB | 90.80 kB | d3.js

1.01 MB | 460.18 kB | 458.89 kB | 126.76 kB | 126.71 kB | bundle.min.js
1.01 MB | 460.18 kB | 458.89 kB | 126.77 kB | 126.71 kB | bundle.min.js

1.25 MB | 652.85 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js

Expand All @@ -23,5 +23,5 @@ Original | minified | minified | gzip | gzip | Fixture

6.69 MB | 2.32 MB | 2.31 MB | 492.64 kB | 488.28 kB | antd.js

10.95 MB | 3.49 MB | 3.49 MB | 907.46 kB | 915.50 kB | typescript.js
10.95 MB | 3.49 MB | 3.49 MB | 907.45 kB | 915.50 kB | typescript.js

0 comments on commit fb2acd8

Please sign in to comment.