diff --git a/crates/oxc_minifier/src/ast_passes/fold_constants.rs b/crates/oxc_minifier/src/ast_passes/fold_constants.rs index 662cbea5de906..752591a5b4862 100644 --- a/crates/oxc_minifier/src/ast_passes/fold_constants.rs +++ b/crates/oxc_minifier/src/ast_passes/fold_constants.rs @@ -53,7 +53,11 @@ impl<'a> FoldConstants<'a> { pub fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { if let Some(folded_expr) = match expr { Expression::BinaryExpression(e) => self.try_fold_binary_operator(e, ctx), - Expression::LogicalExpression(e) => self.try_fold_logical_expression(e, ctx), + Expression::LogicalExpression(e) + if matches!(e.operator, LogicalOperator::And | LogicalOperator::Or) => + { + self.try_fold_and_or(e, ctx) + } // TODO: move to `PeepholeMinimizeConditions` Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx), Expression::UnaryExpression(e) => self.try_fold_unary_expression(e, ctx), @@ -111,7 +115,7 @@ impl<'a> FoldConstants<'a> { ctx: &mut TraverseCtx<'a>, ) -> Option { self.fold_expression(expr, ctx); - ctx.get_boolean_value(expr) + ctx.get_boolean_value(expr).to_option() } fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { @@ -150,11 +154,15 @@ impl<'a> FoldConstants<'a> { expr: &mut ConditionalExpression<'a>, ctx: &mut TraverseCtx<'a>, ) -> Option> { - if ctx.ancestry.parent().is_tagged_template_expression() { - return None; - } match self.fold_expression_and_get_boolean_value(&mut expr.test, ctx) { - Some(true) => Some(self.ast.move_expression(&mut expr.consequent)), + Some(true) => { + // Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;` + let parent = ctx.ancestry.parent(); + if parent.is_tagged_template_expression() || parent.is_call_expression() { + return None; + } + Some(self.ast.move_expression(&mut expr.consequent)) + } Some(false) => Some(self.ast.move_expression(&mut expr.alternate)), _ => None, } @@ -384,7 +392,7 @@ impl<'a> FoldConstants<'a> { let right_bigint = ctx.get_side_free_bigint_value(right_expr); if let (Some(l_big), Some(r_big)) = (left_bigint, right_bigint) { - return Tri::for_boolean(l_big.eq(&r_big)); + return Tri::from(l_big.eq(&r_big)); } } @@ -421,7 +429,7 @@ impl<'a> FoldConstants<'a> { return Tri::Unknown; } - return Tri::for_boolean(left_string.cmp(&right_string) == Ordering::Less); + return Tri::from(left_string.cmp(&right_string) == Ordering::Less); } if let (Expression::UnaryExpression(left), Expression::UnaryExpression(right)) = @@ -450,14 +458,14 @@ impl<'a> FoldConstants<'a> { match (left_bigint, right_bigint, left_num, right_num) { // Next, try to evaluate based on the value of the node. Try comparing as BigInts first. (Some(l_big), Some(r_big), _, _) => { - return Tri::for_boolean(l_big < r_big); + return Tri::from(l_big < r_big); } // try comparing as Numbers. (_, _, Some(l_num), Some(r_num)) => match (l_num, r_num) { (NumberValue::NaN, _) | (_, NumberValue::NaN) => { - return Tri::for_boolean(will_negative); + return Tri::from(will_negative); } - (NumberValue::Number(l), NumberValue::Number(r)) => return Tri::for_boolean(l < r), + (NumberValue::Number(l), NumberValue::Number(r)) => return Tri::from(l < r), _ => {} }, // Finally, try comparisons between BigInt and Number. @@ -484,7 +492,7 @@ impl<'a> FoldConstants<'a> { // if invert is false, then the number is on the right in tryAbstractRelationalComparison // if it's true, then the number is on the left match number_value { - NumberValue::NaN => Tri::for_boolean(will_negative), + NumberValue::NaN => Tri::from(will_negative), NumberValue::PositiveInfinity => Tri::True.xor(invert), NumberValue::NegativeInfinity => Tri::False.xor(invert), NumberValue::Number(num) => { @@ -502,7 +510,7 @@ impl<'a> FoldConstants<'a> { if is_exact_int64(*num) { Tri::False } else { - Tri::for_boolean(num.is_sign_positive()).xor(invert) + Tri::from(num.is_sign_positive()).xor(invert) } } } @@ -534,7 +542,7 @@ impl<'a> FoldConstants<'a> { return Tri::False; } - return Tri::for_boolean(l_num == r_num); + return Tri::from(l_num == r_num); } Tri::Unknown @@ -548,7 +556,7 @@ impl<'a> FoldConstants<'a> { return Tri::Unknown; } - return Tri::for_boolean(left_string == right_string); + return Tri::from(left_string == right_string); } if let (Expression::UnaryExpression(left), Expression::UnaryExpression(right)) = @@ -640,22 +648,20 @@ impl<'a> FoldConstants<'a> { /// Try to fold a AND / OR node. /// /// port from [closure-compiler](https://github.com/google/closure-compiler/blob/09094b551915a6487a980a783831cba58b5739d1/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L587) - pub fn try_fold_logical_expression( + pub fn try_fold_and_or( &self, logical_expr: &mut LogicalExpression<'a>, ctx: &mut TraverseCtx<'a>, ) -> Option> { - if ctx.ancestry.parent().is_tagged_template_expression() { - return None; - } let op = logical_expr.operator; - if !matches!(op, LogicalOperator::And | LogicalOperator::Or) { - return None; - } + debug_assert!(matches!(op, LogicalOperator::And | LogicalOperator::Or)); - if let Some(boolean_value) = ctx.get_boolean_value(&logical_expr.left) { + let left = &logical_expr.left; + let left_val = ctx.get_boolean_value(left).to_option(); + + if let Some(lval) = left_val { // Bail `0 && (module.exports = {})` for `cjs-module-lexer`. - if !boolean_value { + if !lval { if let Expression::AssignmentExpression(assign_expr) = &logical_expr.right { if let Some(member_expr) = assign_expr.left.as_member_expression() { if member_expr.is_specific_member_access("module", "exports") { @@ -667,11 +673,14 @@ impl<'a> FoldConstants<'a> { // (TRUE || x) => TRUE (also, (3 || x) => 3) // (FALSE && x) => FALSE - if (boolean_value && op == LogicalOperator::Or) - || (!boolean_value && op == LogicalOperator::And) - { + if if lval { op == LogicalOperator::Or } else { op == LogicalOperator::And } { return Some(self.ast.move_expression(&mut logical_expr.left)); - } else if !logical_expr.left.may_have_side_effects() { + } else if !left.may_have_side_effects() { + let parent = ctx.ancestry.parent(); + // Bail `let o = { f() { assert.ok(this !== o); } }; (true && o.f)(); (true && o.f)``;` + if parent.is_tagged_template_expression() || parent.is_call_expression() { + return None; + } // (FALSE || x) => x // (TRUE && x) => x return Some(self.ast.move_expression(&mut logical_expr.right)); @@ -688,7 +697,7 @@ impl<'a> FoldConstants<'a> { return Some(sequence_expr); } else if let Expression::LogicalExpression(left_child) = &mut logical_expr.left { if left_child.operator == logical_expr.operator { - let left_child_right_boolean = ctx.get_boolean_value(&left_child.right); + let left_child_right_boolean = ctx.get_boolean_value(&left_child.right).to_option(); let left_child_op = left_child.operator; if let Some(right_boolean) = left_child_right_boolean { if !left_child.right.may_have_side_effects() { diff --git a/crates/oxc_minifier/src/node_util/check_for_state_change.rs b/crates/oxc_minifier/src/node_util/check_for_state_change.rs index 8f5f439a8e8cb..d0c4f66eb766c 100644 --- a/crates/oxc_minifier/src/node_util/check_for_state_change.rs +++ b/crates/oxc_minifier/src/node_util/check_for_state_change.rs @@ -6,10 +6,11 @@ fn is_simple_unary_operator(operator: UnaryOperator) -> bool { operator != UnaryOperator::Delete } -/// port from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L241) /// Returns true if some node in n's subtree changes application state. If /// `check_for_new_objects` is true, we assume that newly created mutable objects (like object /// literals) change state. Otherwise, we assume that they have no side effects. +/// +/// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L241) pub trait CheckForStateChange<'a, 'b> { fn check_for_state_change(&self, check_for_new_objects: bool) -> bool; } diff --git a/crates/oxc_minifier/src/node_util/may_have_side_effects.rs b/crates/oxc_minifier/src/node_util/may_have_side_effects.rs index e6ec638d381d7..32ff26a4b2043 100644 --- a/crates/oxc_minifier/src/node_util/may_have_side_effects.rs +++ b/crates/oxc_minifier/src/node_util/may_have_side_effects.rs @@ -2,16 +2,17 @@ use oxc_ast::ast::*; use super::check_for_state_change::CheckForStateChange; -/// port from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94) /// Returns true if the node which may have side effects when executed. /// This version default to the "safe" assumptions when the compiler object /// is not provided (RegExp have side-effects, etc). +/// +/// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94) pub trait MayHaveSideEffects<'a, 'b> where Self: CheckForStateChange<'a, 'b>, { fn may_have_side_effects(&self) -> bool { - self.check_for_state_change(false) + self.check_for_state_change(/* check_for_new_objects */ false) } } diff --git a/crates/oxc_minifier/src/node_util/mod.rs b/crates/oxc_minifier/src/node_util/mod.rs index c63fbcf9c16bc..c7ee550153a32 100644 --- a/crates/oxc_minifier/src/node_util/mod.rs +++ b/crates/oxc_minifier/src/node_util/mod.rs @@ -11,6 +11,8 @@ use oxc_ast::ast::*; use oxc_semantic::{IsGlobalReference, ScopeTree, SymbolTable}; use oxc_syntax::operator::{AssignmentOperator, LogicalOperator, UnaryOperator}; +use crate::tri::Tri; + pub use self::{ is_literal_value::IsLiteralValue, may_have_side_effects::MayHaveSideEffects, number_value::NumberValue, @@ -89,7 +91,7 @@ pub trait NodeUtil { /// Gets the boolean value of a node that represents an expression, or `None` if no /// such value can be determined by static analysis. /// This method does not consider whether the node may have side-effects. - fn get_boolean_value(&self, expr: &Expression) -> Option { + fn get_boolean_value(&self, expr: &Expression) -> Tri { match expr { Expression::RegExpLiteral(_) | Expression::ArrayExpression(_) @@ -97,12 +99,14 @@ pub trait NodeUtil { | Expression::ClassExpression(_) | Expression::FunctionExpression(_) | Expression::NewExpression(_) - | Expression::ObjectExpression(_) => Some(true), - Expression::NullLiteral(_) => Some(false), - Expression::BooleanLiteral(boolean_literal) => Some(boolean_literal.value), - Expression::NumericLiteral(number_literal) => Some(number_literal.value != 0.0), - Expression::BigIntLiteral(big_int_literal) => Some(!big_int_literal.is_zero()), - Expression::StringLiteral(string_literal) => Some(!string_literal.value.is_empty()), + | Expression::ObjectExpression(_) => Tri::True, + Expression::NullLiteral(_) => Tri::False, + Expression::BooleanLiteral(boolean_literal) => Tri::from(boolean_literal.value), + Expression::NumericLiteral(number_literal) => Tri::from(number_literal.value != 0.0), + Expression::BigIntLiteral(big_int_literal) => Tri::from(!big_int_literal.is_zero()), + Expression::StringLiteral(string_literal) => { + Tri::from(!string_literal.value.is_empty()) + } Expression::TemplateLiteral(template_literal) => { // only for `` template_literal @@ -111,16 +115,17 @@ pub trait NodeUtil { .filter(|quasi| quasi.tail) .and_then(|quasi| quasi.value.cooked.as_ref()) .map(|cooked| !cooked.is_empty()) + .into() } Expression::Identifier(ident) => match ident.name.as_str() { - "NaN" => Some(false), - "Infinity" => Some(true), - "undefined" if self.is_identifier_undefined(ident) => Some(false), - _ => None, + "NaN" => Tri::False, + "Infinity" => Tri::True, + "undefined" if self.is_identifier_undefined(ident) => Tri::False, + _ => Tri::Unknown, }, Expression::AssignmentExpression(assign_expr) => { match assign_expr.operator { - AssignmentOperator::LogicalAnd | AssignmentOperator::LogicalOr => None, + AssignmentOperator::LogicalAnd | AssignmentOperator::LogicalOr => Tri::Unknown, // For ASSIGN, the value is the value of the RHS. _ => self.get_boolean_value(&assign_expr.right), } @@ -133,36 +138,35 @@ pub trait NodeUtil { LogicalOperator::And => { let left = self.get_boolean_value(&logical_expr.left); let right = self.get_boolean_value(&logical_expr.right); - match (left, right) { - (Some(true), Some(true)) => Some(true), - (Some(false), _) | (_, Some(false)) => Some(false), - (None, _) | (_, None) => None, + (Tri::True, Tri::True) => Tri::True, + (Tri::False, _) | (_, Tri::False) => Tri::False, + (Tri::Unknown, _) | (_, Tri::Unknown) => Tri::Unknown, } } // true || false -> true // false || false -> false - // a || b -> None + // a || b -> Tri::Unknown LogicalOperator::Or => { let left = self.get_boolean_value(&logical_expr.left); let right = self.get_boolean_value(&logical_expr.right); match (left, right) { - (Some(true), _) | (_, Some(true)) => Some(true), - (Some(false), Some(false)) => Some(false), - (None, _) | (_, None) => None, + (Tri::True, _) | (_, Tri::True) => Tri::True, + (Tri::False, Tri::False) => Tri::False, + (Tri::Unknown, _) | (_, Tri::Unknown) => Tri::Unknown, } } - LogicalOperator::Coalesce => None, + LogicalOperator::Coalesce => Tri::Unknown, } } Expression::SequenceExpression(sequence_expr) => { // For sequence expression, the value is the value of the RHS. - sequence_expr.expressions.last().and_then(|e| self.get_boolean_value(e)) + sequence_expr.expressions.last().map_or(Tri::Unknown, |e| self.get_boolean_value(e)) } Expression::UnaryExpression(unary_expr) => { if unary_expr.operator == UnaryOperator::Void { - Some(false) + Tri::False } else if matches!( unary_expr.operator, UnaryOperator::BitwiseNot @@ -173,15 +177,17 @@ pub trait NodeUtil { // +1 -> true // +0 -> false // -0 -> false - self.get_number_value(expr).map(|value| value != NumberValue::Number(0_f64)) + self.get_number_value(expr) + .map(|value| value != NumberValue::Number(0_f64)) + .into() } else if unary_expr.operator == UnaryOperator::LogicalNot { // !true -> false - self.get_boolean_value(&unary_expr.argument).map(|boolean| !boolean) + self.get_boolean_value(&unary_expr.argument).not() } else { - None + Tri::Unknown } } - _ => None, + _ => Tri::Unknown, } } @@ -213,7 +219,7 @@ pub trait NodeUtil { } UnaryOperator::LogicalNot => self .get_boolean_value(expr) - .map(|boolean| if boolean { 1_f64 } else { 0_f64 }) + .map(|tri| if tri.is_true() { 1_f64 } else { 0_f64 }) .map(NumberValue::Number), UnaryOperator::Void => Some(NumberValue::NaN), _ => None, @@ -264,7 +270,7 @@ pub trait NodeUtil { } Expression::UnaryExpression(unary_expr) => match unary_expr.operator { UnaryOperator::LogicalNot => self.get_boolean_value(expr).map(|boolean| { - if boolean { + if boolean.is_true() { BigInt::one() } else { BigInt::zero() @@ -336,7 +342,7 @@ pub trait NodeUtil { UnaryOperator::LogicalNot => { self.get_boolean_value(&unary_expr.argument).map(|boolean| { // need reversed. - if boolean { + if boolean.is_true() { Cow::Borrowed("false") } else { Cow::Borrowed("true") diff --git a/crates/oxc_minifier/src/tri.rs b/crates/oxc_minifier/src/tri.rs index bfad8945c6517..815ae3413b9da 100644 --- a/crates/oxc_minifier/src/tri.rs +++ b/crates/oxc_minifier/src/tri.rs @@ -6,7 +6,48 @@ pub enum Tri { Unknown, } +impl From for Tri { + fn from(value: bool) -> Self { + if value { + Tri::True + } else { + Tri::False + } + } +} + +impl From> for Tri { + fn from(value: Option) -> Self { + value.map_or(Tri::Unknown, From::from) + } +} + +impl From for Tri { + fn from(value: i8) -> Self { + match value { + -1 => Self::False, + 1 => Self::True, + _ => Self::Unknown, + } + } +} + impl Tri { + pub fn is_true(self) -> bool { + self == Tri::True + } + + pub fn map(self, f: F) -> Option + where + F: FnOnce(Tri) -> U, + { + match self { + Self::True => Some(f(Tri::True)), + Self::False => Some(f(Tri::False)), + Self::Unknown => None, + } + } + pub fn not(self) -> Self { match self { Self::True => Self::False, @@ -16,22 +57,14 @@ impl Tri { } pub fn xor(self, other: Self) -> Self { - Self::for_int(-self.value() * other.value()) - } - - pub fn for_int(int: i8) -> Self { - match int { - -1 => Self::False, - 1 => Self::True, - _ => Self::Unknown, - } + Self::from(-self.value() * other.value()) } - pub fn for_boolean(boolean: bool) -> Self { - if boolean { - Self::True - } else { - Self::False + pub fn to_option(self) -> Option { + match self { + Self::True => Some(true), + Self::False => Some(false), + Self::Unknown => None, } } diff --git a/crates/oxc_minifier/tests/ast_passes/fold_constants.rs b/crates/oxc_minifier/tests/ast_passes/fold_constants.rs index 2fa64ad4521fc..68e54fa19e544 100644 --- a/crates/oxc_minifier/tests/ast_passes/fold_constants.rs +++ b/crates/oxc_minifier/tests/ast_passes/fold_constants.rs @@ -27,11 +27,9 @@ fn cjs() { fn tagged_template() { test_same("(1, o.f)()"); test_same("(1, o.f)``"); - - test("(true && o.f)()", "o.f()"); + test_same("(true && o.f)()"); test_same("(true && o.f)``"); - - test("(true ? o.f : false)()", "o.f()"); + test_same("(true ? o.f : false)()"); test_same("(true ? o.f : false)``"); }