From 76112768c5d5577c1a4b613eeb7b2a545b656f18 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Tue, 22 Mar 2022 19:03:31 +0000 Subject: [PATCH] Preserve ints when executing int operations (#1964) This Pull Request fixes/closes #1962. It changes the following: - When executing arithmetic operations on `JsValue`s, try to use integer operations and fallback to `f64` operations on error. - Adds tests for serde_json conversions from integer operations. --- boa_engine/src/value/operations.rs | 26 ++++++++--- boa_engine/src/value/serde_json.rs | 70 +++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/boa_engine/src/value/operations.rs b/boa_engine/src/value/operations.rs index 444f856d4fc..4e45f6790e8 100644 --- a/boa_engine/src/value/operations.rs +++ b/boa_engine/src/value/operations.rs @@ -9,15 +9,19 @@ impl JsValue { pub fn add(&self, other: &Self, context: &mut Context) -> JsResult { Ok(match (self, other) { // Fast path: - (Self::Integer(x), Self::Integer(y)) => Self::new(f64::from(*x) + f64::from(*y)), + // Numeric add + (Self::Integer(x), Self::Integer(y)) => x + .checked_add(*y) + .map_or_else(|| Self::new(f64::from(*x) + f64::from(*y)), Self::new), (Self::Rational(x), Self::Rational(y)) => Self::new(x + y), (Self::Integer(x), Self::Rational(y)) => Self::new(f64::from(*x) + y), (Self::Rational(x), Self::Integer(y)) => Self::new(x + f64::from(*y)), + (Self::BigInt(ref x), Self::BigInt(ref y)) => Self::new(JsBigInt::add(x, y)), + // String concat (Self::String(ref x), Self::String(ref y)) => Self::from(JsString::concat(x, y)), (Self::String(ref x), y) => Self::from(JsString::concat(x, y.to_string(context)?)), (x, Self::String(ref y)) => Self::from(JsString::concat(x.to_string(context)?, y)), - (Self::BigInt(ref x), Self::BigInt(ref y)) => Self::new(JsBigInt::add(x, y)), // Slow path: (_, _) => match ( @@ -49,7 +53,9 @@ impl JsValue { pub fn sub(&self, other: &Self, context: &mut Context) -> JsResult { Ok(match (self, other) { // Fast path: - (Self::Integer(x), Self::Integer(y)) => Self::new(f64::from(*x) - f64::from(*y)), + (Self::Integer(x), Self::Integer(y)) => x + .checked_sub(*y) + .map_or_else(|| Self::new(f64::from(*x) - f64::from(*y)), Self::new), (Self::Rational(x), Self::Rational(y)) => Self::new(x - y), (Self::Integer(x), Self::Rational(y)) => Self::new(f64::from(*x) - y), (Self::Rational(x), Self::Integer(y)) => Self::new(x - f64::from(*y)), @@ -73,7 +79,9 @@ impl JsValue { pub fn mul(&self, other: &Self, context: &mut Context) -> JsResult { Ok(match (self, other) { // Fast path: - (Self::Integer(x), Self::Integer(y)) => Self::new(f64::from(*x) * f64::from(*y)), + (Self::Integer(x), Self::Integer(y)) => x + .checked_mul(*y) + .map_or_else(|| Self::new(f64::from(*x) * f64::from(*y)), Self::new), (Self::Rational(x), Self::Rational(y)) => Self::new(x * y), (Self::Integer(x), Self::Rational(y)) => Self::new(f64::from(*x) * y), (Self::Rational(x), Self::Integer(y)) => Self::new(x * f64::from(*y)), @@ -97,7 +105,10 @@ impl JsValue { pub fn div(&self, other: &Self, context: &mut Context) -> JsResult { Ok(match (self, other) { // Fast path: - (Self::Integer(x), Self::Integer(y)) => Self::new(f64::from(*x) / f64::from(*y)), + (Self::Integer(x), Self::Integer(y)) => x + .checked_div(*y) + .filter(|div| *y * div == *x) + .map_or_else(|| Self::new(f64::from(*x) / f64::from(*y)), Self::new), (Self::Rational(x), Self::Rational(y)) => Self::new(x / y), (Self::Integer(x), Self::Rational(y)) => Self::new(f64::from(*x) / y), (Self::Rational(x), Self::Integer(y)) => Self::new(x / f64::from(*y)), @@ -178,7 +189,10 @@ impl JsValue { pub fn pow(&self, other: &Self, context: &mut Context) -> JsResult { Ok(match (self, other) { // Fast path: - (Self::Integer(x), Self::Integer(y)) => Self::new(f64::from(*x).powi(*y)), + (Self::Integer(x), Self::Integer(y)) => u32::try_from(*y) + .ok() + .and_then(|y| x.checked_pow(y)) + .map_or_else(|| Self::new(f64::from(*x).powi(*y)), Self::new), (Self::Rational(x), Self::Rational(y)) => Self::new(x.powf(*y)), (Self::Integer(x), Self::Rational(y)) => Self::new(f64::from(*x).powf(*y)), (Self::Rational(x), Self::Integer(y)) => Self::new(x.powi(*y)), diff --git a/boa_engine/src/value/serde_json.rs b/boa_engine/src/value/serde_json.rs index 9f8d6a75410..efa4422c963 100644 --- a/boa_engine/src/value/serde_json.rs +++ b/boa_engine/src/value/serde_json.rs @@ -157,11 +157,10 @@ impl JsValue { #[cfg(test)] mod tests { use crate::object::JsArray; + use crate::{Context, JsValue}; #[test] fn ut_json_conversions() { - use crate::{Context, JsValue}; - let data = r#" { "name": "John Doe", @@ -208,4 +207,71 @@ mod tests { assert_eq!(json, value.to_json(&mut context).unwrap()); } + + #[test] + fn integer_ops_to_json() { + let mut context = Context::default(); + + let add = context + .eval( + r#" + 1000000 + 500 + "#, + ) + .unwrap(); + let add: u32 = serde_json::from_value(add.to_json(&mut context).unwrap()).unwrap(); + assert_eq!(add, 1_000_500); + + let sub = context + .eval( + r#" + 1000000 - 500 + "#, + ) + .unwrap(); + let sub: u32 = serde_json::from_value(sub.to_json(&mut context).unwrap()).unwrap(); + assert_eq!(sub, 999_500); + + let mult = context + .eval( + r#" + 1000000 * 500 + "#, + ) + .unwrap(); + let mult: u32 = serde_json::from_value(mult.to_json(&mut context).unwrap()).unwrap(); + assert_eq!(mult, 500_000_000); + + let div = context + .eval( + r#" + 1000000 / 500 + "#, + ) + .unwrap(); + let div: u32 = serde_json::from_value(div.to_json(&mut context).unwrap()).unwrap(); + assert_eq!(div, 2000); + + let rem = context + .eval( + r#" + 233894 % 500 + "#, + ) + .unwrap(); + let rem: u32 = serde_json::from_value(rem.to_json(&mut context).unwrap()).unwrap(); + assert_eq!(rem, 394); + + let pow = context + .eval( + r#" + 36 ** 5 + "#, + ) + .unwrap(); + + let pow: u32 = serde_json::from_value(pow.to_json(&mut context).unwrap()).unwrap(); + + assert_eq!(pow, 60466176); + } }