Skip to content

Commit

Permalink
Make insert_unique_unchecked unsafe
Browse files Browse the repository at this point in the history
This is in line with the standard library guarantees that it should be
impossible to create an inconsistent `HashMap` with well-defined key
types.
  • Loading branch information
Amanieu committed Sep 12, 2024
1 parent f01e271 commit fb57217
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 23 deletions.
4 changes: 3 additions & 1 deletion benches/insert_unique_unchecked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ fn insert_unique_unchecked(b: &mut Bencher) {
b.iter(|| {
let mut m = HashMap::with_capacity(1000);
for k in &keys {
m.insert_unique_unchecked(k, k);
unsafe {
m.insert_unique_unchecked(k, k);
}
}
m
});
Expand Down
32 changes: 20 additions & 12 deletions src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,8 +1761,17 @@ where
/// Insert a key-value pair into the map without checking
/// if the key already exists in the map.
///
/// This operation is faster than regular insert, because it does not perform
/// lookup before insertion.
///
/// This operation is useful during initial population of the map.
/// For example, when constructing a map from another map, we know
/// that keys are unique.
///
/// Returns a reference to the key and value just inserted.
///
/// # Safety
///
/// This operation is safe if a key does not exist in the map.
///
/// However, if a key exists in the map already, the behavior is unspecified:
Expand All @@ -1772,12 +1781,9 @@ where
/// That said, this operation (and following operations) are guaranteed to
/// not violate memory safety.
///
/// This operation is faster than regular insert, because it does not perform
/// lookup before insertion.
///
/// This operation is useful during initial population of the map.
/// For example, when constructing a map from another map, we know
/// that keys are unique.
/// However this operation is still unsafe because the resulting `HashMap`
/// may be passed to unsafe code which does expect the map to behave
/// correctly, and would could unsoundness as a result.
///
/// # Examples
///
Expand All @@ -1793,10 +1799,12 @@ where
/// let mut map2 = HashMap::new();
///
/// for (key, value) in map1.into_iter() {
/// map2.insert_unique_unchecked(key, value);
/// unsafe {
/// map2.insert_unique_unchecked(key, value);
/// }
/// }
///
/// let (key, value) = map2.insert_unique_unchecked(4, "d");
/// let (key, value) = unsafe { map2.insert_unique_unchecked(4, "d") };
/// assert_eq!(key, &4);
/// assert_eq!(value, &mut "d");
/// *value = "e";
Expand All @@ -1808,7 +1816,7 @@ where
/// assert_eq!(map2.len(), 4);
/// ```
#[cfg_attr(feature = "inline-more", inline)]
pub fn insert_unique_unchecked(&mut self, k: K, v: V) -> (&K, &mut V) {
pub unsafe fn insert_unique_unchecked(&mut self, k: K, v: V) -> (&K, &mut V) {
let hash = make_hash::<K, S>(&self.hash_builder, &k);
let bucket = self
.table
Expand Down Expand Up @@ -3187,7 +3195,7 @@ impl<'a, K, V, S, A: Allocator> IntoIterator for &'a HashMap<K, V, S, A> {
///
/// for (key, value) in &map_one {
/// println!("Key: {}, Value: {}", key, value);
/// map_two.insert_unique_unchecked(*key, *value);
/// map_two.insert(*key, *value);
/// }
///
/// assert_eq!(map_one, map_two);
Expand Down Expand Up @@ -5710,9 +5718,9 @@ mod test_map {
#[test]
fn test_insert_unique_unchecked() {
let mut map = HashMap::new();
let (k1, v1) = map.insert_unique_unchecked(10, 11);
let (k1, v1) = unsafe { map.insert_unique_unchecked(10, 11) };
assert_eq!((&10, &mut 11), (k1, v1));
let (k2, v2) = map.insert_unique_unchecked(20, 21);
let (k2, v2) = unsafe { map.insert_unique_unchecked(20, 21) };
assert_eq!((&20, &mut 21), (k2, v2));
assert_eq!(Some(&11), map.get(&10));
assert_eq!(Some(&21), map.get(&20));
Expand Down
24 changes: 14 additions & 10 deletions src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,25 +1118,29 @@ where

/// Insert a value the set without checking if the value already exists in the set.
///
/// Returns a reference to the value just inserted.
/// This operation is faster than regular insert, because it does not perform
/// lookup before insertion.
///
/// This operation is useful during initial population of the set.
/// For example, when constructing a set from another set, we know
/// that values are unique.
///
/// # Safety
///
/// This operation is safe if a value does not exist in the set.
/// This operation is safe if a key does not exist in the set.
///
/// However, if a value exists in the set already, the behavior is unspecified:
/// However, if a key exists in the set already, the behavior is unspecified:
/// this operation may panic, loop forever, or any following operation with the set
/// may panic, loop forever or return arbitrary result.
///
/// That said, this operation (and following operations) are guaranteed to
/// not violate memory safety.
///
/// This operation is faster than regular insert, because it does not perform
/// lookup before insertion.
///
/// This operation is useful during initial population of the set.
/// For example, when constructing a set from another set, we know
/// that values are unique.
/// However this operation is still unsafe because the resulting `HashSet`
/// may be passed to unsafe code which does expect the set to behave
/// correctly, and would could unsoundness as a result.
#[cfg_attr(feature = "inline-more", inline)]
pub fn insert_unique_unchecked(&mut self, value: T) -> &T {
pub unsafe fn insert_unique_unchecked(&mut self, value: T) -> &T {
self.map.insert_unique_unchecked(value, ()).0
}

Expand Down

0 comments on commit fb57217

Please sign in to comment.