Open cuviper opened 5 months ago
I didn't notice #400 before, but I think that's doing a lot more than is really needed for the immediate problem. For instance, I don't think it's important to compare the hashes, because that calculation is not under user control here. And value equality already implies hash equality, especially for known-good non-user types.
I didn't notice #400 before, but I think that's doing a lot more than is really needed for the immediate problem. For instance, I don't think it's important to compare the hashes, because that calculation is not under user control here. And value equality already implies hash equality, especially for known-good non-user types.
I can correct the code if necessary.
I didn't notice #400 before, but I think that's doing a lot more than is really needed for the immediate problem. For instance, I don't think it's important to compare the hashes, because that calculation is not under user control here. And value equality already implies hash equality, especially for known-good non-user types.
I removed unnecessary code. Now the comparison is carried out by values. I don’t think this will affect performance. I use the from_key_hashed_nocheck
method, so the hash is calculated only once, where the old version of the function calculated the hash twice (separately inside from_key
and inside or_insert_with
methods)
For some background, this came up while I was trying to remove RawEntry
from hashbrown (by moving it under an off-by-default cargo feature). The only internal uses of the raw entry API were from the HashSet::get_or_insert*
methods. I can rewrite get_or_insert
and get_or_insert_owned
to use Entry
and EntryRef
(with some minor changes). But for get_or_insert_with
I would have to fall back to the RawTable
API, which I would rather avoid: HashSet
should only expose operations that can also be performed on a normal HashMap
.
I can rewrite
get_or_insert
andget_or_insert_owned
to useEntry
andEntryRef
(with some minor changes). But forget_or_insert_with
I would have to fall back to theRawTable
API, which I would rather avoid:HashSet
should only expose operations that can also be performed on a normalHashMap
.
Well, get_or_insert
can be rewritten to Entry
by slightly changing OccupiedEntry::key
, namely by adding the lifetime 'a
to the key K
(pub fn key(&self) -> &'a K
).
But get_or_insert_owned
is not so easy to solve. First, you need to use EntryRef
, not Entry
. Secondly, all existing VacantEntryRef
methods work with K: From<&Q>
, and not with Q: ToOwned<Owned = K>
.
You need to either rewrite get_or_insert_owned
, or change the signatures of VacantEntryRef
to ToOwned
, or add a new method for VacantEntryRef (ala from_owned_with).
Well,
get_or_insert
can be rewritten toEntry
by slightly changingOccupiedEntry::key
, namely by adding the lifetime'a
to the keyK
(pub fn key(&self) -> &'a K
).
That would be unsound, because a user could follow that with a remove
call, making that reference dangling. That's why the raw entry has fn into_key(self) -> &'a mut K
, consuming the entry so it can no longer be used.
We could add a similar OccupiedEntry::into_key(self) -> &'a K
though.
That would be unsound, because a user could follow that with a
remove
call, making that reference dangling. That's why the raw entry hasfn into_key(self) -> &'a mut K
, consuming the entry so it can no longer be used.
That's right, I forgot about the remove
. Well, in any case, we will have to expand the existing HashMap API, if we want remove RawEntry
Since the Rust libs-api team considers it problematic for
HashSet<T>
to be unreliable with well-behavedT
(not user-controlled), we should not allow mismatches to be inserted throughget_or_insert_with
.