Skip to content

Commit

Permalink
refactor(minifier): move conditional assignment to minimize_conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Boshen
Boshen committed Jan 23, 2025
1 parent ba201a6 commit a811d10
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 86 deletions.
116 changes: 95 additions & 21 deletions crates/oxc_minifier/src/peephole/minimize_conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,13 @@ impl<'a> PeepholeOptimizations {
let consequent = Self::get_block_expression(&mut if_stmt.consequent, ctx);
let else_branch = if_stmt.alternate.as_mut().unwrap();
let alternate = Self::get_block_expression(else_branch, ctx);
let mut cond_expr = ctx.ast.conditional_expression(
let expr = Self::minimize_conditional(
if_stmt.span,
test,
consequent,
alternate,
ctx,
);
let expr = Self::try_minimize_conditional(&mut cond_expr, ctx)
.unwrap_or_else(|| {
Expression::ConditionalExpression(ctx.ast.alloc(cond_expr))
});
return Some(ctx.ast.statement_expression(if_stmt.span, expr));
}
}
Expand Down Expand Up @@ -285,14 +282,13 @@ impl<'a> PeepholeOptimizations {
let mut if_stmt = if_stmt.unbox();
let consequent = Self::get_block_return_expression(&mut if_stmt.consequent, ctx);
let alternate = Self::take_return_argument(&mut stmts[i + 1], ctx);
let mut cond_expr = ctx.ast.conditional_expression(
let argument = Self::minimize_conditional(
if_stmt.span,
if_stmt.test,
consequent,
alternate,
ctx,
);
let argument = Self::try_minimize_conditional(&mut cond_expr, ctx)
.unwrap_or_else(|| Expression::ConditionalExpression(ctx.ast.alloc(cond_expr)));
stmts[i] = ctx.ast.statement_return(if_stmt.span, Some(argument));
*changed = true;
break;
Expand Down Expand Up @@ -359,6 +355,18 @@ impl<'a> PeepholeOptimizations {
}
}

pub fn minimize_conditional(
span: Span,
test: Expression<'a>,
consequent: Expression<'a>,
alternate: Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
let mut cond_expr = ctx.ast.conditional_expression(span, test, consequent, alternate);
Self::try_minimize_conditional(&mut cond_expr, ctx)
.unwrap_or_else(|| Expression::ConditionalExpression(ctx.ast.alloc(cond_expr)))
}

// <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745>
fn try_minimize_conditional(
expr: &mut ConditionalExpression<'a>,
Expand All @@ -373,16 +381,13 @@ impl<'a> PeepholeOptimizations {
let Expression::SequenceExpression(sequence_expr) = &mut sequence else {
unreachable!()
};
let mut cond_expr = ctx.ast.conditional_expression(
let expr = Self::minimize_conditional(
span,
sequence_expr.expressions.pop().unwrap(),
ctx.ast.move_expression(&mut expr.consequent),
ctx.ast.move_expression(&mut expr.alternate),
ctx,
);
let expr =
Self::try_minimize_conditional(&mut cond_expr, ctx).unwrap_or_else(|| {
Expression::ConditionalExpression(ctx.ast.alloc(cond_expr))
});
sequence_expr.expressions.push(expr);
return Some(sequence);
}
Expand Down Expand Up @@ -572,16 +577,16 @@ impl<'a> PeepholeOptimizations {
}
}

// `a ? b(c, d) : b(e, d)` -> `b(a ? c : e, d)``
// `a ? b(c, d) : b(e, d)` -> `b(a ? c : e, d)`
if let (
Expression::Identifier(test),
Expression::CallExpression(consequent),
Expression::CallExpression(alternate),
) = (&expr.test, &mut expr.consequent, &mut expr.alternate)
{
if consequent.callee.content_eq(&alternate.callee)
&& consequent.arguments.len() == alternate.arguments.len()
&& ctx.scopes().find_binding(ctx.current_scope_id(), &test.name).is_some()
if consequent.arguments.len() == alternate.arguments.len()
&& !Ctx(ctx).is_global_reference(test)
&& consequent.callee.content_eq(&alternate.callee)
&& consequent
.arguments
.iter()
Expand Down Expand Up @@ -616,10 +621,9 @@ impl<'a> PeepholeOptimizations {
alternate_first_arg,
),
);

return Some(ctx.ast.expression_call(expr.span, callee, NONE, args, false));
}
// `a ? b(c) : b(e)` -> `b(a ? c : e)``
// `a ? b(c) : b(e)` -> `b(a ? c : e)`
if !matches!(consequent.arguments[0], Argument::SpreadElement(_))
&& !matches!(alternate.arguments[0], Argument::SpreadElement(_))
{
Expand All @@ -630,17 +634,24 @@ impl<'a> PeepholeOptimizations {
let alternate_first_arg =
ctx.ast.move_expression(alternate.arguments[0].to_expression_mut());
let mut args = std::mem::replace(&mut consequent.arguments, ctx.ast.vec());
args[0] = Argument::from(ctx.ast.expression_conditional(
let cond_expr = Self::minimize_conditional(
expr.test.span(),
ctx.ast.move_expression(&mut expr.test),
consequent_first_arg,
alternate_first_arg,
));
ctx,
);
args[0] = Argument::from(cond_expr);
return Some(ctx.ast.expression_call(expr.span, callee, NONE, args, false));
}
}
}

// Not part of esbuild
if let Some(e) = Self::try_merge_conditional_expression_inside(expr, ctx) {
return Some(e);
}

// TODO: Try using the "??" or "?." operators

if expr.alternate.content_eq(&expr.consequent) {
Expand All @@ -661,6 +672,47 @@ impl<'a> PeepholeOptimizations {
None
}

/// Merge `consequent` and `alternate` of `ConditionalExpression` inside.
///
/// - `x ? a = 0 : a = 1` -> `a = x ? 0 : 1`
fn try_merge_conditional_expression_inside(
expr: &mut ConditionalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
let (
Expression::AssignmentExpression(consequent),
Expression::AssignmentExpression(alternate),
) = (&mut expr.consequent, &mut expr.alternate)
else {
return None;
};
if !matches!(consequent.left, AssignmentTarget::AssignmentTargetIdentifier(_)) {
return None;
}
if consequent.right.is_anonymous_function_definition() {
return None;
}
if consequent.operator != AssignmentOperator::Assign
|| consequent.operator != alternate.operator
|| consequent.left.content_ne(&alternate.left)
{
return None;
}
let cond_expr = Self::minimize_conditional(
expr.span,
ctx.ast.move_expression(&mut expr.test),
ctx.ast.move_expression(&mut consequent.right),
ctx.ast.move_expression(&mut alternate.right),
ctx,
);
Some(ctx.ast.expression_assignment(
expr.span,
consequent.operator,
ctx.ast.move_assignment_target(&mut alternate.left),
cond_expr,
))
}

/// 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>
Expand Down Expand Up @@ -2009,4 +2061,26 @@ mod test {
test("if (x) { var foo } else { bar }", "if (x) { var foo } else bar");
test_same("if (x) foo; else { var bar }");
}

#[test]
fn test_compress_conditional_expression_inside() {
test("x ? a = 0 : a = 1", "a = x ? 0 : 1");
test(
"x ? a = function foo() { return 'a' } : a = function bar() { return 'b' }",
"a = x ? function foo() { return 'a' } : function bar() { return 'b' }",
);

// a.b might have a side effect
test_same("x ? a.b = 0 : a.b = 1");
// `a = x ? () => 'a' : () => 'b'` does not set the name property of the function
test_same("x ? a = () => 'a' : a = () => 'b'");
test_same("x ? a = function () { return 'a' } : a = function () { return 'b' }");
test_same("x ? a = class { foo = 'a' } : a = class { foo = 'b' }");

// for non `=` operators, `GetValue(lref)` is called before `Evaluation of AssignmentExpression`
// so cannot be fold to `a += x ? 0 : 1`
// example case: `(()=>{"use strict"; (console.log("log"), 1) ? a += 0 : a += 1; })()`
test_same("x ? a += 0 : a += 1");
test_same("x ? a &&= 0 : a &&= 1");
}
}
65 changes: 0 additions & 65 deletions crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ impl<'a> PeepholeOptimizations {
Expression::TemplateLiteral(t) => Self::try_fold_template_literal(t, ctx),
Expression::BinaryExpression(e) => Self::try_fold_loose_equals_undefined(e, ctx)
.or_else(|| Self::try_compress_typeof_undefined(e, ctx)),
Expression::ConditionalExpression(e) => {
Self::try_merge_conditional_expression_inside(e, ctx)
}
Expression::NewExpression(e) => Self::get_fold_constructor_name(&e.callee, ctx)
.and_then(|name| {
Self::try_fold_object_or_array_constructor(e.span, name, &mut e.arguments, ctx)
Expand Down Expand Up @@ -834,46 +831,6 @@ impl<'a> PeepholeOptimizations {
}
}

/// Merge `consequent` and `alternate` of `ConditionalExpression` inside.
///
/// - `x ? a = 0 : a = 1` -> `a = x ? 0 : 1`
fn try_merge_conditional_expression_inside(
expr: &mut ConditionalExpression<'a>,
ctx: Ctx<'a, '_>,
) -> Option<Expression<'a>> {
let (
Expression::AssignmentExpression(consequent),
Expression::AssignmentExpression(alternate),
) = (&mut expr.consequent, &mut expr.alternate)
else {
return None;
};
if !matches!(consequent.left, AssignmentTarget::AssignmentTargetIdentifier(_)) {
return None;
}
if consequent.right.is_anonymous_function_definition() {
return None;
}
if consequent.operator != AssignmentOperator::Assign
|| consequent.operator != alternate.operator
|| consequent.left.content_ne(&alternate.left)
{
return None;
}

Some(ctx.ast.expression_assignment(
expr.span,
consequent.operator,
ctx.ast.move_assignment_target(&mut alternate.left),
ctx.ast.expression_conditional(
expr.span,
ctx.ast.move_expression(&mut expr.test),
ctx.ast.move_expression(&mut consequent.right),
ctx.ast.move_expression(&mut alternate.right),
),
))
}

/// Fold `Boolean`, `Number`, `String`, `BigInt` constructors.
///
/// `Boolean(a)` -> `!!a`
Expand Down Expand Up @@ -1518,28 +1475,6 @@ mod test {
test_same("x += -1");
}

#[test]
fn test_compress_conditional_expression_inside() {
test("x ? a = 0 : a = 1", "a = x ? 0 : 1");
test(
"x ? a = function foo() { return 'a' } : a = function bar() { return 'b' }",
"a = x ? function foo() { return 'a' } : function bar() { return 'b' }",
);

// a.b might have a side effect
test_same("x ? a.b = 0 : a.b = 1");
// `a = x ? () => 'a' : () => 'b'` does not set the name property of the function
test_same("x ? a = () => 'a' : a = () => 'b'");
test_same("x ? a = function () { return 'a' } : a = function () { return 'b' }");
test_same("x ? a = class { foo = 'a' } : a = class { foo = 'b' }");

// for non `=` operators, `GetValue(lref)` is called before `Evaluation of AssignmentExpression`
// so cannot be fold to `a += x ? 0 : 1`
// example case: `(()=>{"use strict"; (console.log("log"), 1) ? a += 0 : a += 1; })()`
test_same("x ? a += 0 : a += 1");
test_same("x ? a &&= 0 : a &&= 1");
}

#[test]
fn test_fold_literal_object_constructors() {
test("x = new Object", "x = ({})");
Expand Down

0 comments on commit a811d10

Please sign in to comment.