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

[feature request] add Copy + PartialOrd + Sum + 'static supertraits to Num trait #243

Open
chahld opened this issue Jul 25, 2022 · 2 comments

Comments

@chahld
Copy link

chahld commented Jul 25, 2022

I find the Num trait to be very useful when defining template functions over generic number types to help avoid long bounds lists. However I almost always need to add the Copy trait bounds and the `static lifetime and less frequently some other bounds to my template bounds for the type in where statement. So far I've found the following are not in Num, but seem like they belong:

  • Copy
  • PartialOrd
  • Sum
  • Debug
  • AddAssign (and all the other ones)
  • 'static

There are likely others.

For example I would like to be able to do:

impl <T> my_struct<T> 
where T: Num
{
    //snip
}

Instead I have to do this (which I always forget to do)

impl <T> my_struct<T> 
where T: Num +PartialOrd + Copy + `static
{
    //snip
}

It appears that all current types that implement the Num trait are also value semantic "copy-able" types (with the possible exception of BigInt).

I realize I can get around this by defining a new trait:

mod num_utils {
    use std::iter::Sum;
    use std::fmt::Debug;
    use std::ops::AddAssign;
    use num::Num;

    pub trait Number: Num + Copy + PartialOrd + Sum + Debug +  AddAssign + 'static {}

    impl<T> Number for T
    where T: Num + Copy + PartialOrd + Sum + Debug + AddAssign + 'static
    {}
}

but I thought this might be of general benefit to other developers so should be included in the num crate.

Discussion:

Perhaps it is because of BigInt that Copy was left out? Bigint doesn't appear to have defined copy. I suppose this is because it could be very large, and maybe ought to force you to clone rather than auto-copying. I'm not sure I agree, but even if so, I think it would be preferable for Bigint to not implement Num but instead implement everything that Num implements. (or some intermediate trait. However that could makes things more complicated if there are too many intermediate aggregating traits)

@cuviper
Copy link
Member

cuviper commented Jul 25, 2022

Perhaps it is because of BigInt that Copy was left out? Bigint doesn't appear to have defined copy. I suppose this is because it could be very large, and maybe ought to force you to clone rather than auto-copying.

BigInt definitely cannot implement Copy, as that's essentially a shallow memcpy, which can never work with its heap-allocated digits. Those are currently in a Vec, which is never Copy, but this would be true of SmallVec and the like too.

I'm not sure I agree, but even if so, I think it would be preferable for Bigint to not implement Num but instead implement everything that Num implements.

It's more flexible to let Num cover more things, and you're free to use your own sub-traits that are more constrained.

Plus, it would be a breaking change for us to add more constraints on the existing traits.

@nsabovic
Copy link

I'd disagree on requiring a bound in a trait which is not used in a trait.

Basically you're trading typing a few characters for legibility and not being able to use num_traits for other numeric types, like BigInt but also a whole slew of other types in scientific computing.

Looking at the design of the standard library, the bounds are only used where they are actually needed. For instance you can construct a HashMap of anything really but you can't call get() unless K is Eq + Hashable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants