diff --git a/crates/oxc_minifier/src/peephole/fold_constants.rs b/crates/oxc_minifier/src/peephole/fold_constants.rs index e5769375e1988..c564a8a6f89c4 100644 --- a/crates/oxc_minifier/src/peephole/fold_constants.rs +++ b/crates/oxc_minifier/src/peephole/fold_constants.rs @@ -1212,7 +1212,7 @@ mod test { #[test] fn test_fold_logical_op() { fold("x = true && x", "x = x"); - fold("x = [foo()] && x", "x = ([foo()],x)"); + fold("x = [foo()] && x", "x = (foo(),x)"); fold("x = false && x", "x = !1"); fold("x = true || x", "x = !0"); @@ -1240,7 +1240,7 @@ mod test { fold("x = foo() || true && bar()", "x = foo() || bar()"); fold("x = foo() || false && bar()", "x = foo() || !1"); fold("x = foo() && false && bar()", "x = foo() && !1"); - fold("x = foo() && false || bar()", "x = (foo() && !1, bar())"); + fold("x = foo() && false || bar()", "x = (foo(), bar())"); fold("x = foo() || false || bar()", "x = foo() || bar()"); fold("x = foo() && true && bar()", "x = foo() && bar()"); fold("x = foo() || true || bar()", "x = foo() || !0"); @@ -1280,7 +1280,7 @@ mod test { fn test_fold_logical_op2() { fold("x = function(){} && x", "x=x"); fold("x = true && function(){}", "x=function(){}"); - fold("x = [(function(){alert(x)})()] && x", "x=([function(){alert(x)}()],x)"); + fold("x = [(function(){alert(x)})()] && x", "x=(function(){alert(x)}(),x)"); } #[test] @@ -1288,7 +1288,7 @@ mod test { // fold if left is null/undefined fold("null ?? 1", "1"); fold("undefined ?? false", "!1"); - fold("(a(), null) ?? 1", "(a(), null, 1)"); + fold("(a(), null) ?? 1", "(a(), 1)"); fold("x = [foo()] ?? x", "x = [foo()]"); diff --git a/crates/oxc_minifier/src/peephole/remove_dead_code.rs b/crates/oxc_minifier/src/peephole/remove_dead_code.rs index 52202c6c0f8a0..552898522574f 100644 --- a/crates/oxc_minifier/src/peephole/remove_dead_code.rs +++ b/crates/oxc_minifier/src/peephole/remove_dead_code.rs @@ -2,6 +2,7 @@ use oxc_allocator::Vec; use oxc_ast::ast::*; use oxc_ast_visit::Visit; use oxc_ecmascript::{constant_evaluation::ConstantEvaluation, side_effects::MayHaveSideEffects}; +use oxc_span::GetSpan; use oxc_traverse::Ancestor; use crate::{ctx::Ctx, keep_var::KeepVar}; @@ -39,7 +40,7 @@ impl<'a, 'b> PeepholeOptimizations { if let Some(folded_expr) = match expr { Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx), Expression::SequenceExpression(sequence_expression) => { - Self::try_fold_sequence_expression(sequence_expression, ctx) + self.try_fold_sequence_expression(sequence_expression, ctx) } _ => None, } { @@ -381,6 +382,7 @@ impl<'a, 'b> PeepholeOptimizations { } fn try_fold_sequence_expression( + &mut self, sequence_expr: &mut SequenceExpression<'a>, ctx: Ctx<'a, 'b>, ) -> Option> { @@ -388,53 +390,41 @@ impl<'a, 'b> PeepholeOptimizations { ctx.parent(), Ancestor::CallExpressionCallee(_) | Ancestor::TaggedTemplateExpressionTag(_) ); - - if should_keep_as_sequence_expr && sequence_expr.expressions.len() == 2 { + if should_keep_as_sequence_expr + && sequence_expr.expressions.len() == 2 + && sequence_expr.expressions.first().unwrap().is_number_0() + { return None; } - let (should_fold, new_len) = sequence_expr.expressions.iter().enumerate().fold( - (false, 0), - |(mut should_fold, mut new_len), (i, expr)| { - if i == sequence_expr.expressions.len() - 1 || expr.may_have_side_effects(&ctx) { - new_len += 1; - } else { - should_fold = true; - } - (should_fold, new_len) - }, - ); - - if new_len == 0 { - return Some(ctx.ast.expression_null_literal(sequence_expr.span)); - } - - if should_fold { - let mut new_exprs = ctx.ast.vec_with_capacity(new_len); - let len = sequence_expr.expressions.len(); - for (i, expr) in sequence_expr.expressions.iter_mut().enumerate() { - if i == len - 1 || expr.may_have_side_effects(&ctx) { - new_exprs.push(ctx.ast.move_expression(expr)); + let old_len = sequence_expr.expressions.len(); + let mut i = 0; + sequence_expr.expressions.retain_mut(|e| { + i += 1; + if should_keep_as_sequence_expr && i == old_len - 1 { + if self.remove_unused_expression(e, ctx) { + *e = ctx.ast.expression_numeric_literal( + e.span(), + 0.0, + None, + NumberBase::Decimal, + ); + self.mark_current_function_as_changed(); } + return true; } - - if should_keep_as_sequence_expr && new_exprs.len() == 1 { - let number = ctx.ast.expression_numeric_literal( - sequence_expr.span, - 1.0, - None, - NumberBase::Decimal, - ); - new_exprs.insert(0, number); - } - - if new_exprs.len() == 1 { - return Some(new_exprs.pop().unwrap()); + if i == old_len { + return true; } - - return Some(ctx.ast.expression_sequence(sequence_expr.span, new_exprs)); + !self.remove_unused_expression(e, ctx) + }); + if sequence_expr.expressions.len() == 1 { + return Some(sequence_expr.expressions.pop().unwrap()); } + if sequence_expr.expressions.len() != old_len { + self.mark_current_function_as_changed(); + } None } } @@ -607,6 +597,15 @@ mod test { test("while(true) { return a; unreachable;}", "for(;;) return a"); } + #[test] + fn remove_unused_expressions_in_sequence() { + test("true, foo();", "foo();"); + test("(true, foo)();", "(0, foo)();"); + test("(true, true, foo)();", "(0, foo)();"); + test("var foo; (true, foo)();", "var foo; (0, foo)();"); + test("var foo; (true, true, foo)();", "var foo; (0, foo)();"); + } + #[test] fn remove_unused_expressions_in_for() { test( diff --git a/crates/oxc_minifier/tests/peephole/mod.rs b/crates/oxc_minifier/tests/peephole/mod.rs index ca634017dc828..0c0f3fe625bcb 100644 --- a/crates/oxc_minifier/tests/peephole/mod.rs +++ b/crates/oxc_minifier/tests/peephole/mod.rs @@ -57,7 +57,7 @@ fn integration() { } console.log(c, d); ", - "if ((() => console.log('effect'))(), !1) for (var c = 1, c; unknownGlobal; unknownGlobal) var d; + "if (console.log('effect'), !1) for (var c = 1, c; unknownGlobal; unknownGlobal) var d; console.log(c, d); ", ); @@ -74,8 +74,8 @@ fn fold() { #[test] // https://github.com/oxc-project/oxc/issues/4341 fn tagged_template() { - test_same("(1, o.f)()"); - test_same("(1, o.f)``"); + test("(1, o.f)()", "(0, o.f)()"); + test("(1, o.f)``", "(0, o.f)``"); test_same("(!0 && o.f)()"); test_same("(!0 && o.f)``"); test("(!0 ? o.f : !1)()", "(0 ? !1: o.f)()"); @@ -91,10 +91,10 @@ fn eval() { test_same("(!0 && eval)(x)"); test_same("(1 ? eval : 2)(x)"); test_same("(1 ? eval : 2)?.(x)"); - test_same("(1, eval)(x)"); - test_same("(1, eval)?.(x)"); - test_same("(3, eval)(x)"); - test_same("(4, eval)?.(x)"); + test("(1, eval)(x)", "(0, eval)(x)"); + test("(1, eval)?.(x)", "(0, eval)?.(x)"); + test("(3, eval)(x)", "(0, eval)(x)"); + test("(4, eval)?.(x)", "(0, eval)?.(x)"); test_same("(eval)(x)"); test_same("(eval)?.(x)"); test_same("eval(x)");