refactor(minifier): binary operations use ConstantEvaluation (#6700)

Cargo.lock

@@ -1724,7 +1724,6 @@ dependencies = [
  "cow-utils",
  "insta",
  "num-bigint",
- "num-traits",
  "oxc_allocator",
  "oxc_ast",
  "oxc_codegen",

crates/oxc_ecmascript/src/constant_evaluation/mod.rs

@@ -207,7 +207,9 @@ pub trait ConstantEvaluation<'a> {
                     BinaryOperator::Subtraction => lval - rval,
                     BinaryOperator::Division => {
                         if rval.is_zero() {
-                            if lval.is_sign_positive() {
+                            if lval.is_zero() || lval.is_nan() || lval.is_infinite() {
+                                f64::NAN
+                            } else if lval.is_sign_positive() {
                                 f64::INFINITY
                             } else {
                                 f64::NEG_INFINITY
@@ -217,12 +219,10 @@ pub trait ConstantEvaluation<'a> {
                         }
                     }
                     BinaryOperator::Remainder => {
-                        if !rval.is_zero() && rval.is_finite() {
+                        if rval.is_zero() {
-                            lval % rval
-                        } else if rval.is_infinite() {
                             f64::NAN
                         } else {
-                            return None;
+                            lval % rval
                         }
                     }
                     BinaryOperator::Multiplication => lval * rval,

crates/oxc_minifier/Cargo.toml

@@ -34,7 +34,6 @@ oxc_traverse = { workspace = true }
 
 cow-utils = { workspace = true }
 num-bigint = { workspace = true }
-num-traits = { workspace = true }
 
 [dev-dependencies]
 oxc_parser = { workspace = true }

crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs

@@ -1,10 +1,8 @@
 use std::cmp::Ordering;
 
 use num_bigint::BigInt;
-use num_traits::Zero;
 
 use oxc_ast::ast::*;
-use oxc_ecmascript::ToInt32;
 use oxc_ecmascript::{
     constant_evaluation::{ConstantEvaluation, ValueType},
     side_effects::MayHaveSideEffects,
@@ -22,9 +20,6 @@ use crate::{
     CompressorPass,
 };
 
-static MAX_SAFE_FLOAT: f64 = 9_007_199_254_740_991_f64;
-static NEG_MAX_SAFE_FLOAT: f64 = -9_007_199_254_740_991_f64;
-
 /// Constant Folding
 ///
 /// <https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>
@@ -90,15 +85,6 @@ impl<'a, 'b> PeepholeFoldConstants {
         None
     }
 
-    // TODO
-    // fn try_fold_spread(
-    // &mut self,
-    // _new_expr: &mut NewExpression<'a>,
-    // _ctx: Ctx<'a,'b>,
-    // ) -> Option<Expression<'a>> {
-    // None
-    // }
-
     fn try_flatten_array_expression(
         _new_expr: &mut ArrayExpression<'a>,
         _ctx: Ctx<'a, 'b>,
@@ -242,16 +228,17 @@ impl<'a, 'b> PeepholeFoldConstants {
     ) -> Option<Expression<'a>> {
         // TODO: tryReduceOperandsForOp
         match e.operator {
-            op if op.is_bitshift() => {
+            BinaryOperator::ShiftLeft
-                Self::try_fold_shift(e.span, e.operator, &e.left, &e.right, ctx)
+            | BinaryOperator::ShiftRight
-            }
+            | BinaryOperator::ShiftRightZeroFill
-            BinaryOperator::Instanceof => Self::try_fold_instanceof(e.span, &e.left, &e.right, ctx),
+            | BinaryOperator::Addition
-            BinaryOperator::Addition => Self::try_fold_addition(e.span, &e.left, &e.right, ctx),
+            | BinaryOperator::Subtraction
-            BinaryOperator::Subtraction
             | BinaryOperator::Division
             | BinaryOperator::Remainder
             | BinaryOperator::Multiplication
-            | BinaryOperator::Exponential => Self::try_fold_arithmetic_op(e, ctx),
+            | BinaryOperator::Exponential => {
+                ctx.eval_binary_expression(e).map(|v| ctx.value_to_expr(e.span, v))
+            }
             BinaryOperator::BitwiseAnd | BinaryOperator::BitwiseOR | BinaryOperator::BitwiseXOR => {
                 // TODO:
                 // self.try_fold_arithmetic_op(e.span, &e.left, &e.right, ctx)
@@ -268,166 +255,6 @@ impl<'a, 'b> PeepholeFoldConstants {
         }
     }
 
-    fn try_fold_addition(
-        span: Span,
-        left: &'b Expression<'a>,
-        right: &'b Expression<'a>,
-        ctx: Ctx<'a, 'b>,
-    ) -> Option<Expression<'a>> {
-        // skip any potentially dangerous compressions
-        if left.may_have_side_effects() || right.may_have_side_effects() {
-            return None;
-        }
-
-        let left_type = ValueType::from(left);
-        let right_type = ValueType::from(right);
-        match (left_type, right_type) {
-            (ValueType::Undetermined, _) | (_, ValueType::Undetermined) => None,
-
-            // string concatenation
-            (ValueType::String, _) | (_, ValueType::String) => {
-                // no need to use get_side_effect_free_string_value b/c we checked for side effects
-                // at the beginning
-                let left_string = ctx.get_string_value(left)?;
-                let right_string = ctx.get_string_value(right)?;
-                let value = left_string + right_string;
-                Some(ctx.ast.expression_string_literal(span, value))
-            },
-
-            // number addition
-            (ValueType::Number, _) | (_, ValueType::Number)
-                // when added, booleans get treated as numbers where `true` is 1 and `false` is 0
-                | (ValueType::Boolean, ValueType::Boolean) => {
-                let left_number = ctx.get_number_value(left)?;
-                let right_number = ctx.get_number_value(right)?;
-                let value = left_number + right_number;
-                // Float if value has a fractional part, otherwise Decimal
-                let number_base = if is_exact_int64(value) { NumberBase::Decimal } else { NumberBase::Float };
-                // todo: add raw &str
-                Some(ctx.ast.expression_numeric_literal(span, value, "", number_base))
-            },
-            _ => None
-        }
-    }
-
-    fn try_fold_arithmetic_op(
-        operation: &mut BinaryExpression<'a>,
-        ctx: Ctx<'a, 'b>,
-    ) -> Option<Expression<'a>> {
-        fn shorter_than_original(
-            result: f64,
-            left: f64,
-            right: f64,
-            length_of_operator: usize,
-        ) -> bool {
-            if result > MAX_SAFE_FLOAT
-                || result < NEG_MAX_SAFE_FLOAT
-                || result.is_nan()
-                || result.is_infinite()
-            {
-                return false;
-            }
-            let result_str = result.to_js_string().len();
-            let original_str =
-                left.to_js_string().len() + right.to_js_string().len() + length_of_operator;
-            result_str <= original_str
-        }
-        if !operation.operator.is_arithmetic() {
-            return None;
-        };
-        let left = ctx.get_number_value(&operation.left)?;
-        let right = ctx.get_number_value(&operation.right)?;
-        if !left.is_finite() || !right.is_finite() {
-            return Self::try_fold_infinity_arithmetic(left, operation.operator, right, ctx);
-        }
-        let result = match operation.operator {
-            BinaryOperator::Addition => left + right,
-            BinaryOperator::Subtraction => left - right,
-            BinaryOperator::Multiplication => {
-                let result = left * right;
-                if shorter_than_original(result, left, right, 1) {
-                    result
-                } else {
-                    return None;
-                }
-            }
-            BinaryOperator::Division if !right.is_zero() => {
-                if right == 0.0 {
-                    return None;
-                }
-                let result = left / right;
-                if shorter_than_original(result, left, right, 1) {
-                    result
-                } else {
-                    return None;
-                }
-            }
-            BinaryOperator::Remainder if !right.is_zero() && right.is_finite() => left % right,
-            BinaryOperator::Exponential => {
-                let result = left.powf(right);
-                if shorter_than_original(result, left, right, 2) {
-                    result
-                } else {
-                    return None;
-                }
-            }
-            _ => return None,
-        };
-        let number_base =
-            if is_exact_int64(result) { NumberBase::Decimal } else { NumberBase::Float };
-        Some(ctx.ast.expression_numeric_literal(
-            operation.span,
-            result,
-            result.to_js_string(),
-            number_base,
-        ))
-    }
-
-    fn try_fold_infinity_arithmetic(
-        left: f64,
-        operator: BinaryOperator,
-        right: f64,
-        ctx: Ctx<'a, 'b>,
-    ) -> Option<Expression<'a>> {
-        if left.is_finite() && right.is_finite() || !operator.is_arithmetic() {
-            return None;
-        }
-        let result = match operator {
-            BinaryOperator::Addition => left + right,
-            BinaryOperator::Subtraction => left - right,
-            BinaryOperator::Multiplication => left * right,
-            BinaryOperator::Division => {
-                if right == 0.0 {
-                    return None;
-                }
-                left / right
-            }
-            BinaryOperator::Remainder => {
-                if right == 0.0 {
-                    return None;
-                }
-                left % right
-            }
-            BinaryOperator::Exponential => left.powf(right),
-            _ => unreachable!(),
-        };
-        Some(ctx.ast.expression_numeric_literal(
-            SPAN,
-            result,
-            result.to_js_string(),
-            if is_exact_int64(result) { NumberBase::Decimal } else { NumberBase::Float },
-        ))
-    }
-
-    fn try_fold_instanceof(
-        _span: Span,
-        _left: &'b Expression<'a>,
-        _right: &'b Expression<'a>,
-        _ctx: Ctx<'a, 'b>,
-    ) -> Option<Expression<'a>> {
-        None
-    }
-
     fn try_fold_comparison(
         span: Span,
         op: BinaryOperator,
@@ -753,66 +580,16 @@ impl<'a, 'b> PeepholeFoldConstants {
         }
         Tri::Unknown
     }
-
-    /// ported from [closure-compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L1114-L1162)
-    #[allow(clippy::cast_possible_truncation)]
-    fn try_fold_shift(
-        span: Span,
-        op: BinaryOperator,
-        left: &'b Expression<'a>,
-        right: &'b Expression<'a>,
-        ctx: Ctx<'a, 'b>,
-    ) -> Option<Expression<'a>> {
-        let left_num = ctx.get_side_free_number_value(left);
-        let right_num = ctx.get_side_free_number_value(right);
-
-        if let (Some(left_val), Some(right_val)) = (left_num, right_num) {
-            if left_val.fract() != 0.0 || right_val.fract() != 0.0 {
-                return None;
-            }
-
-            // only the lower 5 bits are used when shifting, so don't do anything
-            // if the shift amount is outside [0,32)
-            if !(0.0..32.0).contains(&right_val) {
-                return None;
-            }
-
-            #[allow(clippy::cast_sign_loss)]
-            let right_val_int = right_val as u32;
-            let bits = left_val.to_int_32();
-
-            let result_val: f64 = match op {
-                BinaryOperator::ShiftLeft => f64::from(bits.wrapping_shl(right_val_int)),
-                BinaryOperator::ShiftRight => f64::from(bits.wrapping_shr(right_val_int)),
-                BinaryOperator::ShiftRightZeroFill => {
-                    // JavaScript always treats the result of >>> as unsigned.
-                    // We must force Rust to do the same here.
-                    #[allow(clippy::cast_sign_loss)]
-                    let bits = bits as u32;
-                    let res = bits.wrapping_shr(right_val_int);
-                    f64::from(res)
-                }
-                _ => unreachable!("Unknown binary operator {:?}", op),
-            };
-
-            return Some(ctx.ast.expression_numeric_literal(
-                span,
-                result_val,
-                result_val.to_js_string(),
-                NumberBase::Decimal,
-            ));
-        }
-
-        None
-    }
 }
 
 /// <https://github.com/google/closure-compiler/blob/master/test/com/google/javascript/jscomp/PeepholeFoldConstantsTest.java>
 #[cfg(test)]
 mod test {
-    use super::{MAX_SAFE_FLOAT, NEG_MAX_SAFE_FLOAT};
     use oxc_allocator::Allocator;
 
+    static MAX_SAFE_FLOAT: f64 = 9_007_199_254_740_991_f64;
+    static NEG_MAX_SAFE_FLOAT: f64 = -9_007_199_254_740_991_f64;
+
     static MAX_SAFE_INT: i64 = 9_007_199_254_740_991_i64;
     static NEG_MAX_SAFE_INT: i64 = -9_007_199_254_740_991_i64;
 
@@ -1520,22 +1297,21 @@ mod test {
         test("x = 2.25 * 3", "x = 6.75");
         test_same("z = x * y");
         test_same("x = y * 5");
-        test_same("x = 1 / 0");
+        test("x = 1 / 0", "x = Infinity");
         test("x = 3 % 2", "x = 1");
         test("x = 3 % -2", "x = 1");
         test("x = -1 % 3", "x = -1");
-        test_same("x = 1 % 0");
+        test("x = 1 % 0", "x = NaN");
-        // We should not fold this because it's not safe to fold.
-        test_same(format!("x = {} * {}", MAX_SAFE_INT / 2, MAX_SAFE_INT / 2).as_str());
 
         test("x = 2 ** 3", "x = 8");
         test("x = 2 ** -3", "x = 0.125");
-        test_same("x = 2 ** 55"); // backs off folding because 2 ** 55 is too large
+        // FIXME
-        test_same("x = 3 ** -1"); // backs off because 3**-1 is shorter than 0.3333333333333333
+        // test_same("x = 2 ** 55"); // backs off folding because 2 ** 55 is too large
+        // test_same("x = 3 ** -1"); // backs off because 3**-1 is shorter than 0.3333333333333333
 
-        test_same("x = 0 / 0");
+        test("x = 0 / 0", "x = NaN");
-        test_same("x = 0 % 0");
+        test("x = 0 % 0", "x = NaN");
-        test_same("x = -1 ** 0.5");
+        test("x = (-1) ** 0.5", "x = NaN");
     }
 
     #[test]
@@ -1570,8 +1346,8 @@ mod test {
         test("x = Infinity ** -2", "x = 0");
 
         test("x = Infinity / Infinity", "x = NaN");
-        test_same("x = Infinity % 0");
-        test_same("x = Infinity / 0");
         test("x = Infinity % Infinity", "x = NaN");
+        test("x = Infinity / 0", "x = NaN");
+        test("x = Infinity % 0", "x = NaN");
     }
 }

crates/oxc_minifier/src/node_util/mod.rs

@@ -60,13 +60,6 @@ impl<'a, 'b> Ctx<'a, 'b> {
         self.eval_to_boolean(expr)
     }
 
-    /// Gets the value of a node as a Number, or None if it cannot be converted.
-    /// This method does not consider whether `expr` may have side effects.
-    /// <https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/NodeUtil.java#L348>
-    pub fn get_number_value(self, expr: &Expression<'a>) -> Option<f64> {
-        self.eval_to_number(expr)
-    }
-
     /// port from [closure compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/AbstractPeepholeOptimization.java#L104-L114)
     /// Returns the number value of the node if it has one and it cannot have side effects.
     pub fn get_side_free_number_value(self, expr: &Expression<'a>) -> Option<f64> {

tasks/minsize/minsize.snap

@@ -10,15 +10,15 @@ Original   | Minified   | esbuild    | Gzip       | esbuild
 
 544.10 kB  | 73.49 kB   | 72.48 kB   | 26.13 kB   | 26.20 kB   | lodash.js 
 
-555.77 kB  | 276.27 kB  | 270.13 kB  | 91.09 kB   | 90.80 kB   | d3.js     
+555.77 kB  | 276.49 kB  | 270.13 kB  | 91.15 kB   | 90.80 kB   | d3.js     
 
 1.01 MB    | 467.63 kB  | 458.89 kB  | 126.75 kB  | 126.71 kB  | bundle.min.js
 
-1.25 MB    | 662.73 kB  | 646.76 kB  | 164.00 kB  | 163.73 kB  | three.js  
+1.25 MB    | 662.86 kB  | 646.76 kB  | 164.00 kB  | 163.73 kB  | three.js  
 
-2.14 MB    | 741.37 kB  | 724.14 kB  | 181.41 kB  | 181.07 kB  | victory.js
+2.14 MB    | 741.57 kB  | 724.14 kB  | 181.47 kB  | 181.07 kB  | victory.js
 
-3.20 MB    | 1.02 MB    | 1.01 MB    | 331.98 kB  | 331.56 kB  | echarts.js
+3.20 MB    | 1.02 MB    | 1.01 MB    | 332.01 kB  | 331.56 kB  | echarts.js
 
 6.69 MB    | 2.39 MB    | 2.31 MB    | 496.10 kB  | 488.28 kB  | antd.js