Skip to content

Commit 8bceb97

Browse files
committed
Wean HashSet from the raw-entry API
This changes `get_or_insert`, `get_or_insert_with`, and `bitxor_assign` to poke directly at the `RawTable` instead of using `raw_entry_mut()`. `HashSet::get_or_insert_with` also asserts that the converted value is actually equivalent after conversion, so we can ensure set consistency. `HashSet::get_or_insert_owned` is removed for now, since it offers no value over the `_with` method, as we would need to assert that too.
1 parent 3b350d7 commit 8bceb97

File tree

2 files changed

+48
-58
lines changed

2 files changed

+48
-58
lines changed

ci/run.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ if [ "${CHANNEL}" = "nightly" ]; then
4343
fi
4444

4545
# Make sure we can compile without the default hasher
46-
"${CARGO}" -vv build --target="${TARGET}" --no-default-features --features raw-entry
47-
"${CARGO}" -vv build --target="${TARGET}" --release --no-default-features --features raw-entry
46+
"${CARGO}" -vv build --target="${TARGET}" --no-default-features
47+
"${CARGO}" -vv build --target="${TARGET}" --release --no-default-features
4848

4949
"${CARGO}" -vv ${OP} --target="${TARGET}"
5050
"${CARGO}" -vv ${OP} --target="${TARGET}" --features "${FEATURES}"

src/set.rs

Lines changed: 46 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::{Equivalent, TryReserveError};
2-
use alloc::borrow::ToOwned;
32
use core::hash::{BuildHasher, Hash};
43
use core::iter::{Chain, FusedIterator};
54
use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Sub, SubAssign};
@@ -911,45 +910,16 @@ where
911910
/// ```
912911
#[cfg_attr(feature = "inline-more", inline)]
913912
pub fn get_or_insert(&mut self, value: T) -> &T {
914-
// Although the raw entry gives us `&mut T`, we only return `&T` to be consistent with
915-
// `get`. Key mutation is "raw" because you're not supposed to affect `Eq` or `Hash`.
916-
self.map
917-
.raw_entry_mut()
918-
.from_key(&value)
919-
.or_insert(value, ())
920-
.0
921-
}
922-
923-
/// Inserts an owned copy of the given `value` into the set if it is not
924-
/// present, then returns a reference to the value in the set.
925-
///
926-
/// # Examples
927-
///
928-
/// ```
929-
/// use hashbrown::HashSet;
930-
///
931-
/// let mut set: HashSet<String> = ["cat", "dog", "horse"]
932-
/// .iter().map(|&pet| pet.to_owned()).collect();
933-
///
934-
/// assert_eq!(set.len(), 3);
935-
/// for &pet in &["cat", "dog", "fish"] {
936-
/// let value = set.get_or_insert_owned(pet);
937-
/// assert_eq!(value, pet);
938-
/// }
939-
/// assert_eq!(set.len(), 4); // a new "fish" was inserted
940-
/// ```
941-
#[inline]
942-
pub fn get_or_insert_owned<Q>(&mut self, value: &Q) -> &T
943-
where
944-
Q: Hash + Equivalent<T> + ToOwned<Owned = T> + ?Sized,
945-
{
946-
// Although the raw entry gives us `&mut T`, we only return `&T` to be consistent with
947-
// `get`. Key mutation is "raw" because you're not supposed to affect `Eq` or `Hash`.
948-
self.map
949-
.raw_entry_mut()
950-
.from_key(value)
951-
.or_insert_with(|| (value.to_owned(), ()))
952-
.0
913+
let hash = make_hash(&self.map.hash_builder, &value);
914+
let bucket = match self.map.table.find_or_find_insert_slot(
915+
hash,
916+
equivalent_key(&value),
917+
make_hasher(&self.map.hash_builder),
918+
) {
919+
Ok(bucket) => bucket,
920+
Err(slot) => unsafe { self.map.table.insert_in_slot(hash, slot, (value, ())) },
921+
};
922+
unsafe { &bucket.as_ref().0 }
953923
}
954924

955925
/// Inserts a value computed from `f` into the set if the given `value` is
@@ -970,19 +940,33 @@ where
970940
/// }
971941
/// assert_eq!(set.len(), 4); // a new "fish" was inserted
972942
/// ```
943+
///
944+
/// The following example will panic because the new value doesn't match.
945+
///
946+
/// ```should_panic
947+
/// let mut set = hashbrown::HashSet::new();
948+
/// set.get_or_insert_with("rust", |_| String::new());
949+
/// ```
973950
#[cfg_attr(feature = "inline-more", inline)]
974951
pub fn get_or_insert_with<Q, F>(&mut self, value: &Q, f: F) -> &T
975952
where
976953
Q: Hash + Equivalent<T> + ?Sized,
977954
F: FnOnce(&Q) -> T,
978955
{
979-
// Although the raw entry gives us `&mut T`, we only return `&T` to be consistent with
980-
// `get`. Key mutation is "raw" because you're not supposed to affect `Eq` or `Hash`.
981-
self.map
982-
.raw_entry_mut()
983-
.from_key(value)
984-
.or_insert_with(|| (f(value), ()))
985-
.0
956+
let hash = make_hash(&self.map.hash_builder, &value);
957+
let bucket = match self.map.table.find_or_find_insert_slot(
958+
hash,
959+
equivalent_key(value),
960+
make_hasher(&self.map.hash_builder),
961+
) {
962+
Ok(bucket) => bucket,
963+
Err(slot) => {
964+
let new = f(value);
965+
assert!(value.equivalent(&new), "new value is not equivalent");
966+
unsafe { self.map.table.insert_in_slot(hash, slot, (new, ())) }
967+
}
968+
};
969+
unsafe { &bucket.as_ref().0 }
986970
}
987971

988972
/// Gets the given value's corresponding entry in the set for in-place manipulation.
@@ -1607,15 +1591,21 @@ where
16071591
/// ```
16081592
fn bitxor_assign(&mut self, rhs: &HashSet<T, S, A>) {
16091593
for item in rhs {
1610-
let entry = self.map.raw_entry_mut().from_key(item);
1611-
match entry {
1612-
map::RawEntryMut::Occupied(e) => {
1613-
e.remove();
1614-
}
1615-
map::RawEntryMut::Vacant(e) => {
1616-
e.insert(item.to_owned(), ());
1617-
}
1618-
};
1594+
let hash = make_hash(&self.map.hash_builder, &item);
1595+
match self.map.table.find_or_find_insert_slot(
1596+
hash,
1597+
equivalent_key(item),
1598+
make_hasher(&self.map.hash_builder),
1599+
) {
1600+
Ok(bucket) => unsafe {
1601+
self.map.table.remove(bucket);
1602+
},
1603+
Err(slot) => unsafe {
1604+
self.map
1605+
.table
1606+
.insert_in_slot(hash, slot, (item.clone(), ()));
1607+
},
1608+
}
16191609
}
16201610
}
16211611
}

0 commit comments

Comments
 (0)