Skip to content

Commit

Permalink
Signed Integers TwosComplement fix to WrappingNeg (#263)
Browse files Browse the repository at this point in the history
## Type of change

<!--Delete points that do not apply-->

- Bug fix

## Changes

The following changes have been made:

- The `TwosComplement` trait was not correctly implemented. To stay in
line with Rust's practices, the `WrappingNeg` trait now replaces it with
the original desired functionality of the `TwosComplement` trait.
- Bits for `I128` and `I256` are now correct
- The indent for `I128` and `I256` are now correct

## Notes

- This PR changes the folder names in the tests from
`iX_twos_complement_test` to `iX_wrapping_neg_test`.

## Breaking Changes

The following demonstrates the breaking change. While this example code
uses the `I8` type, the same logic may be applied to the `I16`, `I32`,
`I64`, `I128`, and `I256` types.

Before:
```sway
let my_i8 = i8::zero();
let twos_complement = my_i8.twos_complement();
```

After:
```sway
let my_i8 = i8::zero();
let wrapping_neg = my_i8.wrapping_neg();
```

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
- [x] I have updated the changelog to reflect the changes on this PR.

---------

Co-authored-by: K1-R1 <[email protected]>
  • Loading branch information
bitzoic and K1-R1 authored Jul 25, 2024
1 parent 76cf2cc commit 624b664
Show file tree
Hide file tree
Showing 55 changed files with 693 additions and 518 deletions.
19 changes: 17 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,23 @@ Description of the upcoming release here.

- [#258](https://github.com/FuelLabs/sway-libs/pull/258) Fixes incorrect instructions on how to run tests in README and docs hub.
- [#262](https://github.com/FuelLabs/sway-libs/pull/262) Fixes incorrect ordering comparison for IFP64, IFP128 and IFP256.
- [#263](https://github.com/FuelLabs/sway-libs/pull/263) Fixes `I256`'s returned bits.
- [#263](https://github.com/FuelLabs/sway-libs/pull/263) Fixes `I128` and `I256`'s zero or "indent" value.

#### Breaking

- Some breaking change here 1
- Some breaking change here 2
- [#263](https://github.com/FuelLabs/sway-libs/pull/263) Removes the `TwosComplement` trait in favor of `WrappingNeg`.

The following demonstrates the breaking change. While this example code uses the `I8` type, the same logic may be applied to the `I16`, `I32`, `I64`, `I128`, and `I256` types.

Before:
```sway
let my_i8 = i8::zero();
let twos_complement = my_i8.twos_complement();
```

After:
```sway
let my_i8 = i8::zero();
let wrapping_neg = my_i8.wrapping_neg();
```
12 changes: 8 additions & 4 deletions libs/src/signed_integers/common.sw
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
library;

/// Trait for the Two's Complement of a value.
pub trait TwosComplement {
/// Returns the two's complement of a value.
/// Wrapping (modular) negation. Computes -self, wrapping around at the boundary of the type.
pub trait WrappingNeg {
/// Negates a signed number.
///
/// # Additional Information
///
/// * The only case where such wrapping can occur is when one negates self::min(). In such a case, this function returns self::min() itself.
///
/// # Returns
///
/// * [Self] - The value as two's complement.
fn twos_complement(self) -> Self;
fn wrapping_neg(self) -> Self;
}
36 changes: 14 additions & 22 deletions libs/src/signed_integers/i128.sw
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
library;

use std::u128::U128;
use ::signed_integers::common::TwosComplement;
use ::signed_integers::common::WrappingNeg;
use ::signed_integers::errors::Error;

/// The 128-bit signed integer type.
Expand Down Expand Up @@ -31,11 +31,11 @@ impl I128 {
///
/// fn foo() {
/// let zero = I128::indent();
/// assert(zero == U128::from((1, 0)));
/// assert(zero == (U128::max() / (U128::from(0, 2)) - U128::from(0,1));
/// }
/// ```
pub fn indent() -> U128 {
U128::from((1, 0))
U128::from((9223372036854775808, 0))
}
}

Expand Down Expand Up @@ -189,7 +189,7 @@ impl I128 {
///
/// # Additional Information
///
/// The zero value of I128 is U128::from((1, 0)).
/// The zero value of I128 is U128::from((9223372036854775808, 0)).
///
/// # Returns
///
Expand Down Expand Up @@ -269,7 +269,7 @@ impl I128 {
///
/// fn foo() {
/// let i128 = I128::zero();
/// assert(i128.underlying() == U128::from((1, 0)));
/// assert(i128.underlying() == U128::from((9223372036854775808, 0)));
/// }
/// ```
pub fn underlying(self) -> U128 {
Expand Down Expand Up @@ -342,10 +342,8 @@ impl core::ops::Multiply for I128 {
/// Multiply a I128 with a I128. Panics of overflow.
fn multiply(self, other: Self) -> Self {
let mut res = Self::new();
if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
&& (other.underlying > Self::indent()
|| other.underlying == Self::indent())
if self.underlying >= Self::indent()
&& other.underlying >= Self::indent()
{
res = Self::from_uint(
(self.underlying - Self::indent()) * (other.underlying - Self::indent()) + Self::indent(),
Expand All @@ -356,16 +354,14 @@ impl core::ops::Multiply for I128 {
res = Self::from_uint(
(Self::indent() - self.underlying) * (Self::indent() - other.underlying) + Self::indent(),
);
} else if (self.underlying > Self::indent()
|| self.underlying == Self::indent())
} else if self.underlying >= Self::indent()
&& other.underlying < Self::indent()
{
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())
&& other.underlying >= Self::indent()
{
res = Self::from_uint(
Self::indent() - (other.underlying - Self::indent()) * (Self::indent() - self.underlying),
Expand Down Expand Up @@ -412,15 +408,11 @@ impl core::ops::Subtract for I128 {
}
}

impl TwosComplement for I128 {
fn twos_complement(self) -> Self {
if self.underlying == Self::indent()
|| self.underlying > Self::indent()
{
return self;
impl WrappingNeg for I128 {
fn wrapping_neg(self) -> Self {
if self == self::min() {
return self::min()
}
let u_one = U128::from((0, 1));
let res = I128::from_uint(!self.underlying + u_one);
res
self * Self::neg_from(U128::from((0, 1)))
}
}
13 changes: 6 additions & 7 deletions libs/src/signed_integers/i16.sw
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
library;

use ::signed_integers::errors::Error;
use ::signed_integers::common::TwosComplement;
use ::signed_integers::common::WrappingNeg;

/// The 16-bit signed integer type.
///
Expand Down Expand Up @@ -391,12 +391,11 @@ impl core::ops::Subtract for I16 {
}
}

impl TwosComplement for I16 {
fn twos_complement(self) -> Self {
if self.underlying >= Self::indent() {
return self;
impl WrappingNeg for I16 {
fn wrapping_neg(self) -> Self {
if self == self::min() {
return self::min()
}
let res = Self::from_uint(!self.underlying + 1u16);
res
self * Self::neg_from(1u16)
}
}
Loading

0 comments on commit 624b664

Please sign in to comment.