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

impl IsUnsignedInteger trait for u8 too #590

Open
tdelabro opened this issue Oct 5, 2023 · 3 comments
Open

impl IsUnsignedInteger trait for u8 too #590

tdelabro opened this issue Oct 5, 2023 · 3 comments

Comments

@tdelabro
Copy link
Contributor

tdelabro commented Oct 5, 2023

in math/src/unsigned_integer/traits.rs

pub trait IsUnsignedInteger:
    Shr<usize, Output = Self>
    + BitAnd<Output = Self>
    + Eq
    + Ord
    + From<u16>
    + Copy
    + Display
    + Add<Self, Output = Self>
{
}

The From<u16> part prevents the trait from being impl for u8. Why is that? u8 is an unsigned integer too afterall? Can we change it and allow the trait to work with u8 too?

More broadly the trait name does not seem to correlate really well with the traits it requires: Shr, BitAnd, Add. It looks a bit arbitrary.

Also, this trait exists: https://docs.rs/num/latest/num/trait.Unsigned.html

@MauroToscano
Copy link
Collaborator

It could be improved a bit. The trait is expected to be used for custom Big Integers implementation. Creating a BigInt for an u8 seems a bit weird, but it's true it could be a improved. As for the operations it has, they are not arbitrary, they have the ones we need to build finite fields on top of it. More could be added

@tdelabro
Copy link
Contributor Author

tdelabro commented Oct 5, 2023

So it's probably just the choice of name which is misleading. It is more an IsValidForFiniteField trait than an IsUnsignedInteger trait.

@mahmudsudo
Copy link

can i take on this ?

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

No branches or pull requests

3 participants