From 374a670affcb0ef391495cee1516cf5e16311ba9 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 12 Sep 2024 07:45:03 -0400 Subject: [PATCH] Make `insert_unique_unchecked` unsafe This is in line with the standard library guarantees that it should be impossible to create an inconsistent `HashMap` with well-defined key types. --- src/map.rs | 24 +++++++++++++++--------- src/set.rs | 24 ++++++++++++++---------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/map.rs b/src/map.rs index 7bfd4b20e..9272e621c 100644 --- a/src/map.rs +++ b/src/map.rs @@ -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: @@ -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 /// @@ -1808,7 +1814,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::(&self.hash_builder, &k); let bucket = self .table @@ -5710,9 +5716,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)); diff --git a/src/set.rs b/src/set.rs index 6baa2d63a..3c59076b1 100644 --- a/src/set.rs +++ b/src/set.rs @@ -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 }