Auto merge of #556 - Amanieu:api-changes, r=cuviper

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.

Author: bors

benches/insert_unique_unchecked.rs

@@ -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
     });

src/map.rs

@@ -1771,8 +1771,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:
@@ -1782,12 +1791,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 cause unsoundness as a result.
     ///
     /// # Examples
     ///
@@ -1803,10 +1809,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";
@@ -1818,7 +1826,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
@@ -3021,7 +3029,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);
@@ -5040,9 +5048,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));

src/set.rs

@@ -1093,7 +1093,14 @@ 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.
     ///
@@ -1104,14 +1111,11 @@ 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 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 cause 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
     }