Fix BinaryHeap place by only constructing vec::PlaceBack once

Author: Martin Hafskjold Thoresen

src/libcollections/binary_heap.rs

@@ -673,7 +673,7 @@ impl<T: Ord> BinaryHeap<T> {
     // the hole is filled back at the end of its scope, even on panic.
     // Using a hole reduces the constant factor compared to using swaps,
     // which involves twice as many moves.
-    fn sift_up(&mut self, start: usize, pos: usize) {
+    fn sift_up(&mut self, start: usize, pos: usize) -> usize {
         unsafe {
             // Take out the value at `pos` and create a hole.
             let mut hole = Hole::new(&mut self.data, pos);
@@ -685,21 +685,6 @@ impl<T: Ord> BinaryHeap<T> {
                 }
                 hole.move_to(parent);
             }
-        }
-    }
-
-    fn sift_up_ind(&mut self, start: usize, pos: usize) -> usize {
-        unsafe {
-            // Take out the value at `pos` and create a hole.
-            let mut hole = Hole::new(&mut self.data, pos);
-
-            while hole.pos() > start {
-                let parent = (hole.pos() - 1) / 2;
-                if hole.element() <= hole.get(parent) {
-                    return hole.pos();
-                }
-                hole.move_to(parent);
-            }
             hole.pos()
         }
     }
@@ -905,19 +890,6 @@ impl<T: Ord> BinaryHeap<T> {
     }
 }
 
-impl<T> BinaryHeap<T>
-where T: Clone + Ord {
-    /// kek
-    #[unstable(feature = "collection_placement",
-               reason = "placement protocol is subject to change",
-               issue = "30172")]
-    pub fn place(&mut self) -> PlaceIn<T> {
-        PlaceIn {
-            heap: self,
-        }
-    }
-}
-
 /// Hole represents a hole in a slice i.e. an index without valid value
 /// (because it was moved from or duplicated).
 /// In drop, `Hole` will restore the slice by filling the hole
@@ -1222,45 +1194,52 @@ impl<'a, T: 'a + Ord + Copy> Extend<&'a T> for BinaryHeap<T> {
 #[unstable(feature = "collection_placement",
            reason = "placement protocol is subject to change",
            issue = "30172")]
-pub struct PlaceIn<'a, T: 'a>
+pub struct BinaryHeapPlace<'a, T: 'a>
 where T: Clone + Ord {
-    heap: &'a mut BinaryHeap<T>,
+    heap: *mut BinaryHeap<T>,
+    place: vec::PlaceBack<'a, T>,
 }
 
 #[unstable(feature = "collection_placement",
            reason = "placement protocol is subject to change",
            issue = "30172")]
-impl<'a, T> Place<T> for PlaceIn<'a, T>
+impl<'a, T: 'a> Placer<T> for &'a mut BinaryHeap<T>
 where T: Clone + Ord {
-    fn pointer(&mut self) -> *mut T {
-        self.heap.data.place_back().pointer()
+    type Place = BinaryHeapPlace<'a, T>;
+
+    fn make_place(self) -> Self::Place {
+        let ptr = self as *mut BinaryHeap<T>;
+        let place = Placer::make_place(self.data.place_back());
+        BinaryHeapPlace {
+            heap: ptr,
+            place: place,
+        }
     }
 }
 
 #[unstable(feature = "collection_placement",
            reason = "placement protocol is subject to change",
            issue = "30172")]
-impl<'a, T> Placer<T> for PlaceIn<'a, T>
+impl<'a, T> Place<T> for BinaryHeapPlace<'a, T>
 where T: Clone + Ord {
-    type Place = PlaceIn<'a, T>;
-
-    fn make_place(self) -> Self {
-        let _ = self.heap.data.place_back().make_place();
-        self
+    fn pointer(&mut self) -> *mut T {
+        self.place.pointer()
     }
 }
 
 #[unstable(feature = "collection_placement",
            reason = "placement protocol is subject to change",
            issue = "30172")]
-impl<'a, T> InPlace<T> for PlaceIn<'a, T>
+impl<'a, T> InPlace<T> for BinaryHeapPlace<'a, T>
 where T: Clone + Ord {
     type Owner = &'a T;
 
     unsafe fn finalize(self) -> &'a T {
-        let len = self.heap.len();
-        let _ = self.heap.data.place_back().finalize();
-        let i = self.heap.sift_up_ind(0, len);
-        &mut self.heap.data[i]
+        self.place.finalize();
+
+        let heap: &mut BinaryHeap<T> = &mut *self.heap;
+        let len = heap.len();
+        let i = heap.sift_up(0, len - 1);
+        heap.data.get_unchecked(i)
     }
 }

src/libcollectionstest/binary_heap.rs

@@ -314,20 +314,20 @@ fn test_extend_specialization() {
 #[test]
 fn test_placement() {
     let mut a = BinaryHeap::new();
-    a.place() <- 2;
-    a.place() <- 4;
-    a.place() <- 3;
+    &mut a <- 2;
+    &mut a <- 4;
+    &mut a <- 3;
     assert_eq!(a.peek(), Some(&4));
     assert_eq!(a.len(), 3);
-    a.place() <- 1;
+    &mut a <- 1;
     assert_eq!(a.into_sorted_vec(), vec![1, 2, 3, 4]);
 }
 
 #[test]
 fn test_placement_panic() {
     let mut heap = BinaryHeap::from(vec![1, 2, 3]);
     fn mkpanic() -> usize { panic!() }
-    let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| { heap.place() <- mkpanic(); }));
+    let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| { &mut heap <- mkpanic(); }));
     assert_eq!(heap.len(), 3);
 }