Skip to content

fix(minifier): call mark_current_function_as_changed in remove_unused_expression #9457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Mar 1, 2025
Merged

fix(minifier): call mark_current_function_as_changed in remove_unused_expression #9457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/oxc_minifier/src/peephole/remove_dead_code.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ impl<'a, 'b> PeepholeOptimizations {

/// Try folding conditional expression (?:) if the condition results of the condition is known.
fn try_fold_conditional_expression(
&self,
&mut self,
expr: &mut ConditionalExpression<'a>,
ctx: Ctx<'a, 'b>,
) -> Option<Expression<'a>> {
Expand Down
73 changes: 57 additions & 16 deletions crates/oxc_minifier/src/peephole/remove_unused_expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use super::PeepholeOptimizations;

impl<'a> PeepholeOptimizations {
/// `SimplifyUnusedExpr`: <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L534>
pub fn remove_unused_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
pub fn remove_unused_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
match e {
Expression::ArrayExpression(_) => self.fold_array_expression(e, ctx),
Expression::UnaryExpression(_) => self.fold_unary_expression(e, ctx),
Expand All @@ -34,52 +34,64 @@ impl<'a> PeepholeOptimizations {
}
}

fn fold_unary_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_unary_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::UnaryExpression(unary_expr) = e else { return false };
match unary_expr.operator {
UnaryOperator::Void | UnaryOperator::LogicalNot => {
*e = ctx.ast.move_expression(&mut unary_expr.argument);
self.mark_current_function_as_changed();
self.remove_unused_expression(e, ctx)
}
UnaryOperator::Typeof => {
if unary_expr.argument.is_identifier_reference() {
true
} else {
*e = ctx.ast.move_expression(&mut unary_expr.argument);
self.mark_current_function_as_changed();
self.remove_unused_expression(e, ctx)
}
}
_ => false,
}
}

fn fold_sequence_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_sequence_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::SequenceExpression(sequence_expr) = e else { return false };

let old_len = sequence_expr.expressions.len();
sequence_expr.expressions.retain_mut(|e| !self.remove_unused_expression(e, ctx));
if sequence_expr.expressions.len() != old_len {
self.mark_current_function_as_changed();
}

sequence_expr.expressions.is_empty()
}

fn fold_logical_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_logical_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::LogicalExpression(logical_expr) = e else { return false };
if !logical_expr.operator.is_coalesce() {
self.try_fold_expr_in_boolean_context(&mut logical_expr.left, ctx);
if !logical_expr.operator.is_coalesce()
&& self.try_fold_expr_in_boolean_context(&mut logical_expr.left, ctx)
{
self.mark_current_function_as_changed();
}
if self.remove_unused_expression(&mut logical_expr.right, ctx) {
self.remove_unused_expression(&mut logical_expr.left, ctx);
*e = ctx.ast.move_expression(&mut logical_expr.left);
self.mark_current_function_as_changed();
}
false
}

// `([1,2,3, foo()])` -> `foo()`
fn fold_array_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_array_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::ArrayExpression(array_expr) = e else {
return false;
};
if array_expr.elements.len() == 0 {
return true;
}

let old_len = array_expr.elements.len();
array_expr.elements.retain_mut(|el| match el {
ArrayExpressionElement::SpreadElement(_) => el.may_have_side_effects(&ctx),
ArrayExpressionElement::Elision(_) => false,
Expand All @@ -88,6 +100,10 @@ impl<'a> PeepholeOptimizations {
!self.remove_unused_expression(el_expr, ctx)
}
});
if array_expr.elements.len() != old_len {
self.mark_current_function_as_changed();
}

if array_expr.elements.len() == 0 {
return true;
}
Expand Down Expand Up @@ -164,11 +180,16 @@ impl<'a> PeepholeOptimizations {
}

// "`${1}2${foo()}3`" -> "`${foo()}`"
fn fold_template_literal(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_template_literal(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::TemplateLiteral(temp_lit) = e else { return false };
if temp_lit.expressions.is_empty() {
return true;
}
if temp_lit.expressions.iter().all(|e| e.to_primitive(&ctx).is_symbol() != Some(false))
&& temp_lit.quasis.iter().all(|q| q.value.raw.is_empty())
{
return false;
}

let mut transformed_elements = ctx.ast.vec();
let mut pending_to_string_required_exprs = ctx.ast.vec();
Expand Down Expand Up @@ -229,21 +250,26 @@ impl<'a> PeepholeOptimizations {
return true;
} else if transformed_elements.len() == 1 {
*e = transformed_elements.pop().unwrap();
self.mark_current_function_as_changed();
return false;
}

*e = ctx.ast.expression_sequence(temp_lit.span, transformed_elements);
self.mark_current_function_as_changed();
false
}

// `({ 1: 1, [foo()]: bar() })` -> `foo(), bar()`
fn fold_object_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_object_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::ObjectExpression(object_expr) = e else {
return false;
};
if object_expr.properties.is_empty() {
return true;
}
if object_expr.properties.iter().all(ObjectPropertyKind::is_spread) {
return false;
}

let mut transformed_elements = ctx.ast.vec();
let mut pending_spread_elements = ctx.ast.vec();
Expand Down Expand Up @@ -296,14 +322,16 @@ impl<'a> PeepholeOptimizations {
return true;
} else if transformed_elements.len() == 1 {
*e = transformed_elements.pop().unwrap();
self.mark_current_function_as_changed();
return false;
}

*e = ctx.ast.expression_sequence(object_expr.span, transformed_elements);
self.mark_current_function_as_changed();
false
}

fn fold_conditional_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_conditional_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::ConditionalExpression(conditional_expr) = e else {
return false;
};
Expand All @@ -318,6 +346,7 @@ impl<'a> PeepholeOptimizations {
return true;
}
*e = ctx.ast.move_expression(&mut conditional_expr.test);
self.mark_current_function_as_changed();
return false;
}

Expand All @@ -330,6 +359,7 @@ impl<'a> PeepholeOptimizations {
ctx.ast.move_expression(&mut conditional_expr.alternate),
ctx,
);
self.mark_current_function_as_changed();
return false;
}

Expand All @@ -342,13 +372,14 @@ impl<'a> PeepholeOptimizations {
ctx.ast.move_expression(&mut conditional_expr.consequent),
ctx,
);
self.mark_current_function_as_changed();
return false;
}

false
}

fn fold_binary_expression(&self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_binary_expression(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::BinaryExpression(binary_expr) = e else {
return false;
};
Expand All @@ -368,10 +399,12 @@ impl<'a> PeepholeOptimizations {
(true, true) => true,
(true, false) => {
*e = ctx.ast.move_expression(&mut binary_expr.right);
self.mark_current_function_as_changed();
false
}
(false, true) => {
*e = ctx.ast.move_expression(&mut binary_expr.left);
self.mark_current_function_as_changed();
false
}
(false, false) => {
Expand All @@ -382,49 +415,57 @@ impl<'a> PeepholeOptimizations {
ctx.ast.move_expression(&mut binary_expr.right),
]),
);
self.mark_current_function_as_changed();
false
}
}
}
BinaryOperator::Addition => {
Self::fold_string_addition_chain(e, ctx);
self.fold_string_addition_chain(e, ctx);
matches!(e, Expression::StringLiteral(_))
}
_ => !e.may_have_side_effects(&ctx),
}
}

/// returns whether the passed expression is a string
fn fold_string_addition_chain(e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
fn fold_string_addition_chain(&mut self, e: &mut Expression<'a>, ctx: Ctx<'a, '_>) -> bool {
let Expression::BinaryExpression(binary_expr) = e else {
return e.to_primitive(&ctx).is_string() == Some(true);
};
if binary_expr.operator != BinaryOperator::Addition {
return e.to_primitive(&ctx).is_string() == Some(true);
}

let left_is_string = Self::fold_string_addition_chain(&mut binary_expr.left, ctx);
let left_is_string = self.fold_string_addition_chain(&mut binary_expr.left, ctx);
if left_is_string {
if !binary_expr.left.may_have_side_effects(&ctx) {
if !binary_expr.left.may_have_side_effects(&ctx)
&& !binary_expr.left.is_specific_string_literal("")
{
binary_expr.left =
ctx.ast.expression_string_literal(binary_expr.left.span(), "", None);
self.mark_current_function_as_changed();
}

let right_as_primitive = binary_expr.right.to_primitive(&ctx);
if right_as_primitive.is_symbol() == Some(false)
&& !binary_expr.right.may_have_side_effects(&ctx)
{
*e = ctx.ast.move_expression(&mut binary_expr.left);
self.mark_current_function_as_changed();
return true;
}
return true;
}

let right_as_primitive = binary_expr.right.to_primitive(&ctx);
if right_as_primitive.is_string() == Some(true) {
if !binary_expr.right.may_have_side_effects(&ctx) {
if !binary_expr.right.may_have_side_effects(&ctx)
&& !binary_expr.right.is_specific_string_literal("")
{
binary_expr.right =
ctx.ast.expression_string_literal(binary_expr.right.span(), "", None);
self.mark_current_function_as_changed();
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_minifier/tests/peephole/esbuild.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ fn js_parser_test() {
test("a ? b?.c : b.c", "a ? b?.c : b.c;");
test("a ? b?.() : b()", "a ? b?.() : b();");
test("a ? b?.[c] : b[c]", "a ? b?.[c] : b[c];");
// test("a ? b == c : b != c", "a ? (b, c) : (b, c);");
test("a ? b == c : b != c", "a, b, c;");
test("a ? b.c(d + e[f]) : b.c(d + e[g])", "a ? b.c(d + e[f]) : b.c(d + e[g]);");
test("(a, b) ? c : d", "a, b ? c : d;");
test(
Expand Down
Loading