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

Add widening_mul, group absolute value operations, reformat. #1

Merged
merged 6 commits into from
Oct 22, 2024

Conversation

lleoha
Copy link

@lleoha lleoha commented Oct 17, 2024

@erik-3milabs
I also did some changes, please review and incorporate in yours if you will.
What I did is:

  • grouped absolute value operations in seperate file, renamed them so they resamble what's in the num_traits::signed,
  • changed the order so absolute value is before sign (so it's consistent with LE where we use for any other operations (e.g. split)
  • added widening_mul

@@ -31,13 +31,30 @@ impl<const LIMBS: usize> Int<LIMBS> {

(lo, hi, negate)
}

/// Multiply `self` by `rhs`, returning a concatenated "wide" result.
pub const fn widening_mul<const RHS_LIMBS: usize, const WIDE_LIMBS: usize>(
Copy link

Choose a reason for hiding this comment

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

Now that we have widening_mul, could be nice to implement Mul<Int<RHS_LIMBS>> for Int<LIMBS> similarly to Uint. This was done by me at the time using macros, the idea was for the standard type to have Mul<Int128> for Int192 and such.

src/int/sign.rs Show resolved Hide resolved
src/int/sign.rs Outdated Show resolved Hide resolved
src/int/sign.rs Outdated Show resolved Hide resolved
src/int/sign.rs Outdated Show resolved Hide resolved
src/int/sign.rs Outdated Show resolved Hide resolved
src/int/sign.rs Outdated Show resolved Hide resolved
src/int/mul.rs Show resolved Hide resolved
@erik-3milabs
Copy link
Owner

@lleoha thanks for adding this! It is a great addition 👌 Let's fine-tune the details in the conversations I started.

@erik-3milabs
Copy link
Owner

@lleoha looks good! There are two remarks still open. Could you please fix these? Then I'll merge.

Also, I've been so bold to commit a small code change :)

@lleoha
Copy link
Author

lleoha commented Oct 21, 2024

@lleoha looks good! There are two remarks still open. Could you please fix these? Then I'll merge.

Also, I've been so bold to commit a small code change :)

sure, i'll update it today after business hours CEST.

@lleoha
Copy link
Author

lleoha commented Oct 21, 2024

Could you please fix these?

done @erik-3milabs

@erik-3milabs erik-3milabs merged commit c8edd3d into erik-3milabs:int-twos Oct 22, 2024
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.

3 participants