Skip to content

Commit

Permalink
Fix signed integer bugs (#66)
Browse files Browse the repository at this point in the history 
Fixed subtraction logic, and fix after renaming `from` and `from_int`
  • Loading branch information
@rostyslavtyshko
rostyslavtyshko authored Feb 7, 2023
1 parent e238aa4 commit 55a1a3d
Show file tree
Hide file tree
Showing 7 changed files with 299 additions and 96 deletions.
67 changes: 52 additions & 15 deletions libs/signed_integers/src/i128.sw
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,22 @@ impl core::ops::Add for I128 {
/// Add a I128 to a I128. Panics on overflow.
fn add(self, other: Self) -> Self {
// subtract 1 << 63 to avoid double move
Self::from(self.underlying - Self::indent() + other.underlying)
let mut res = Self::new();
if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
{
res = Self::from_uint(self.underlying - Self::indent() + other.underlying) // subtract 1 << 31 to avoid double move
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
} else if self.underlying < Self::indent()
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from_uint(other.underlying - Self::indent() + self.underlying);
}
res
}
}

Expand All @@ -107,20 +122,20 @@ impl core::ops::Divide for I128 {
|| self.underlying == Self::indent())
&& divisor.underlying > Self::indent()
{
res = Self::from((self.underlying - Self::indent()) / (divisor.underlying - Self::indent()) + Self::indent());
res = Self::from_uint((self.underlying - Self::indent()) / (divisor.underlying - Self::indent()) + Self::indent());
} else if self.underlying < Self::indent()
&& divisor.underlying < Self::indent()
{
res = Self::from((Self::indent() - self.underlying) / (Self::indent() - divisor.underlying) + Self::indent());
res = Self::from_uint((Self::indent() - self.underlying) / (Self::indent() - divisor.underlying) + Self::indent());
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& divisor.underlying < Self::indent()
{
res = Self::from(Self::indent() - (self.underlying - Self::indent()) / (Self::indent() - divisor.underlying));
res = Self::from_uint(Self::indent() - (self.underlying - Self::indent()) / (Self::indent() - divisor.underlying));
} else if self.underlying < Self::indent()
&& divisor.underlying > Self::indent()
{
res = Self::from(Self::indent() - (Self::indent() - self.underlying) / (divisor.underlying - Self::indent()));
res = Self::from_uint(Self::indent() - (Self::indent() - self.underlying) / (divisor.underlying - Self::indent()));
}
res
}
Expand All @@ -135,21 +150,21 @@ impl core::ops::Multiply for I128 {
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from((self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent());
res = Self::from_uint((self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent());
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from((Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent());
res = Self::from_uint((Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent());
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& other.underlying < Self::indent()
{
res = Self::from(Self::indent() - (self.underlying - Self::indent()) * (Self::indent() - other.underlying));
res = Self::from_uint(Self::indent() - (self.underlying - Self::indent()) * (Self::indent() - other.underlying));
} else if self.underlying < Self::indent()
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from(Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying));
res = Self::from_uint(Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying));
}
res
}
Expand All @@ -159,12 +174,34 @@ impl core::ops::Subtract for I128 {
/// Subtract a I128 from a I128. Panics of overflow.
fn subtract(self, other: Self) -> Self {
let mut res = Self::new();
if self > other {
// add 1 << 63 to avoid loosing the move
res = Self::from(self.underlying - other.underlying + Self::indent());
} else {
// subtract from 1 << 63 as we are getting a negative value
res = Self::from(Self::indent() - (other.underlying - self.underlying));
if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
if self.underlying > other.underlying {
res = Self::from_uint(self.underlying - other.underlying + Self::indent());
} else {
res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
}
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& other.underlying < Self::indent()
{
res = Self::from_uint(self.underlying - Self::indent() + other.underlying);
} else if self.underlying < Self::indent()
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
if self.underlying < other.underlying {
res = Self::from_uint(other.underlying - self.underlying + Self::indent());
} else {
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
}
}
res
}
Expand Down
61 changes: 45 additions & 16 deletions libs/signed_integers/src/i16.sw
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,19 @@ impl I16 {
impl core::ops::Add for I16 {
/// Add a I16 to a I16. Panics on overflow.
fn add(self, other: Self) -> Self {
// subtract 1 << 15 to avoid double move
Self::from(self.underlying - Self::indent() + other.underlying)
let mut res = Self::new();
if self.underlying >= Self::indent() {
res = Self::from_uint(self.underlying - Self::indent() + other.underlying) // subtract 1 << 15 to avoid double move
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
} else if self.underlying < Self::indent()
&& other.underlying >= Self::indent()
{
res = Self::from_uint(other.underlying - Self::indent() + self.underlying);
}
res
}
}

Expand All @@ -102,19 +113,19 @@ impl core::ops::Divide for I16 {
if self.underlying >= Self::indent()
&& divisor.underlying > Self::indent()
{
res = Self::from((self.underlying - Self::indent()) / (divisor.underlying - Self::indent()) + Self::indent());
res = Self::from_uint((self.underlying - Self::indent()) / (divisor.underlying - Self::indent()) + Self::indent());
} else if self.underlying < Self::indent()
&& divisor.underlying < Self::indent()
{
res = Self::from((Self::indent() - self.underlying) / (Self::indent() - divisor.underlying) + Self::indent());
res = Self::from_uint((Self::indent() - self.underlying) / (Self::indent() - divisor.underlying) + Self::indent());
} else if self.underlying >= Self::indent()
&& divisor.underlying < Self::indent()
{
res = Self::from(Self::indent() - (self.underlying - Self::indent()) / (Self::indent() - divisor.underlying));
res = Self::from_uint(Self::indent() - (self.underlying - Self::indent()) / (Self::indent() - divisor.underlying));
} else if self.underlying < Self::indent()
&& divisor.underlying > Self::indent()
{
res = Self::from(Self::indent() - (Self::indent() - self.underlying) / (divisor.underlying - Self::indent()));
res = Self::from_uint(Self::indent() - (Self::indent() - self.underlying) / (divisor.underlying - Self::indent()));
}
res
}
Expand All @@ -127,19 +138,19 @@ impl core::ops::Multiply for I16 {
if self.underlying >= Self::indent()
&& other.underlying >= Self::indent()
{
res = Self::from((self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent());
res = Self::from_uint((self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent());
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from((Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent());
res = Self::from_uint((Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent());
} else if self.underlying >= Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from(Self::indent() - (self.underlying - Self::indent()) * (Self::indent() - other.underlying));
res = Self::from_uint(Self::indent() - (self.underlying - Self::indent()) * (Self::indent() - other.underlying));
} else if self.underlying < Self::indent()
&& other.underlying >= Self::indent()
{
res = Self::from(Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying));
res = Self::from_uint(Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying));
}
res
}
Expand All @@ -149,12 +160,30 @@ impl core::ops::Subtract for I16 {
/// Subtract a I16 from a I16. Panics of overflow.
fn subtract(self, other: Self) -> Self {
let mut res = Self::new();
if self > other {
// add 1 << 15 to avoid loosing the move
res = Self::from(self.underlying - other.underlying + Self::indent());
} else {
// subtract from 1 << 15 as we are getting a negative value
res = Self::from(Self::indent() - (other.underlying - self.underlying));
if self.underlying >= Self::indent()
&& other.underlying >= Self::indent()
{
if self.underlying > other.underlying {
res = Self::from_uint(self.underlying - other.underlying + Self::indent());
} else {
res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
}
} else if self.underlying >= Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from_uint(self.underlying - Self::indent() + other.underlying);
} else if self.underlying < Self::indent()
&& other.underlying >= Self::indent()
{
res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
if self.underlying < other.underlying {
res = Self::from_uint(other.underlying - self.underlying + Self::indent());
} else {
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
}
}
res
}
Expand Down
80 changes: 61 additions & 19 deletions libs/signed_integers/src/i256.sw
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,22 @@ impl core::ops::Add for I256 {
/// Add a I256 to a I256. Panics on overflow.
fn add(self, other: Self) -> Self {
// subtract 1 << 63 to avoid double move
Self::from(self.underlying - Self::indent() + other.underlying)
let mut res = Self::new();
if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
{
res = Self::from_uint(self.underlying - Self::indent() + other.underlying) // subtract 1 << 31 to avoid double move
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
} else if self.underlying < Self::indent()
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from_uint(other.underlying - Self::indent() + self.underlying);
}
res
}
}

Expand All @@ -104,24 +119,23 @@ impl core::ops::Divide for I256 {
fn divide(self, divisor: Self) -> Self {
require(divisor != Self::new(), Error::ZeroDivisor);
let mut res = Self::new();
if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& divisor.underlying > Self::indent()
{
res = Self::from((self.underlying - Self::indent()) / (divisor.underlying - Self::indent()) + Self::indent());
let self_ge_indent = self.underlying > Self::indent() || self.underlying == Self::indent();
let divisor_gt_indent = divisor.underlying > Self::indent();
if self_ge_indent && divisor_gt_indent {
res = Self::from_uint((self.underlying - Self::indent()) / (divisor.underlying - Self::indent()) + Self::indent());
} else if self.underlying < Self::indent()
&& divisor.underlying < Self::indent()
{
res = Self::from((Self::indent() - self.underlying) / (Self::indent() - divisor.underlying) + Self::indent());
res = Self::from_uint((Self::indent() - self.underlying) / (Self::indent() - divisor.underlying) + Self::indent());
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& divisor.underlying < Self::indent()
{
res = Self::from(Self::indent() - (self.underlying - Self::indent()) / (Self::indent() - divisor.underlying));
res = Self::from_uint(Self::indent() - (self.underlying - Self::indent()) / (Self::indent() - divisor.underlying));
} else if self.underlying < Self::indent()
&& divisor.underlying > Self::indent()
{
res = Self::from(Self::indent() - (Self::indent() - self.underlying) / (divisor.underlying - Self::indent()));
res = Self::from_uint(Self::indent() - (Self::indent() - self.underlying) / (divisor.underlying - Self::indent()));
}
res
}
Expand All @@ -136,21 +150,21 @@ impl core::ops::Multiply for I256 {
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from((self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent());
res = Self::from_uint((self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent());
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
res = Self::from((Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent());
res = Self::from_uint((Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent());
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& other.underlying < Self::indent()
{
res = Self::from(Self::indent() - (self.underlying - Self::indent()) * (Self::indent() - other.underlying));
res = Self::from_uint(Self::indent() - (self.underlying - Self::indent()) * (Self::indent() - other.underlying));
} else if self.underlying < Self::indent()
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from(Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying));
res = Self::from_uint(Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying));
}
res
}
Expand All @@ -160,12 +174,40 @@ impl core::ops::Subtract for I256 {
/// Subtract a I256 from a I256. Panics of overflow.
fn subtract(self, other: Self) -> Self {
let mut res = Self::new();
if self > other {
// add 1 << 63 to avoid loosing the move
res = Self::from(self.underlying - other.underlying + Self::indent());
} else {
// subtract from 1 << 63 as we are getting a negative value
res = Self::from(Self::indent() - (other.underlying - self.underlying));
if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
if self.underlying > other.underlying {
res = Self::from_uint(self.underlying - other.underlying + Self::indent());
} else {
let q = other.underlying - Self::indent();

// std::logging::log(self.underlying.a);
// std::logging::log(self.underlying.b);
// std::logging::log(self.underlying.c);
// std::logging::log(self.underlying.d);
res = Self::from_uint(self.underlying - q);
}
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& other.underlying < Self::indent()
{
res = Self::from_uint(self.underlying - Self::indent() + other.underlying);
} else if self.underlying < Self::indent()
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
{
res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
} else if self.underlying < Self::indent()
&& other.underlying < Self::indent()
{
if self.underlying < other.underlying {
res = Self::from_uint(other.underlying - self.underlying + Self::indent());
} else {
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
}
}
res
}
Expand Down
Loading

0 comments on commit 55a1a3d

Please sign in to comment.