Skip to content

Commit

Permalink
Preserve ints when executing int operations (#1964)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jedel1043 @Razican
jedel1043 authored and Razican committed Jun 8, 2022
1 parent da9bca4 commit 7611276
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 8 deletions.
26 changes: 20 additions & 6 deletions boa_engine/src/value/operations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,19 @@ impl JsValue {
pub fn add(&self, other: &Self, context: &mut Context) -> JsResult<Self> {
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 (
Expand Down Expand Up @@ -49,7 +53,9 @@ impl JsValue {
pub fn sub(&self, other: &Self, context: &mut Context) -> JsResult<Self> {
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)),
Expand All @@ -73,7 +79,9 @@ impl JsValue {
pub fn mul(&self, other: &Self, context: &mut Context) -> JsResult<Self> {
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)),
Expand All @@ -97,7 +105,10 @@ impl JsValue {
pub fn div(&self, other: &Self, context: &mut Context) -> JsResult<Self> {
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)),
Expand Down Expand Up @@ -178,7 +189,10 @@ impl JsValue {
pub fn pow(&self, other: &Self, context: &mut Context) -> JsResult<Self> {
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)),
Expand Down
70 changes: 68 additions & 2 deletions boa_engine/src/value/serde_json.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
}
}

0 comments on commit 7611276

Please sign in to comment.