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 Hash as a trait bound for PrimInt #320

Closed
obskyr opened this issue May 3, 2024 · 2 comments
Closed

Add Hash as a trait bound for PrimInt #320

obskyr opened this issue May 3, 2024 · 2 comments

Comments

@obskyr
Copy link

obskyr commented May 3, 2024

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
Copy link
Member

cuviper commented May 3, 2024

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
Copy link
Author

obskyr commented May 3, 2024

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?

@obskyr obskyr closed this as completed May 3, 2024
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

2 participants