Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make insert_unique_unchecked unsafe #556

Merged
merged 2 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
cuviper marked this conversation as resolved.
Show resolved Hide resolved
///
/// # 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:
cuviper marked this conversation as resolved.
Show resolved Hide resolved
/// 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.
cuviper marked this conversation as resolved.
Show resolved Hide resolved
#[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
Loading