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 truncate_* operations on BigInt and BigUint #202

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

digama0
Copy link

@digama0 digama0 commented Apr 11, 2021

Implements rust-num/num#404

@digama0
Copy link
Author

digama0 commented May 20, 2021

Hi @cuviper, gentle reminder that this PR exists and could use review.

@cuviper
Copy link
Member

cuviper commented May 22, 2021

I wonder if there's a way we can do this generically, rather than so many new methods, something like:

impl BigUint {
    pub fn truncate<T>(&self) -> T
    where
        T: PrimInt,
        u128: AsPrimitive<T>,
    {
        todo!("truncate as u128").as_()
    }
}

impl BigInt {
    pub fn truncate<T>(&self) -> T
    where
        T: PrimInt,
        i128: AsPrimitive<T>,
    {
        todo!("truncate as i128").as_()
    }
}

The u128/i128 distinction here doesn't really matter, but it looks appropriate.

Or maybe T: PrimInt is sufficient to build T directly? Not sure -- I think signs may be tricky.

@digama0
Copy link
Author

digama0 commented May 26, 2021

Check out the latest commit for a version that puts all the functions under a trait. It's not PrimInt, but a TruncateFrom trait; the actual front-end functions are the inherent truncate methods on BigInt and BigUint (which allows for turbofishing and also avoids the need to import the trait at use sites). BigInt::truncate has the cumbersome bound TruncateFrom<BigUint> + Zero + WrappingNeg but users shouldn't have to deal with it directly.

It probably shouldn't be merged as is, but if this looks good then the trait should move to the num_traits crate.

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.

2 participants