Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(vm): Fix the ranges for SmallInteger #58

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Conversation

andreubotella
Copy link
Collaborator

@andreubotella andreubotella commented Oct 31, 2023

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.

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`.
@andreubotella andreubotella changed the title fix(ecmascript): Fix the ranges for SmallInteger fix(vm): Fix the ranges for SmallInteger Oct 31, 2023
Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice and thanks <3

@aapoalas aapoalas merged commit c6fc727 into main Oct 31, 2023
1 check passed
@aapoalas aapoalas deleted the fix/small-integer-ranges branch October 31, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants