From 9b5d800f450f2b12a111edb46593c51ba0601ace Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Mon, 10 Feb 2025 14:09:05 +0000 Subject: [PATCH] refactor(minifier): move equality comparison to ConstantEvaluation (#9009) --- .../equality_comparison.rs | 130 ++++++++++++ .../src/constant_evaluation/mod.rs | 32 ++- crates/oxc_minifier/src/ctx.rs | 8 +- .../src/peephole/fold_constants.rs | 188 +----------------- 4 files changed, 170 insertions(+), 188 deletions(-) create mode 100644 crates/oxc_ecmascript/src/constant_evaluation/equality_comparison.rs diff --git a/crates/oxc_ecmascript/src/constant_evaluation/equality_comparison.rs b/crates/oxc_ecmascript/src/constant_evaluation/equality_comparison.rs new file mode 100644 index 0000000000000..31fee6ffe9fe6 --- /dev/null +++ b/crates/oxc_ecmascript/src/constant_evaluation/equality_comparison.rs @@ -0,0 +1,130 @@ +use oxc_ast::ast::{Expression, NumberBase}; + +use super::{ConstantEvaluation, ValueType}; + +/// +pub(super) fn abstract_equality_comparison<'a>( + c: &impl ConstantEvaluation<'a>, + left_expr: &Expression<'a>, + right_expr: &Expression<'a>, +) -> Option { + let left = ValueType::from(left_expr); + let right = ValueType::from(right_expr); + if left != ValueType::Undetermined && right != ValueType::Undetermined { + if left == right { + return strict_equality_comparison(c, left_expr, right_expr); + } + if matches!( + (left, right), + (ValueType::Null, ValueType::Undefined) | (ValueType::Undefined, ValueType::Null) + ) { + return Some(true); + } + + if matches!((left, right), (ValueType::Number, ValueType::String)) + || matches!(right, ValueType::Boolean) + { + if let Some(num) = c.get_side_free_number_value(right_expr) { + let number_literal_expr = c.ast().expression_numeric_literal( + oxc_span::SPAN, + num, + None, + if num.fract() == 0.0 { NumberBase::Decimal } else { NumberBase::Float }, + ); + return abstract_equality_comparison(c, left_expr, &number_literal_expr); + } + return None; + } + + if matches!((left, right), (ValueType::String, ValueType::Number)) + || matches!(left, ValueType::Boolean) + { + if let Some(num) = c.get_side_free_number_value(left_expr) { + let number_literal_expr = c.ast().expression_numeric_literal( + oxc_span::SPAN, + num, + None, + if num.fract() == 0.0 { NumberBase::Decimal } else { NumberBase::Float }, + ); + return abstract_equality_comparison(c, &number_literal_expr, right_expr); + } + return None; + } + + if matches!(left, ValueType::BigInt) || matches!(right, ValueType::BigInt) { + let left_bigint = c.get_side_free_bigint_value(left_expr); + let right_bigint = c.get_side_free_bigint_value(right_expr); + if let (Some(l_big), Some(r_big)) = (left_bigint, right_bigint) { + return Some(l_big.eq(&r_big)); + } + } + + if matches!(left, ValueType::String | ValueType::Number | ValueType::BigInt) + && matches!(right, ValueType::Object) + { + return None; + } + + if matches!(left, ValueType::Object) + && matches!(right, ValueType::String | ValueType::Number | ValueType::BigInt) + { + return None; + } + + return Some(false); + } + None +} + +/// +#[expect(clippy::float_cmp)] +pub(super) fn strict_equality_comparison<'a>( + c: &impl ConstantEvaluation<'a>, + left_expr: &Expression<'a>, + right_expr: &Expression<'a>, +) -> Option { + let left = ValueType::from(left_expr); + let right = ValueType::from(right_expr); + if !left.is_undetermined() && !right.is_undetermined() { + // Strict equality can only be true for values of the same type. + if left != right { + return Some(false); + } + return match left { + ValueType::Number => { + let lnum = c.get_side_free_number_value(left_expr)?; + let rnum = c.get_side_free_number_value(right_expr)?; + if lnum.is_nan() || rnum.is_nan() { + return Some(false); + } + Some(lnum == rnum) + } + ValueType::String => { + let left = c.get_side_free_string_value(left_expr)?; + let right = c.get_side_free_string_value(right_expr)?; + Some(left == right) + } + ValueType::Undefined | ValueType::Null => Some(true), + ValueType::Boolean => { + let left = c.get_boolean_value(left_expr)?; + let right = c.get_boolean_value(right_expr)?; + Some(left == right) + } + ValueType::BigInt => { + let left = c.get_side_free_bigint_value(left_expr)?; + let right = c.get_side_free_bigint_value(right_expr)?; + Some(left == right) + } + ValueType::Object => None, + ValueType::Undetermined => unreachable!(), + }; + } + + // Then, try to evaluate based on the value of the expression. + // There's only one special case: + // Any strict equality comparison against NaN returns false. + if left_expr.is_nan() || right_expr.is_nan() { + return Some(false); + } + None +} diff --git a/crates/oxc_ecmascript/src/constant_evaluation/mod.rs b/crates/oxc_ecmascript/src/constant_evaluation/mod.rs index 1a930f14b924a..5c59693612838 100644 --- a/crates/oxc_ecmascript/src/constant_evaluation/mod.rs +++ b/crates/oxc_ecmascript/src/constant_evaluation/mod.rs @@ -3,10 +3,12 @@ use std::{borrow::Cow, cmp::Ordering}; use num_bigint::BigInt; use num_traits::{ToPrimitive, Zero}; -use oxc_ast::ast::*; +use equality_comparison::{abstract_equality_comparison, strict_equality_comparison}; +use oxc_ast::{ast::*, AstBuilder}; use crate::{side_effects::MayHaveSideEffects, ToBigInt, ToBoolean, ToInt32, ToJsString, ToNumber}; +mod equality_comparison; mod is_literal_value; mod value; mod value_type; @@ -15,6 +17,8 @@ pub use value::ConstantValue; pub use value_type::ValueType; pub trait ConstantEvaluation<'a>: MayHaveSideEffects { + fn ast(&self) -> AstBuilder<'a>; + fn resolve_binding(&self, ident: &IdentifierReference<'a>) -> Option> { match ident.name.as_str() { "undefined" if self.is_global_reference(ident) => Some(ConstantValue::Undefined), @@ -374,7 +378,31 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects { } None } - _ => None, + BinaryOperator::StrictEquality + | BinaryOperator::StrictInequality + | BinaryOperator::Equality + | BinaryOperator::Inequality => { + if self.expression_may_have_side_effects(left) + || self.expression_may_have_side_effects(right) + { + return None; + } + let value = match operator { + BinaryOperator::StrictEquality | BinaryOperator::StrictInequality => { + strict_equality_comparison(self, left, right)? + } + BinaryOperator::Equality | BinaryOperator::Inequality => { + abstract_equality_comparison(self, left, right)? + } + _ => unreachable!(), + }; + Some(ConstantValue::Boolean(match operator { + BinaryOperator::StrictEquality | BinaryOperator::Equality => value, + BinaryOperator::StrictInequality | BinaryOperator::Inequality => !value, + _ => unreachable!(), + })) + } + BinaryOperator::In => None, } } diff --git a/crates/oxc_minifier/src/ctx.rs b/crates/oxc_minifier/src/ctx.rs index 00e2816658b3d..bdccd4a4dadf1 100644 --- a/crates/oxc_minifier/src/ctx.rs +++ b/crates/oxc_minifier/src/ctx.rs @@ -1,6 +1,6 @@ use std::ops::Deref; -use oxc_ast::ast::*; +use oxc_ast::{ast::*, AstBuilder}; use oxc_ecmascript::{ constant_evaluation::{ConstantEvaluation, ConstantValue}, side_effects::MayHaveSideEffects, @@ -19,7 +19,11 @@ impl<'a, 'b> Deref for Ctx<'a, 'b> { } } -impl<'a> ConstantEvaluation<'a> for Ctx<'a, '_> {} +impl<'a> ConstantEvaluation<'a> for Ctx<'a, '_> { + fn ast(&self) -> AstBuilder<'a> { + self.ast + } +} impl MayHaveSideEffects for Ctx<'_, '_> { fn is_global_reference(&self, ident: &IdentifierReference<'_>) -> bool { diff --git a/crates/oxc_minifier/src/peephole/fold_constants.rs b/crates/oxc_minifier/src/peephole/fold_constants.rs index 6bbc140205fa5..8750020538b42 100644 --- a/crates/oxc_minifier/src/peephole/fold_constants.rs +++ b/crates/oxc_minifier/src/peephole/fold_constants.rs @@ -4,10 +4,7 @@ use oxc_ecmascript::{ side_effects::MayHaveSideEffects, }; use oxc_span::GetSpan; -use oxc_syntax::{ - number::NumberBase, - operator::{BinaryOperator, LogicalOperator}, -}; +use oxc_syntax::operator::{BinaryOperator, LogicalOperator}; use oxc_traverse::Ancestor; use crate::ctx::Ctx; @@ -229,7 +226,9 @@ impl<'a> PeepholeOptimizations { | BinaryOperator::LessThan | BinaryOperator::GreaterThan | BinaryOperator::LessEqualThan - | BinaryOperator::GreaterEqualThan => Self::try_fold_comparison(e, ctx), + | BinaryOperator::GreaterEqualThan + | BinaryOperator::ShiftRight + | BinaryOperator::Instanceof => ctx.eval_binary(e), BinaryOperator::BitwiseAnd | BinaryOperator::BitwiseOR | BinaryOperator::BitwiseXOR => { ctx.eval_binary(e).or_else(|| Self::try_fold_left_child_op(e, ctx)) } @@ -288,7 +287,6 @@ impl<'a> PeepholeOptimizations { } None } - BinaryOperator::ShiftRight | BinaryOperator::Instanceof => ctx.eval_binary(e), BinaryOperator::In => None, } } @@ -373,184 +371,6 @@ impl<'a> PeepholeOptimizations { )) } - fn try_fold_comparison(e: &BinaryExpression<'a>, ctx: Ctx<'a, '_>) -> Option> { - let left = &e.left; - let right = &e.right; - let op = e.operator; - if ctx.expression_may_have_side_effects(left) || ctx.expression_may_have_side_effects(right) - { - return None; - } - let value = match op { - BinaryOperator::LessThan - | BinaryOperator::GreaterThan - | BinaryOperator::LessEqualThan - | BinaryOperator::GreaterEqualThan => { - return ctx.eval_binary_expression(e).map(|v| ctx.value_to_expr(e.span, v)); - } - BinaryOperator::Equality => Self::try_abstract_equality_comparison(left, right, ctx), - BinaryOperator::Inequality => { - Self::try_abstract_equality_comparison(left, right, ctx).map(|b| !b) - } - BinaryOperator::StrictEquality => { - Self::try_strict_equality_comparison(left, right, ctx) - } - BinaryOperator::StrictInequality => { - Self::try_strict_equality_comparison(left, right, ctx).map(|b| !b) - } - _ => None, - }; - let value = match value { - Some(true) => true, - Some(false) => false, - None => return None, - }; - Some(ctx.ast.expression_boolean_literal(e.span, value)) - } - - /// - fn try_abstract_equality_comparison( - left_expr: &Expression<'a>, - right_expr: &Expression<'a>, - ctx: Ctx<'a, '_>, - ) -> Option { - let left = ValueType::from(left_expr); - let right = ValueType::from(right_expr); - if left != ValueType::Undetermined && right != ValueType::Undetermined { - if left == right { - return Self::try_strict_equality_comparison(left_expr, right_expr, ctx); - } - if matches!( - (left, right), - (ValueType::Null, ValueType::Undefined) | (ValueType::Undefined, ValueType::Null) - ) { - return Some(true); - } - - if matches!((left, right), (ValueType::Number, ValueType::String)) - || matches!(right, ValueType::Boolean) - { - let right_number = ctx.get_side_free_number_value(right_expr); - - if let Some(num) = right_number { - let number_literal_expr = ctx.ast.expression_numeric_literal( - right_expr.span(), - num, - None, - if num.fract() == 0.0 { NumberBase::Decimal } else { NumberBase::Float }, - ); - - return Self::try_abstract_equality_comparison( - left_expr, - &number_literal_expr, - ctx, - ); - } - - return None; - } - - if matches!((left, right), (ValueType::String, ValueType::Number)) - || matches!(left, ValueType::Boolean) - { - let left_number = ctx.get_side_free_number_value(left_expr); - - if let Some(num) = left_number { - let number_literal_expr = ctx.ast.expression_numeric_literal( - left_expr.span(), - num, - None, - if num.fract() == 0.0 { NumberBase::Decimal } else { NumberBase::Float }, - ); - - return Self::try_abstract_equality_comparison( - &number_literal_expr, - right_expr, - ctx, - ); - } - - return None; - } - - if matches!(left, ValueType::BigInt) || matches!(right, ValueType::BigInt) { - let left_bigint = ctx.get_side_free_bigint_value(left_expr); - let right_bigint = ctx.get_side_free_bigint_value(right_expr); - - if let (Some(l_big), Some(r_big)) = (left_bigint, right_bigint) { - return Some(l_big.eq(&r_big)); - } - } - - if matches!(left, ValueType::String | ValueType::Number | ValueType::BigInt) - && matches!(right, ValueType::Object) - { - return None; - } - - if matches!(left, ValueType::Object) - && matches!(right, ValueType::String | ValueType::Number | ValueType::BigInt) - { - return None; - } - - return Some(false); - } - None - } - - /// - #[expect(clippy::float_cmp)] - fn try_strict_equality_comparison( - left_expr: &Expression<'a>, - right_expr: &Expression<'a>, - ctx: Ctx<'a, '_>, - ) -> Option { - let left = ValueType::from(left_expr); - let right = ValueType::from(right_expr); - if !left.is_undetermined() && !right.is_undetermined() { - // Strict equality can only be true for values of the same type. - if left != right { - return Some(false); - } - return match left { - ValueType::Number => { - let lnum = ctx.get_side_free_number_value(left_expr)?; - let rnum = ctx.get_side_free_number_value(right_expr)?; - if lnum.is_nan() || rnum.is_nan() { - return Some(false); - } - Some(lnum == rnum) - } - ValueType::String => { - let left = ctx.get_side_free_string_value(left_expr)?; - let right = ctx.get_side_free_string_value(right_expr)?; - Some(left == right) - } - ValueType::Undefined | ValueType::Null => Some(true), - ValueType::Boolean if right.is_boolean() => { - let left = ctx.get_boolean_value(left_expr)?; - let right = ctx.get_boolean_value(right_expr)?; - Some(left == right) - } - ValueType::BigInt => { - let left = ctx.get_side_free_bigint_value(left_expr)?; - let right = ctx.get_side_free_bigint_value(right_expr)?; - Some(left == right) - } - ValueType::Object | ValueType::Boolean | ValueType::Undetermined => None, - }; - } - - // Then, try to evaluate based on the value of the expression. - // There's only one special case: - // Any strict equality comparison against NaN returns false. - if left_expr.is_nan() || right_expr.is_nan() { - return Some(false); - } - None - } - fn try_fold_number_constructor( e: &CallExpression<'a>, ctx: Ctx<'a, '_>,