Make RawIter::{reflect_insert, reflect_remove} unsafe

Fixes #412

Author: Amanieu

src/raw/mod.rs

@@ -2894,7 +2894,7 @@ impl<T> RawIter<T> {
     /// This method should be called _before_ the removal is made. It is not necessary to call this
     /// method if you are removing an item that this iterator yielded in the past.
     #[cfg(feature = "raw")]
-    pub fn reflect_remove(&mut self, b: &Bucket<T>) {
+    pub unsafe fn reflect_remove(&mut self, b: &Bucket<T>) {
         self.reflect_toggle_full(b, false);
     }
 
@@ -2908,97 +2908,95 @@ impl<T> RawIter<T> {
     ///
     /// This method should be called _after_ the given insert is made.
     #[cfg(feature = "raw")]
-    pub fn reflect_insert(&mut self, b: &Bucket<T>) {
+    pub unsafe fn reflect_insert(&mut self, b: &Bucket<T>) {
         self.reflect_toggle_full(b, true);
     }
 
     /// Refresh the iterator so that it reflects a change to the state of the given bucket.
     #[cfg(feature = "raw")]
-    fn reflect_toggle_full(&mut self, b: &Bucket<T>, is_insert: bool) {
-        unsafe {
-            if b.as_ptr() > self.iter.data.as_ptr() {
-                // The iterator has already passed the bucket's group.
-                // So the toggle isn't relevant to this iterator.
-                return;
+    unsafe fn reflect_toggle_full(&mut self, b: &Bucket<T>, is_insert: bool) {
+        if b.as_ptr() > self.iter.data.as_ptr() {
+            // The iterator has already passed the bucket's group.
+            // So the toggle isn't relevant to this iterator.
+            return;
+        }
+
+        if self.iter.next_ctrl < self.iter.end
+            && b.as_ptr() <= self.iter.data.next_n(Group::WIDTH).as_ptr()
+        {
+            // The iterator has not yet reached the bucket's group.
+            // We don't need to reload anything, but we do need to adjust the item count.
+
+            if cfg!(debug_assertions) {
+                // Double-check that the user isn't lying to us by checking the bucket state.
+                // To do that, we need to find its control byte. We know that self.iter.data is
+                // at self.iter.next_ctrl - Group::WIDTH, so we work from there:
+                let offset = offset_from(self.iter.data.as_ptr(), b.as_ptr());
+                let ctrl = self.iter.next_ctrl.sub(Group::WIDTH).add(offset);
+                // This method should be called _before_ a removal, or _after_ an insert,
+                // so in both cases the ctrl byte should indicate that the bucket is full.
+                assert!(is_full(*ctrl));
             }
 
-            if self.iter.next_ctrl < self.iter.end
-                && b.as_ptr() <= self.iter.data.next_n(Group::WIDTH).as_ptr()
-            {
-                // The iterator has not yet reached the bucket's group.
-                // We don't need to reload anything, but we do need to adjust the item count.
+            if is_insert {
+                self.items += 1;
+            } else {
+                self.items -= 1;
+            }
 
-                if cfg!(debug_assertions) {
-                    // Double-check that the user isn't lying to us by checking the bucket state.
-                    // To do that, we need to find its control byte. We know that self.iter.data is
-                    // at self.iter.next_ctrl - Group::WIDTH, so we work from there:
-                    let offset = offset_from(self.iter.data.as_ptr(), b.as_ptr());
-                    let ctrl = self.iter.next_ctrl.sub(Group::WIDTH).add(offset);
-                    // This method should be called _before_ a removal, or _after_ an insert,
-                    // so in both cases the ctrl byte should indicate that the bucket is full.
-                    assert!(is_full(*ctrl));
-                }
+            return;
+        }
+
+        // The iterator is at the bucket group that the toggled bucket is in.
+        // We need to do two things:
+        //
+        //  - Determine if the iterator already yielded the toggled bucket.
+        //    If it did, we're done.
+        //  - Otherwise, update the iterator cached group so that it won't
+        //    yield a to-be-removed bucket, or _will_ yield a to-be-added bucket.
+        //    We'll also need to update the item count accordingly.
+        if let Some(index) = self.iter.current_group.lowest_set_bit() {
+            let next_bucket = self.iter.data.next_n(index);
+            if b.as_ptr() > next_bucket.as_ptr() {
+                // The toggled bucket is "before" the bucket the iterator would yield next. We
+                // therefore don't need to do anything --- the iterator has already passed the
+                // bucket in question.
+                //
+                // The item count must already be correct, since a removal or insert "prior" to
+                // the iterator's position wouldn't affect the item count.
+            } else {
+                // The removed bucket is an upcoming bucket. We need to make sure it does _not_
+                // get yielded, and also that it's no longer included in the item count.
+                //
+                // NOTE: We can't just reload the group here, both since that might reflect
+                // inserts we've already passed, and because that might inadvertently unset the
+                // bits for _other_ removals. If we do that, we'd have to also decrement the
+                // item count for those other bits that we unset. But the presumably subsequent
+                // call to reflect for those buckets might _also_ decrement the item count.
+                // Instead, we _just_ flip the bit for the particular bucket the caller asked
+                // us to reflect.
+                let our_bit = offset_from(self.iter.data.as_ptr(), b.as_ptr());
+                let was_full = self.iter.current_group.flip(our_bit);
+                debug_assert_ne!(was_full, is_insert);
 
                 if is_insert {
                     self.items += 1;
                 } else {
                     self.items -= 1;
                 }
 
-                return;
-            }
-
-            // The iterator is at the bucket group that the toggled bucket is in.
-            // We need to do two things:
-            //
-            //  - Determine if the iterator already yielded the toggled bucket.
-            //    If it did, we're done.
-            //  - Otherwise, update the iterator cached group so that it won't
-            //    yield a to-be-removed bucket, or _will_ yield a to-be-added bucket.
-            //    We'll also need to update the item count accordingly.
-            if let Some(index) = self.iter.current_group.lowest_set_bit() {
-                let next_bucket = self.iter.data.next_n(index);
-                if b.as_ptr() > next_bucket.as_ptr() {
-                    // The toggled bucket is "before" the bucket the iterator would yield next. We
-                    // therefore don't need to do anything --- the iterator has already passed the
-                    // bucket in question.
-                    //
-                    // The item count must already be correct, since a removal or insert "prior" to
-                    // the iterator's position wouldn't affect the item count.
-                } else {
-                    // The removed bucket is an upcoming bucket. We need to make sure it does _not_
-                    // get yielded, and also that it's no longer included in the item count.
-                    //
-                    // NOTE: We can't just reload the group here, both since that might reflect
-                    // inserts we've already passed, and because that might inadvertently unset the
-                    // bits for _other_ removals. If we do that, we'd have to also decrement the
-                    // item count for those other bits that we unset. But the presumably subsequent
-                    // call to reflect for those buckets might _also_ decrement the item count.
-                    // Instead, we _just_ flip the bit for the particular bucket the caller asked
-                    // us to reflect.
-                    let our_bit = offset_from(self.iter.data.as_ptr(), b.as_ptr());
-                    let was_full = self.iter.current_group.flip(our_bit);
-                    debug_assert_ne!(was_full, is_insert);
-
-                    if is_insert {
-                        self.items += 1;
+                if cfg!(debug_assertions) {
+                    if b.as_ptr() == next_bucket.as_ptr() {
+                        // The removed bucket should no longer be next
+                        debug_assert_ne!(self.iter.current_group.lowest_set_bit(), Some(index));
                     } else {
-                        self.items -= 1;
-                    }
-
-                    if cfg!(debug_assertions) {
-                        if b.as_ptr() == next_bucket.as_ptr() {
-                            // The removed bucket should no longer be next
-                            debug_assert_ne!(self.iter.current_group.lowest_set_bit(), Some(index));
-                        } else {
-                            // We should not have changed what bucket comes next.
-                            debug_assert_eq!(self.iter.current_group.lowest_set_bit(), Some(index));
-                        }
+                        // We should not have changed what bucket comes next.
+                        debug_assert_eq!(self.iter.current_group.lowest_set_bit(), Some(index));
                     }
                 }
-            } else {
-                // We must have already iterated past the removed item.
             }
+        } else {
+            // We must have already iterated past the removed item.
         }
     }