feat: add support for custom random state

[zhu-he]

HashMap and HashSet with custom random states such as ahash::HashMap and ahash::HashSet will be supported in this PR.

Note that this is a breaking change, as the type of return value is changed from std::collections::HashMap<_, _, std::collections::hash_map::RandomState> to std::collections::HashMap<_, _, S>.

[fujiapple852]

@zhu-he if you rebase this PR now it should green up the checks

[fujiapple852]

I was wondering about these new bounds and trying to understand why they are needed:

S: std::hash::BuildHasher + Default + 'static

For the Default bound it seems it is needed due to the use of collect() which needs FromIterator which is defined as follows (example for HashMap):

#[stable(feature = "rust1", since = "1.0.0")]
impl<K, V, S> FromIterator<(K, V)> for HashMap<K, V, S>
where
    K: Eq + Hash,
    S: BuildHasher + Default,
{
    fn from_iter<T: IntoIterator<Item = (K, V)>>(iter: T) -> HashMap<K, V, S> {
        let mut map = HashMap::with_hasher(Default::default());
        map.extend(iter);
        map
    }
}

The HashMap::with_hasher(Default::default() part is a bit surprising and a bit limiting.

For the 'static bound, it seems it is needed for a similar reason, by using collect() we don't get the chance to access S and call to_static() on it.

If we avoid using collect() and inline the impl we can solve both of these problems. I tried this:

#[cfg(feature = "std")]
/// Blanket [`ToBoundedStatic`] impl for converting `HashMap<K, V>` to `HashMap<K, V>: 'static`.
impl<K, V, S> ToBoundedStatic for std::collections::HashMap<K, V, S>
where
    K: ToBoundedStatic,
    K::Static: Eq + std::hash::Hash,
    V: ToBoundedStatic,
    S: ToBoundedStatic,
    S::Static: std::hash::BuildHasher,
{
    type Static = std::collections::HashMap<K::Static, V::Static, S::Static>;

    fn to_static(&self) -> Self::Static {
        let mut map = std::collections::HashMap::with_hasher(self.hasher().to_static());
        map.extend(self.iter().map(|(k, v)| (k.to_static(), v.to_static())));
        map
    }
}

To make it work I had to add an impl of ToBoundedStatic for RandomState:

impl ToBoundedStatic for std::collections::hash_map::RandomState {
    type Static = Self;

    fn to_static(&self) -> Self::Static {
        self.clone()
    }
}

I also had to fix up the unit tests to add the above impl for your custom RandomState (as well as making it Clone):

impl ToBoundedStatic for RandomState {
    type Static = Self;

    fn to_static(&self) -> Self::Static {
        self.clone()
    }
}

#[derive(Clone)]
struct RandomState;

I imagine the same approach could be used for the HashSet and will likely work for IntoBoundedStatic as well.

What do you think?

[zhu-he]

@fujiapple852 Thanks for the suggestion, but I think it is unnecessary to add the ToBoundedStatic impl for RandomState , and it is inappropriate to clone the RandomState.

First, almost all RandomStates as far as I know are already 'static, and I cannot imagine what a non-'static RandomState would look like.

Second, in the trait implementation, HashMap is created from scratch, so it is more meaningful to use the default RandomState than to clone the RandomState of the original HashMap.

After all, due to the orphan rule, it is hard to implement ToBoundedStatic for every RandomState in third-party crates. So I prefer to impl for all RandomStates which have BuildHasher + Default Clone + 'static bounds.

EDIT: It seems that I misunderstood the meaning of the BuildHasher.

[zhu-he]

@fujiapple852 I have updated the PR, and change the bound of S to BuildHasher + Clone + 'static.

There is another bonus reason not to impl IntoBoundedStatic for RandomState: there is no way to get the owned RandomState from a HashMap.

[fujiapple852]

almost all RandomStates as far as I know are already 'static, and I cannot imagine what a non-'static RandomState would look like

I tend to agree that this will be true in practice. My hesitation here is introducing bounds which aren't imposed by the underlying type such as HashMap, especially the 'static bound. The underlying types do impose some bounds such as S: Clone (for cloning) and S: Default (for FromIterator) so i'm less concerned about them. Notably RandomState from both the stdlib and aHash impl Clone and Default.

due to the orphan rule, it is hard to implement ToBoundedStatic for every RandomState in third-party crates.

This is generally true for the library; if you have a Vec<Foo> where Foo is 3rd party you have the same issue if you wish to use this library.

One way around this is to have an impl for common 3rd party types in the library behind a feature flag. This is the approach for some common types such as smol_str or smallvec. We could have impls for RandomState from aHash easily enough. Would that suffice?

[zhu-he]

@fujiapple852 Fine, I have added impls for RandomState, AHashMap and AHashSet from aHash, and added a feature flag ahash to enable them.

[fujiapple852]

LGTM!