From 02b292db629ba101383cd9cde574cedc0ad0e48c Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Tue, 9 Jul 2024 19:06:03 +0200 Subject: [PATCH 1/2] Implement lossless TryFromJs for integers from f64 --- .../src/value/conversions/try_from_js.rs | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index 955b57f7239..89cffecd85c 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -156,6 +156,47 @@ impl TryFromJs for f64 { } } +trait ToF64: Sized { + fn cast_to_f64(self) -> f64; + fn cast_from_f64(v: f64) -> Self; +} + +macro_rules! impl_to_f64 { + ($type_:ident) => { + impl ToF64 for $type_ { + #[inline] + // NOTE: Lint only applies to types that are lossless conversion to f64. + #[allow(clippy::cast_lossless)] + fn cast_to_f64(self) -> f64 { + self as f64 + } + #[inline] + fn cast_from_f64(v: f64) -> Self { + v as Self + } + } + }; +} + +impl_to_f64!(i8); +impl_to_f64!(u8); +impl_to_f64!(i16); +impl_to_f64!(u16); +impl_to_f64!(i32); +impl_to_f64!(u32); +impl_to_f64!(i64); +impl_to_f64!(u64); +impl_to_f64!(usize); +impl_to_f64!(i128); +impl_to_f64!(u128); + +fn from_f64<T: ToF64>(v: f64) -> Option<T> { + if T::cast_from_f64(v).cast_to_f64().to_bits() == v.to_bits() { + return Some(T::cast_from_f64(v)); + } + None +} + impl TryFromJs for i8 { fn try_from_js(value: &JsValue, _context: &mut Context) -> JsResult<Self> { match value { @@ -164,6 +205,11 @@ impl TryFromJs for i8 { .with_message(format!("cannot convert value to a i8: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a i8") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a i8") .into()), @@ -179,6 +225,11 @@ impl TryFromJs for u8 { .with_message(format!("cannot convert value to a u8: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a u8") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a u8") .into()), @@ -194,6 +245,11 @@ impl TryFromJs for i16 { .with_message(format!("cannot convert value to a i16: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a i16") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a i16") .into()), @@ -209,6 +265,11 @@ impl TryFromJs for u16 { .with_message(format!("cannot convert value to a iu16: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a u16") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a u16") .into()), @@ -220,6 +281,11 @@ impl TryFromJs for i32 { fn try_from_js(value: &JsValue, _context: &mut Context) -> JsResult<Self> { match value { JsValue::Integer(i) => Ok(*i), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a i32") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a i32") .into()), @@ -235,6 +301,11 @@ impl TryFromJs for u32 { .with_message(format!("cannot convert value to a u32: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a u32") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a u32") .into()), @@ -246,6 +317,11 @@ impl TryFromJs for i64 { fn try_from_js(value: &JsValue, _context: &mut Context) -> JsResult<Self> { match value { JsValue::Integer(i) => Ok((*i).into()), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a i64") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a i64") .into()), @@ -261,6 +337,11 @@ impl TryFromJs for u64 { .with_message(format!("cannot convert value to a u64: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a u64") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a u64") .into()), @@ -276,6 +357,11 @@ impl TryFromJs for usize { .with_message(format!("cannot convert value to a usize: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a usize") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a usize") .into()), @@ -287,6 +373,11 @@ impl TryFromJs for i128 { fn try_from_js(value: &JsValue, _context: &mut Context) -> JsResult<Self> { match value { JsValue::Integer(i) => Ok((*i).into()), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a i128") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a i128") .into()), @@ -302,6 +393,11 @@ impl TryFromJs for u128 { .with_message(format!("cannot convert value to a u128: {e}")) .into() }), + JsValue::Rational(f) => from_f64(*f).ok_or_else(|| { + JsNativeError::typ() + .with_message("cannot convert value to a u128") + .into() + }), _ => Err(JsNativeError::typ() .with_message("cannot convert value to a u128") .into()), @@ -309,6 +405,36 @@ impl TryFromJs for u128 { } } +#[test] +fn integer_floating_js_value_to_integer() { + let context = &mut Context::default(); + + assert_eq!(i8::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(u8::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(i16::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(u16::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(i32::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(u32::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(i64::try_from_js(&JsValue::from(4.0), context), Ok(4)); + assert_eq!(u64::try_from_js(&JsValue::from(4.0), context), Ok(4)); + + // Floating with fractional part + let result = i32::try_from_js(&JsValue::from(4.000_000_000_000_001), context); + assert!(result.is_err()); + + // NaN + let result = i32::try_from_js(&JsValue::nan(), context); + assert!(result.is_err()); + + // +Infinity + let result = i32::try_from_js(&JsValue::positive_infinity(), context); + assert!(result.is_err()); + + // -Infinity + let result = i32::try_from_js(&JsValue::negative_infinity(), context); + assert!(result.is_err()); +} + #[test] fn value_into_vec() { use boa_engine::{run_test_actions, TestAction}; From 23d50565bde6ce5075e3b10e99b51c08fca7bff1 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:02:50 +0200 Subject: [PATCH 2/2] Use `num_traits::AsPrimitive` --- .../src/value/conversions/try_from_js.rs | 45 ++++--------------- 1 file changed, 8 insertions(+), 37 deletions(-) diff --git a/core/engine/src/value/conversions/try_from_js.rs b/core/engine/src/value/conversions/try_from_js.rs index 89cffecd85c..b41645d0f36 100644 --- a/core/engine/src/value/conversions/try_from_js.rs +++ b/core/engine/src/value/conversions/try_from_js.rs @@ -1,6 +1,7 @@ //! This module contains the [`TryFromJs`] trait, and conversions to basic Rust types. use num_bigint::BigInt; +use num_traits::AsPrimitive; use crate::{js_string, Context, JsBigInt, JsNativeError, JsObject, JsResult, JsString, JsValue}; @@ -156,43 +157,13 @@ impl TryFromJs for f64 { } } -trait ToF64: Sized { - fn cast_to_f64(self) -> f64; - fn cast_from_f64(v: f64) -> Self; -} - -macro_rules! impl_to_f64 { - ($type_:ident) => { - impl ToF64 for $type_ { - #[inline] - // NOTE: Lint only applies to types that are lossless conversion to f64. - #[allow(clippy::cast_lossless)] - fn cast_to_f64(self) -> f64 { - self as f64 - } - #[inline] - fn cast_from_f64(v: f64) -> Self { - v as Self - } - } - }; -} - -impl_to_f64!(i8); -impl_to_f64!(u8); -impl_to_f64!(i16); -impl_to_f64!(u16); -impl_to_f64!(i32); -impl_to_f64!(u32); -impl_to_f64!(i64); -impl_to_f64!(u64); -impl_to_f64!(usize); -impl_to_f64!(i128); -impl_to_f64!(u128); - -fn from_f64<T: ToF64>(v: f64) -> Option<T> { - if T::cast_from_f64(v).cast_to_f64().to_bits() == v.to_bits() { - return Some(T::cast_from_f64(v)); +fn from_f64<T>(v: f64) -> Option<T> +where + T: AsPrimitive<f64>, + f64: AsPrimitive<T>, +{ + if <f64 as AsPrimitive<T>>::as_(v).as_().to_bits() == v.to_bits() { + return Some(v.as_()); } None }