Merge pull request #448 from cuviper/assert_index

Unify index assertions with new util functions

Author: cuviper

src/inner.rs

@@ -15,7 +15,7 @@ use core::mem;
 use core::ops::RangeBounds;
 use hashbrown::hash_table;
 
-use crate::util::simplify_range;
+use crate::util::{assert_index_le, assert_index_lt, simplify_range};
 use crate::{Bucket, Equivalent, HashValue, TryReserveError};
 
 type Indices = hash_table::HashTable<usize>;
@@ -195,11 +195,7 @@ impl<K, V> Core<K, V> {
 
     #[track_caller]
     pub(crate) fn split_off(&mut self, at: usize) -> Self {
-        let len = self.entries.len();
-        assert!(
-            at <= len,
-            "index out of bounds: the len is {len} but the index is {at}. Expected index <= len"
-        );
+        assert_index_le(at, self.len());
 
         self.erase_indices(at, self.entries.len());
         let entries = self.entries.split_off(at);
@@ -651,9 +647,10 @@ impl<K, V> Core<K, V> {
 
     #[track_caller]
     pub(super) fn move_index(&mut self, from: usize, to: usize) {
+        assert_index_lt(from, self.len());
         let from_hash = self.entries[from].hash;
         if from != to {
-            let _ = self.entries[to]; // explicit bounds check
+            assert_index_lt(to, self.len());
 
             // Find the bucket index first so we won't lose it among other updated indices.
             let bucket = self
@@ -679,13 +676,14 @@ impl<K, V> Core<K, V> {
 
     #[track_caller]
     pub(crate) fn swap_indices(&mut self, a: usize, b: usize) {
-        // If they're equal and in-bounds, there's nothing to do.
-        if a == b && a < self.entries.len() {
+        assert_index_lt(a, self.len());
+        if a == b {
+            // If they're equal, there's nothing to do.
             return;
         }
+        assert_index_lt(b, self.len());
 
-        // We'll get a "nice" bounds-check from indexing `entries`,
-        // and then we expect to find it in the table as well.
+        // Since the indices are in-bounds, we expect to find them in the table as well.
         match self.indices.get_disjoint_mut(
             [self.entries[a].hash.get(), self.entries[b].hash.get()],
             move |i, &x| if i == 0 { x == a } else { x == b },

src/inner/entry.rs

@@ -1,6 +1,7 @@
 use super::{Bucket, Core, equal, get_hash};
 use crate::HashValue;
 use crate::map::{Entry, IndexedEntry};
+use crate::util::assert_index_lt;
 use core::cmp::Ordering;
 use core::mem;
 
@@ -199,7 +200,7 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> {
     #[track_caller]
     pub fn move_index(self, to: usize) {
         if self.index != to {
-            let _ = self.map.entries[to]; // explicit bounds check
+            assert_index_lt(to, self.map.len());
             self.map.move_index_inner(self.index, to);
             self.update_index(to);
         }
@@ -216,6 +217,8 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> {
     #[track_caller]
     pub fn swap_indices(self, other: usize) {
         if self.index != other {
+            assert_index_lt(other, self.map.len());
+
             // Since we already know where our bucket is, we only need to find the other.
             let hash = self.map.entries[other].hash;
             let other_mut = self.map.indices.find_mut(hash.get(), move |&i| i == other);
@@ -376,6 +379,7 @@ impl<'a, K, V> VacantEntry<'a, K, V> {
     #[track_caller]
     pub fn replace_index(self, index: usize) -> (K, OccupiedEntry<'a, K, V>) {
         let Self { map, hash, key } = self;
+        assert_index_lt(index, map.len());
 
         // NB: This removal and insertion isn't "no grow" (with unreachable hasher)
         // because hashbrown's tombstones might force a resize anyway.

src/map.rs

@@ -43,7 +43,7 @@ use core::ops::{Index, IndexMut, RangeBounds};
 use std::hash::RandomState;
 
 use crate::inner::Core;
-use crate::util::{third, try_simplify_range};
+use crate::util::{assert_index_le, assert_index_lt, third, try_simplify_range};
 use crate::{Bucket, Equivalent, GetDisjointMutError, HashValue, TryReserveError};
 
 /// A hash table where the iteration order of the key-value pairs is independent
@@ -595,12 +595,7 @@ where
     /// ```
     #[track_caller]
     pub fn insert_before(&mut self, mut index: usize, key: K, value: V) -> (usize, Option<V>) {
-        let len = self.len();
-
-        assert!(
-            index <= len,
-            "index out of bounds: the len is {len} but the index is {index}. Expected index <= len"
-        );
+        assert_index_le(index, self.len());
 
         match self.entry(key) {
             Entry::Occupied(mut entry) => {
@@ -683,21 +678,13 @@ where
         let len = self.len();
         match self.entry(key) {
             Entry::Occupied(mut entry) => {
-                assert!(
-                    index < len,
-                    "index out of bounds: the len is {len} but the index is {index}"
-                );
-
+                assert_index_lt(index, len);
                 let old = mem::replace(entry.get_mut(), value);
                 entry.move_index(index);
                 Some(old)
             }
             Entry::Vacant(entry) => {
-                assert!(
-                    index <= len,
-                    "index out of bounds: the len is {len} but the index is {index}. Expected index <= len"
-                );
-
+                assert_index_le(index, len);
                 entry.shift_insert(index, value);
                 None
             }
@@ -721,6 +708,8 @@ where
     /// Computes in **O(1)** time (average).
     #[track_caller]
     pub fn replace_index(&mut self, index: usize, key: K) -> Result<K, (usize, K)> {
+        assert_index_lt(index, self.len());
+
         // If there's a direct match, we don't even need to hash it.
         let entry = &mut self.as_entries_mut()[index];
         if key == entry.key {
@@ -1709,14 +1698,8 @@ impl<K, V, S> Index<usize> for IndexMap<K, V, S> {
     ///
     /// ***Panics*** if `index` is out of bounds.
     fn index(&self, index: usize) -> &V {
-        if let Some((_, value)) = self.get_index(index) {
-            value
-        } else {
-            panic!(
-                "index out of bounds: the len is {len} but the index is {index}",
-                len = self.len()
-            );
-        }
+        assert_index_lt(index, self.len());
+        &self.as_entries()[index].value
     }
 }
 
@@ -1754,13 +1737,8 @@ impl<K, V, S> IndexMut<usize> for IndexMap<K, V, S> {
     ///
     /// ***Panics*** if `index` is out of bounds.
     fn index_mut(&mut self, index: usize) -> &mut V {
-        let len: usize = self.len();
-
-        if let Some((_, value)) = self.get_index_mut(index) {
-            value
-        } else {
-            panic!("index out of bounds: the len is {len} but the index is {index}");
-        }
+        assert_index_lt(index, self.len());
+        &mut self.as_entries_mut()[index].value
     }
 }
 

src/set.rs

@@ -20,7 +20,7 @@ pub use crate::rayon::set as rayon;
 #[cfg(feature = "std")]
 use std::hash::RandomState;
 
-use crate::util::try_simplify_range;
+use crate::util::{assert_index_lt, try_simplify_range};
 use alloc::boxed::Box;
 use alloc::vec::Vec;
 use core::cmp::Ordering;
@@ -1281,14 +1281,8 @@ impl<T, S> Index<usize> for IndexSet<T, S> {
     ///
     /// ***Panics*** if `index` is out of bounds.
     fn index(&self, index: usize) -> &T {
-        if let Some(value) = self.get_index(index) {
-            value
-        } else {
-            panic!(
-                "index out of bounds: the len is {len} but the index is {index}",
-                len = self.len()
-            );
-        }
+        assert_index_lt(index, self.len());
+        &self.map.as_entries()[index].key
     }
 }
 

src/util.rs

@@ -4,6 +4,24 @@ pub(crate) fn third<A, B, C>(t: (A, B, C)) -> C {
     t.2
 }
 
+#[inline]
+#[track_caller]
+pub(crate) fn assert_index_lt(index: usize, len: usize) {
+    assert!(
+        index < len,
+        "index out of bounds: the len is {len} but the index is {index}",
+    );
+}
+
+#[inline]
+#[track_caller]
+pub(crate) fn assert_index_le(index: usize, len: usize) {
+    assert!(
+        index <= len,
+        "index out of bounds: the len is {len} but the index is {index}. Expected index <= len"
+    );
+}
+
 #[track_caller]
 pub(crate) fn simplify_range<R>(range: R, len: usize) -> Range<usize>
 where