Skip to content

Commit

Permalink
fix(ecmascript): Fix the ranges for SmallInteger
Browse files Browse the repository at this point in the history
Since `SmallInteger` represents numbers in two's complement, the
minimum value for that type is -2^55, rather than -2^55 + 1, even
though its maximum value is 2^55 - 1. This is because there are as
many negative numbers representable in the type as non-negative
numbers, but the non-negative numbers include zero. This patch updates
`SmallInteger::MIN_BIGINT` to reflect this.

This patch also updates `SmallInteger::{MIN,MAX}_NUMBER` to remove the
division by 2, to make them truly reflect Javascript's
`Number.{MIN,MAX}_SAFE_INTEGER`. It also changes the ranges in
`SmallInteger::from_i64_unchecked` and in the `TryFrom<i64>` impl to
use `{MIN,MAX}_BIGINT`, rather than `{MIN,MAX}_NUMBER`.
  • Loading branch information
andreubotella committed Oct 31, 2023
1 parent 4eeb0f1 commit 6c55f51
Showing 1 changed file with 24 additions and 12 deletions.
36 changes: 24 additions & 12 deletions nova_vm/src/engine/small_integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ impl std::fmt::Debug for SmallInteger {
}

impl SmallInteger {
pub const MIN_BIGINT: i64 = -(2i64.pow(56)) / 2 + 1;
pub const MAX_BIGINT: i64 = 2i64.pow(56) / 2 + 1;
pub const MIN_BIGINT: i64 = -2i64.pow(55);
pub const MAX_BIGINT: i64 = 2i64.pow(55) - 1;

pub const MIN_NUMBER: i64 = -(2i64).pow(53) / 2 + 1;
pub const MAX_NUMBER: i64 = (2i64).pow(53) / 2 - 1;
pub const MIN_NUMBER: i64 = -2i64.pow(53) + 1;
pub const MAX_NUMBER: i64 = 2i64.pow(53) - 1;

#[inline]
pub fn into_i64(self) -> i64 {
Expand All @@ -29,7 +29,7 @@ impl SmallInteger {
}

pub fn from_i64_unchecked(value: i64) -> SmallInteger {
debug_assert!((Self::MIN_NUMBER..=Self::MAX_NUMBER).contains(&value));
debug_assert!((Self::MIN_BIGINT..=Self::MAX_BIGINT).contains(&value));
let bytes = i64::to_ne_bytes(value);

let data = if cfg!(target_endian = "little") {
Expand All @@ -49,7 +49,7 @@ impl SmallInteger {
impl TryFrom<i64> for SmallInteger {
type Error = ();
fn try_from(value: i64) -> Result<Self, Self::Error> {
if (Self::MIN_NUMBER..=Self::MAX_NUMBER).contains(&value) {
if (Self::MIN_BIGINT..=Self::MAX_BIGINT).contains(&value) {
Ok(Self::from_i64_unchecked(value))
} else {
Err(())
Expand Down Expand Up @@ -83,17 +83,29 @@ fn valid_small_integers() {
assert_eq!(5i64, SmallInteger::try_from(5).unwrap().into());
assert_eq!(23i64, SmallInteger::try_from(23).unwrap().into());
assert_eq!(
SmallInteger::MAX_NUMBER,
SmallInteger::try_from(SmallInteger::MAX_NUMBER)
SmallInteger::MAX_NUMBER + 1,
SmallInteger::try_from(SmallInteger::MAX_NUMBER + 1)
.unwrap()
.into()
);
assert_eq!(
SmallInteger::MAX_BIGINT,
SmallInteger::try_from(SmallInteger::MAX_BIGINT)
.unwrap()
.into()
);

assert_eq!(-5i64, SmallInteger::try_from(-5).unwrap().into());
assert_eq!(-59i64, SmallInteger::try_from(-59).unwrap().into());
assert_eq!(
SmallInteger::MIN_NUMBER,
SmallInteger::try_from(SmallInteger::MIN_NUMBER)
SmallInteger::MIN_NUMBER - 1,
SmallInteger::try_from(SmallInteger::MIN_NUMBER - 1)
.unwrap()
.into()
);
assert_eq!(
SmallInteger::MIN_BIGINT,
SmallInteger::try_from(SmallInteger::MIN_BIGINT)
.unwrap()
.into()
);
Expand All @@ -102,12 +114,12 @@ fn valid_small_integers() {
#[test]
fn invalid_small_integers() {
assert_eq!(
SmallInteger::try_from(SmallInteger::MAX_NUMBER + 1),
SmallInteger::try_from(SmallInteger::MAX_BIGINT + 1),
Err(())
);
assert_eq!(SmallInteger::try_from(i64::MAX), Err(()));
assert_eq!(
SmallInteger::try_from(SmallInteger::MIN_NUMBER - 1),
SmallInteger::try_from(SmallInteger::MIN_BIGINT - 1),
Err(())
);
assert_eq!(SmallInteger::try_from(i64::MIN), Err(()));
Expand Down

0 comments on commit 6c55f51

Please sign in to comment.