Skip to content

Commit

Permalink
Fix neg_from() in favor of neg_try_from() for Signed Integers (#273)
Browse files Browse the repository at this point in the history
## Type of change

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

- Bug fix

## Breaking

The `neg_from` implementation for all signed integers has been removed.
The `neg_try_from()` implementation has been added in its place.

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_negative_i8: I8 = I8::neg_from(1u8);
```

After:
```sway
let my_negative_i8: I8 = I8::neg_try_from(1u8).unwrap();
```

## Changes

The following changes have been made:

- Removes `neg_from()` in favor of `neg_try_from()` for `I8`
- Removes `neg_from()` in favor of `neg_try_from()` for `I16`
- Removes `neg_from()` in favor of `neg_try_from()` for `I32`
- Removes `neg_from()` in favor of `neg_try_from()` for `I64`
- Removes `neg_from()` in favor of `neg_try_from()` for `I128`
- Removes `neg_from()` in favor of `neg_try_from()` for `I256`

## Notes

- This is a breaking change
- Previously the `neg_from()` implementation would break on values
greater than the indent for each signed integer

## 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.
  • Loading branch information
bitzoic authored Aug 1, 2024
1 parent 7fdf8ec commit ddfe684
Show file tree
Hide file tree
Showing 19 changed files with 284 additions and 133 deletions.
25 changes: 22 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,54 @@ Description of the upcoming release here.
- [#263](https://github.com/FuelLabs/sway-libs/pull/263) Fixes `I128` and `I256`'s zero or "indent" value.
- [#268](https://github.com/FuelLabs/sway-libs/pull/268) Fixes subtraction involving negative numbers for `I8`, `I16`, `I32`, `I64`, `I128`, and `I256`.
- [#272](https://github.com/FuelLabs/sway-libs/pull/272) Fixes `From` implementations for Signed Integers with `TryFrom`.
- [#273](https://github.com/FuelLabs/sway-libs/pull/273) Fixes negative from implementations for Signed Integers.

#### Breaking

- [#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.
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();
```

- [#272](https://github.com/FuelLabs/sway-libs/pull/272) The `From` implementation for all signed integers to their respective unsigned integer has been removed. The `TryFrom` implementation has been added in its place.

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 = I8::from(1u8);
```

After:

```sway
let my_i8: I8 = I8::try_from(1u8).unwrap();
```

- [#273](https://github.com/FuelLabs/sway-libs/pull/273) The `neg_from` implementation for all signed integers has been removed. The `neg_try_from()` implementation has been added in its place.

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_negative_i8: I8 = I8::neg_from(1u8);
```

After:

```sway
let my_negative_i8: I8 = I8::neg_try_from(1u8).unwrap();
```
16 changes: 10 additions & 6 deletions libs/src/signed_integers/i128.sw
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl I128 {
///
/// # Returns
///
/// * [I128] - The newly created `I128` struct.
/// * [Option<I128>] - The newly created `I128` struct.
///
/// # Examples
///
Expand All @@ -168,13 +168,17 @@ impl I128 {
///
/// fn foo() {
/// let underlying = U128::from((1, 0));
/// let i128 = I128::neg_from(underlying);
/// let i128 = I128::neg_try_from(underlying).unwrap();
/// assert(i128.underlying() == U128::from((0, 0)));
/// }
/// ```
pub fn neg_from(value: U128) -> Self {
Self {
underlying: Self::indent() - value,
pub fn neg_try_from(value: U128) -> Option<Self> {
if value <= Self::indent() {
Some(Self {
underlying: Self::indent() - value,
})
} else {
None
}
}

Expand Down Expand Up @@ -394,7 +398,7 @@ impl WrappingNeg for I128 {
if self == self::min() {
return self::min()
}
self * Self::neg_from(U128::from((0, 1)))
self * Self::neg_try_from(U128::from((0, 1))).unwrap()
}
}

Expand Down
16 changes: 10 additions & 6 deletions libs/src/signed_integers/i16.sw
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl I16 {
///
/// # Returns
///
/// * [I16] - The newly created `I16` struct.
/// * [Option<I16>] - The newly created `I16` struct.
///
/// # Examples
///
Expand All @@ -163,13 +163,17 @@ impl I16 {
///
/// fn foo() {
/// let underlying = 1u16;
/// let i16 = I16::neg_from(underlying);
/// let i16 = I16::neg_try_from(underlying).unwrap();
/// assert(i16.underlying() == 32767u16)
/// }
/// ```
pub fn neg_from(value: u16) -> Self {
Self {
underlying: Self::indent() - value,
pub fn neg_try_from(value: u16) -> Option<Self> {
if value <= Self::indent() {
Some(Self {
underlying: Self::indent() - value,
})
} else {
None
}
}

Expand Down Expand Up @@ -382,7 +386,7 @@ impl WrappingNeg for I16 {
if self == self::min() {
return self::min()
}
self * Self::neg_from(1u16)
self * Self::neg_try_from(1u16).unwrap()
}
}

Expand Down
16 changes: 10 additions & 6 deletions libs/src/signed_integers/i256.sw
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl I256 {
///
/// # Returns
///
/// * [I256] - The newly created `I256` struct.
/// * [Option<I256>] - The newly created `I256` struct.
///
/// # Examples
///
Expand All @@ -166,13 +166,17 @@ impl I256 {
///
/// fn foo() {
/// let underlying = 0x0000000000000000000000000000000000000000000000000000000000000000u256;
/// let i256 = I256::neg_from(underlying);
/// let i256 = I256::neg_try_from(underlying).unwrap();
/// assert(i256.underlying() == 0x8000000000000000000000000000000000000000000000000000000000000000u256);
/// }
/// ```
pub fn neg_from(value: u256) -> Self {
Self {
underlying: Self::indent() - value,
pub fn neg_try_from(value: u256) -> Option<Self> {
if value <= Self::indent() {
Some(Self {
underlying: Self::indent() - value,
})
} else {
None
}
}

Expand Down Expand Up @@ -373,7 +377,7 @@ impl WrappingNeg for I256 {
if self == self::min() {
return self::min()
}
self * Self::neg_from(0x0000000000000000000000000000000000000000000000000000000000000001u256)
self * Self::neg_try_from(0x0000000000000000000000000000000000000000000000000000000000000001u256).unwrap()
}
}

Expand Down
16 changes: 10 additions & 6 deletions libs/src/signed_integers/i32.sw
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl I32 {
///
/// # Returns
///
/// * [I32] - The newly created `I32` struct.
/// * [Option<I32>] - The newly created `I32` struct.
///
/// # Examples
///
Expand All @@ -163,13 +163,17 @@ impl I32 {
///
/// fn foo() {
/// let underlying = 1u32;
/// let i32 = I32::neg_from(underlying);
/// let i32 = I32::neg_try_from(underlying).unwrap();
/// assert(i32.underlying() == 2147483647u32)
/// }
/// ```
pub fn neg_from(value: u32) -> Self {
Self {
underlying: Self::indent() - value,
pub fn neg_try_from(value: u32) -> Option<Self> {
if value <= Self::indent() {
Some(Self {
underlying: Self::indent() - value,
})
} else {
None
}
}

Expand Down Expand Up @@ -382,7 +386,7 @@ impl WrappingNeg for I32 {
if self == self::min() {
return self::min()
}
self * Self::neg_from(1u32)
self * Self::neg_try_from(1u32).unwrap()
}
}

Expand Down
16 changes: 10 additions & 6 deletions libs/src/signed_integers/i64.sw
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl I64 {
///
/// # Returns
///
/// * [I64] - The newly created `I64` struct.
/// * [Option<I64>] - The newly created `I64` struct.
///
/// # Examples
///
Expand All @@ -163,13 +163,17 @@ impl I64 {
///
/// fn foo() {
/// let underlying = 1u64;
/// let i64 = I64::neg_from(underlying);
/// let i64 = I64::neg_try_from(underlying).unwrap();
/// assert(i64.underlying() == 9223372036854775807u64);
/// }
/// ```
pub fn neg_from(value: u64) -> Self {
Self {
underlying: Self::indent() - value,
pub fn neg_try_from(value: u64) -> Option<Self> {
if value <= Self::indent() {
Some(Self {
underlying: Self::indent() - value,
})
} else {
None
}
}

Expand Down Expand Up @@ -383,7 +387,7 @@ impl WrappingNeg for I64 {
if self == self::min() {
return self::min()
}
self * Self::neg_from(1)
self * Self::neg_try_from(1).unwrap()
}
}

Expand Down
16 changes: 10 additions & 6 deletions libs/src/signed_integers/i8.sw
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl I8 {
///
/// # Returns
///
/// * [I8] - The newly created `I8` struct.
/// * [Option<I8>] - The newly created `I8` struct.
///
/// # Examples
///
Expand All @@ -163,13 +163,17 @@ impl I8 {
///
/// fn foo() {
/// let underlying = 1u8;
/// let i8 = I8::neg_from(underlying);
/// let i8 = I8::neg_try_from(underlying);
/// assert(i8.underlying() == 127u8);
/// }
/// ```
pub fn neg_from(value: u8) -> Self {
Self {
underlying: Self::indent() - value,
pub fn neg_try_from(value: u8) -> Option<Self> {
if value <= Self::indent() {
Some(Self {
underlying: Self::indent() - value,
})
} else {
None
}
}

Expand Down Expand Up @@ -382,7 +386,7 @@ impl WrappingNeg for I8 {
if self == self::min() {
return self::min()
}
self * Self::neg_from(1u8)
self * Self::neg_try_from(1u8).unwrap()
}
}

Expand Down
40 changes: 29 additions & 11 deletions tests/src/signed_integers/signed_i128/src/main.sw
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ fn main() -> bool {
res = I128::try_from(u128_10).unwrap() - I128::try_from(u128_11).unwrap();
assert(res.underlying().lower() == u64::max());

res = I128::try_from(u128_10).unwrap() * I128::neg_from(u128_one);
assert(res == I128::neg_from(u128_10));
res = I128::try_from(u128_10).unwrap() * I128::neg_try_from(u128_one).unwrap();
assert(res == I128::neg_try_from(u128_10).unwrap());

res = I128::try_from(u128_10).unwrap() * I128::try_from(u128_10).unwrap();
let u128_100 = U128::from((0, 100));
Expand All @@ -26,7 +26,7 @@ fn main() -> bool {
let u128_lower_max_u64 = U128::from((0, u64::max()));

res = I128::try_from(u128_10).unwrap() / I128::try_from(u128_lower_max_u64).unwrap();
assert(res == I128::neg_from(u128_10));
assert(res == I128::neg_try_from(u128_10).unwrap());

let u128_5 = U128::from((0, 5));

Expand All @@ -38,13 +38,13 @@ fn main() -> bool {
// Subtraction tests
let pos1 = I128::try_from(U128::from((0, 1))).unwrap();
let pos2 = I128::try_from(U128::from((0, 2))).unwrap();
let neg1 = I128::neg_from(U128::from((0, 1)));
let neg2 = I128::neg_from(U128::from((0, 2)));
let neg1 = I128::neg_try_from(U128::from((0, 1))).unwrap();
let neg2 = I128::neg_try_from(U128::from((0, 2))).unwrap();

// Both positive:
let res1 = pos1 - pos2;
let res1_2 = pos2 - pos1;
assert(res1 == I128::neg_from(U128::from((0, 1))));
assert(res1 == I128::neg_try_from(U128::from((0, 1))).unwrap());

let res2 = pos2 - pos1;
assert(res2 == I128::try_from(U128::from((0, 1))).unwrap());
Expand All @@ -55,20 +55,20 @@ fn main() -> bool {

// Second positive
let res4 = neg1 - pos1;
assert(res4 == I128::neg_from(U128::from((0, 2))));
assert(res4 == I128::neg_try_from(U128::from((0, 2))).unwrap());

// Both negative
let res5 = neg1 - neg2;
assert(res5 == I128::try_from(U128::from((0, 1))).unwrap());

let res6 = neg2 - neg1;
assert(res6 == I128::neg_from(U128::from((0, 1))));
assert(res6 == I128::neg_try_from(U128::from((0, 1))).unwrap());

// OrqEq Tests
let one_1 = I128::try_from(U128::from((0, 1))).unwrap();
let one_2 = I128::try_from(U128::from((0, 1))).unwrap();
let neg_one_1 = I128::neg_from(U128::from((0, 1)));
let neg_one_2 = I128::neg_from(U128::from((0, 1)));
let neg_one_1 = I128::neg_try_from(U128::from((0, 1))).unwrap();
let neg_one_2 = I128::neg_try_from(U128::from((0, 1))).unwrap();
let max_1 = I128::max();
let max_2 = I128::max();
let min_1 = I128::min();
Expand Down Expand Up @@ -97,6 +97,24 @@ fn main() -> bool {
assert(one_1 >= min_1);
assert(neg_one_1 >= min_1);

// Test neg try from
let indent = I128::indent();

let neg_try_from_zero = I128::neg_try_from(U128::min());
assert(neg_try_from_zero.is_some());
assert(neg_try_from_zero.unwrap() == I128::zero());

let neg_try_from_one = I128::neg_try_from(U128::from((0, 1)));
assert(neg_try_from_one.is_some());
assert(neg_try_from_one.unwrap().underlying() == I128::indent() - U128::from((0, 1)));

let neg_try_from_max = I128::neg_try_from(indent);
assert(neg_try_from_max.is_some());
assert(neg_try_from_max.unwrap().underlying() == U128::min());

let neg_try_from_overflow = I128::neg_try_from(indent + U128::from((0, 1)));
assert(neg_try_from_overflow.is_none());

// Test into I128
let indent: U128 = I128::indent();

Expand Down Expand Up @@ -124,7 +142,7 @@ fn main() -> bool {

// Test into U128
let zero = I128::zero();
let negative = I128::neg_from(U128::from((0, 1)));
let negative = I128::neg_try_from(U128::from((0, 1))).unwrap();
let max = I128::max();

let U128_max_try_from: Option<U128> = U128::try_from(max);
Expand Down
Loading

0 comments on commit ddfe684

Please sign in to comment.