rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
731 stars 135 forks source link

Add `Hash` as a trait bound for `PrimInt` #320

Closed obskyr closed 7 months ago

obskyr commented 7 months ago

Currently, generic integers can't be used with hashmaps (which by extension means that they can't either be used with, for example, .unique() from itertools). Give the following code a whirl:

fn map_numbers<N: PrimInt>(numbers: Vec<N>) -> HashMap<N, N> {
    HashMap::from(
        numbers.iter()
        .map(|&n| (n, n.into()))
        .collect::<HashMap<N, N>>()
    )
}

It raises the following compiler error:

error[E0277]: the trait bound `N: Hash` is not satisfied
 |
 |         .collect::<HashMap<N, N>>()
 |          -------   ^^^^^^^^^^^^^ the trait `Hash` is not implemented for `N`, which is required by `HashMap<N, N>: 
 |
 = note: required for `HashMap<N, N>` to implement `FromIterator<(N, N)>`
note: required by a bound in `std::iter::Iterator::collect`

All of the standard types that PrimInt is implemented for implement Hash (which is why it makes sense for it to be a part of PrimInt), so this is only a breaking change if it's in scope for the API to implement PrimInt (which I wouldn't guessed it is based on the name, but I suppose it might be?).


It's possible to work around this issue by manually adding the trait bound:

fn map_numbers<N: PrimInt + std::hash::Hash>(numbers: Vec<N>) -> HashMap<N, N> {
    HashMap::from(
        numbers.iter()
        .map(|&n| (n, n.into()))
        .collect::<HashMap<N, N>>()
    )
}

But that's highly boilerplatitudinous, and bubbles up through anything that wants to call the function. It'd be wonderful to be able to comfortably write generic code for different number types in this area as well!

cuviper commented 7 months ago

so this is only a breaking change if it's in scope for the API to implement PrimInt (which I wouldn't guessed it is based on the name, but I suppose it might be?).

Yes it is, and there are real implementations in the wild, e.g. bnum. That example does also implement Hash, and probably most do, but we can't really assume that.

Slightly less boilerplate is to create your own extended trait with a blanket implementation:

trait MyPrimInt: PrimInt + Hash {}
impl<T: PrimInt + Hash> MyPrimInt for T {}
obskyr commented 7 months ago

Great to know! Alright, now that it's on your radar, I'll close this issue – I suppose it should be added to #47 “Tracking issue for potential breaking changes”, right?