Merge pull request #287 from japaric/binary-heap-double-drop

fix: BinaryHeap elements are dropped twice

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * Fixed `pool` example in docstring.
 * Fixed undefined behavior in `Vec::truncate()`, `Vec::swap_remove_unchecked()`,
   and `Hole::move_to()` (internal to the binary heap implementation).
+* Fixed `BinaryHeap` elements are being dropped twice
 
 ### Added
 

src/binary_heap.rs

@@ -537,12 +537,6 @@ where
     }
 }
 
-impl<T, K, const N: usize> Drop for BinaryHeap<T, K, N> {
-    fn drop(&mut self) {
-        unsafe { ptr::drop_in_place(self.data.as_mut_slice()) }
-    }
-}
-
 impl<T, K, const N: usize> fmt::Debug for BinaryHeap<T, K, N>
 where
     K: Kind,
@@ -577,6 +571,45 @@ mod tests {
         static mut _B: BinaryHeap<i32, Min, 16> = BinaryHeap::new();
     }
 
+    #[test]
+    fn drop() {
+        droppable!();
+
+        {
+            let mut v: BinaryHeap<Droppable, Max, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+            v.pop().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+
+        {
+            let mut v: BinaryHeap<Droppable, Max, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+
+        {
+            let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+            v.pop().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+
+        {
+            let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+    }
+
     #[test]
     fn min() {
         let mut heap = BinaryHeap::<_, Min, 16>::new();

src/lib.rs

@@ -85,6 +85,10 @@ pub use pool::singleton::arc::Arc;
 pub use string::String;
 pub use vec::Vec;
 
+#[macro_use]
+#[cfg(test)]
+mod test_helpers;
+
 mod deque;
 mod histbuf;
 mod indexmap;

src/test_helpers.rs

@@ -0,0 +1,23 @@
+macro_rules! droppable {
+    () => {
+        #[derive(Eq, Ord, PartialEq, PartialOrd)]
+        struct Droppable(i32);
+        impl Droppable {
+            fn new() -> Self {
+                unsafe {
+                    COUNT += 1;
+                    Droppable(COUNT)
+                }
+            }
+        }
+        impl Drop for Droppable {
+            fn drop(&mut self) {
+                unsafe {
+                    COUNT -= 1;
+                }
+            }
+        }
+
+        static mut COUNT: i32 = 0;
+    };
+}

src/vec.rs

@@ -901,29 +901,6 @@ mod tests {
         assert!(v.is_full());
     }
 
-    macro_rules! droppable {
-        () => {
-            struct Droppable;
-            impl Droppable {
-                fn new() -> Self {
-                    unsafe {
-                        COUNT += 1;
-                    }
-                    Droppable
-                }
-            }
-            impl Drop for Droppable {
-                fn drop(&mut self) {
-                    unsafe {
-                        COUNT -= 1;
-                    }
-                }
-            }
-
-            static mut COUNT: i32 = 0;
-        };
-    }
-
     #[test]
     fn drop() {
         droppable!();