Open virtualritz opened 7 months ago
Hi @virtualritz, on my end I'll try in my projects replacing occurrences of HashMap
s with it as well :)
It seems the only things missing are new()
and with_capacity()
.
I looked at the aHash
crate. They use two traits (orphan rules), HashMapExt
and HashSetExt
, that provide those and need to be brought into scope if the resp. type alias is being used.
On a sidenote: these aliases and their traits are also behind a std
feature as ahash
is no_std
when the collections are not used.
Maybe GxHash
could be no_std
too in this case (for embedded etc.)?
Furthermore aHash
's type aliases have the same names as their std
equivalents (no prefix). I.e. ahash::HashMap
is a type alias for std::collections::HashMap
and requires ahash::HashMapExt
to provide the constructors (to be 100% drop-in).
They also have a newtype wrapper, AHashMap
, which does not need additional traits but requires a lot of boilerplate. Maybe something for the future (and should have its own issue).
I suggest to rename the type alias GxHashMap
to HashMap
and put it behind a std
feature, i.e. mirror what ahash
is doing.
Then, if anyone asks for it in the future, the newtypes (called GxHashMap
and GxHashSet
) could be added at a later stage.
That way replacing aHash
with GxHash
in some third party crate is really just changing the top-level import prefix and the resp. crate in Cargo.toml
from ahash
to gxhash
:
use ahash::{HashMap, HashMapExt};
becomes:
use gxhash::{HashMap, HashMapExt};
Does that make sense? I could have a PR ready in the next hour. I'm on a train home and bored. :]
I checked in a few projects and I came up to the same conclusion (missing new
and with_capacity
because those are actually defined only for the HashMap
with RandomState
).
What you propose sounds great! Very convenient to use. Making it no_std
compatible might take some time however, maybe it can be done in two PRs, as you wish!
I'd say std
feature should be a default feature (same as in ahash)
I'm looking into both (the changes and the no_std
stuff). Should have something tomorrow.
There is a PR (#35) that should close this issue.
Unless you wanto to copy aHash
and add full newtype wrappers (GxHashMap
/GxHashSet
) at some point.
I just looked into replacing
ahash
withgxhash
elsewhere and ran intoHashMap::new()
missing.I reckon there may be more. I'll have a look later and may add some PRs to this issue. I'd keep it open until I'm certain the
gxhash
containers are full drop-in replacements for thestd
versions.