Merge pull request #367 from savannstm/better-panics

Improve panic messages and add `#[track_caller]` attribute to functions that may panic
indexmap-rs · Jan 20, 2025 · f0ec924 · f0ec924
2 parents 38ef618 + 2f55755
commit f0ec924
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 13 deletions.
diff --git a/src/map.rs b/src/map.rs
@@ -299,6 +299,7 @@ impl<K, V, S> IndexMap<K, V, S> {
     ///
     /// ***Panics*** if the starting point is greater than the end point or if
     /// the end point is greater than the length of the map.
+    #[track_caller]
     pub fn drain<R>(&mut self, range: R) -> Drain<'_, K, V>
     where
         R: RangeBounds<usize>,
@@ -313,6 +314,7 @@ impl<K, V, S> IndexMap<K, V, S> {
     /// the elements `[0, at)` with its previous capacity unchanged.
     ///
     /// ***Panics*** if `at > len`.
+    #[track_caller]
     pub fn split_off(&mut self, at: usize) -> Self
     where
         S: Clone,
@@ -493,8 +495,15 @@ where
     /// assert_eq!(map.get_index_of(&'+'), Some(27));
     /// assert_eq!(map.len(), 28);
     /// ```
+    #[track_caller]
     pub fn insert_before(&mut self, mut index: usize, key: K, value: V) -> (usize, Option<V>) {
-        assert!(index <= self.len(), "index out of bounds");
+        let len = self.len();
+
+        assert!(
+            index <= len,
+            "index out of bounds: the len is {len} but the index is {index}. Expected index <= len"
+        );
+
         match self.entry(key) {
             Entry::Occupied(mut entry) => {
                 if index > entry.index() {
@@ -571,17 +580,26 @@ where
     /// // This is an invalid index for moving an existing key!
     /// map.shift_insert(map.len(), 'a', ());
     /// ```
+    #[track_caller]
     pub fn shift_insert(&mut self, index: usize, key: K, value: V) -> Option<V> {
         let len = self.len();
         match self.entry(key) {
             Entry::Occupied(mut entry) => {
-                assert!(index < len, "index out of bounds");
+                assert!(
+                    index < len,
+                    "index out of bounds: the len is {len} but the index is {index}"
+                );
+
                 let old = mem::replace(entry.get_mut(), value);
                 entry.move_index(index);
                 Some(old)
             }
             Entry::Vacant(entry) => {
-                assert!(index <= len, "index out of bounds");
+                assert!(
+                    index <= len,
+                    "index out of bounds: the len is {len} but the index is {index}. Expected index <= len"
+                );
+
                 entry.shift_insert(index, value);
                 None
             }
@@ -627,6 +645,7 @@ where
     /// assert!(map.into_iter().eq([(0, '_'), (1, 'A'), (5, 'E'), (3, 'C'), (2, 'B'), (4, 'D')]));
     /// assert_eq!(removed, &[(2, 'b'), (3, 'c')]);
     /// ```
+    #[track_caller]
     pub fn splice<R, I>(&mut self, range: R, replace_with: I) -> Splice<'_, I::IntoIter, K, V, S>
     where
         R: RangeBounds<usize>,
@@ -1278,6 +1297,7 @@ impl<K, V, S> IndexMap<K, V, S> {
     /// ***Panics*** if `from` or `to` are out of bounds.
     ///
     /// Computes in **O(n)** time (average).
+    #[track_caller]
     pub fn move_index(&mut self, from: usize, to: usize) {
         self.core.move_index(from, to)
     }
@@ -1287,6 +1307,7 @@ impl<K, V, S> IndexMap<K, V, S> {
     /// ***Panics*** if `a` or `b` are out of bounds.
     ///
     /// Computes in **O(1)** time (average).
+    #[track_caller]
     pub fn swap_indices(&mut self, a: usize, b: usize) {
         self.core.swap_indices(a, b)
     }
@@ -1325,7 +1346,7 @@ where
     ///
     /// ***Panics*** if `key` is not present in the map.
     fn index(&self, key: &Q) -> &V {
-        self.get(key).expect("IndexMap: key not found")
+        self.get(key).expect("no entry found for key")
     }
 }
 
@@ -1367,7 +1388,7 @@ where
     ///
     /// ***Panics*** if `key` is not present in the map.
     fn index_mut(&mut self, key: &Q) -> &mut V {
-        self.get_mut(key).expect("IndexMap: key not found")
+        self.get_mut(key).expect("no entry found for key")
     }
 }
 
@@ -1411,7 +1432,12 @@ impl<K, V, S> Index<usize> for IndexMap<K, V, S> {
     /// ***Panics*** if `index` is out of bounds.
     fn index(&self, index: usize) -> &V {
         self.get_index(index)
-            .expect("IndexMap: index out of bounds")
+            .unwrap_or_else(|| {
+                panic!(
+                    "index out of bounds: the len is {len} but the index is {index}",
+                    len = self.len()
+                );
+            })
             .1
     }
 }
@@ -1450,8 +1476,12 @@ 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();
+
         self.get_index_mut(index)
-            .expect("IndexMap: index out of bounds")
+            .unwrap_or_else(|| {
+                panic!("index out of bounds: the len is {len} but the index is {index}");
+            })
             .1
     }
 }

diff --git a/src/map/core.rs b/src/map/core.rs
@@ -183,6 +183,7 @@ impl<K, V> IndexMapCore<K, V> {
         }
     }
 
+    #[track_caller]
     pub(crate) fn drain<R>(&mut self, range: R) -> vec::Drain<'_, Bucket<K, V>>
     where
         R: RangeBounds<usize>,
@@ -205,8 +206,14 @@ impl<K, V> IndexMapCore<K, V> {
         self.entries.par_drain(range)
     }
 
+    #[track_caller]
     pub(crate) fn split_off(&mut self, at: usize) -> Self {
-        assert!(at <= self.entries.len());
+        let len = self.entries.len();
+        assert!(
+            at <= len,
+            "index out of bounds: the len is {len} but the index is {at}. Expected index <= len"
+        );
+
         self.erase_indices(at, self.entries.len());
         let entries = self.entries.split_off(at);
 
@@ -215,6 +222,7 @@ impl<K, V> IndexMapCore<K, V> {
         Self { indices, entries }
     }
 
+    #[track_caller]
     pub(crate) fn split_splice<R>(&mut self, range: R) -> (Self, vec::IntoIter<Bucket<K, V>>)
     where
         R: RangeBounds<usize>,
@@ -403,11 +411,13 @@ impl<K, V> IndexMapCore<K, V> {
     }
 
     #[inline]
+    #[track_caller]
     pub(super) fn move_index(&mut self, from: usize, to: usize) {
         self.borrow_mut().move_index(from, to);
     }
 
     #[inline]
+    #[track_caller]
     pub(crate) fn swap_indices(&mut self, a: usize, b: usize) {
         self.borrow_mut().swap_indices(a, b);
     }
@@ -670,6 +680,7 @@ impl<'a, K, V> RefMut<'a, K, V> {
         }
     }
 
+    #[track_caller]
     fn move_index(&mut self, from: usize, to: usize) {
         let from_hash = self.entries[from].hash;
         let _ = self.entries[to]; // explicit bounds check
@@ -691,6 +702,7 @@ impl<'a, K, V> RefMut<'a, K, V> {
         }
     }
 
+    #[track_caller]
     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() {

diff --git a/src/map/core/entry.rs b/src/map/core/entry.rs
@@ -294,6 +294,7 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> {
     /// ***Panics*** if `to` is out of bounds.
     ///
     /// Computes in **O(n)** time (average).
+    #[track_caller]
     pub fn move_index(self, to: usize) {
         let index = self.index();
         self.into_ref_mut().move_index(index, to);
@@ -532,6 +533,7 @@ impl<'a, K, V> IndexedEntry<'a, K, V> {
     /// ***Panics*** if `to` is out of bounds.
     ///
     /// Computes in **O(n)** time (average).
+    #[track_caller]
     pub fn move_index(mut self, to: usize) {
         self.map.move_index(self.index, to);
     }

diff --git a/src/map/iter.rs b/src/map/iter.rs
@@ -667,6 +667,7 @@ where
     K: Hash + Eq,
     S: BuildHasher,
 {
+    #[track_caller]
     pub(super) fn new<R>(map: &'a mut IndexMap<K, V, S>, range: R, replace_with: I) -> Self
     where
         R: RangeBounds<usize>,

diff --git a/src/set.rs b/src/set.rs
@@ -250,6 +250,7 @@ impl<T, S> IndexSet<T, S> {
     ///
     /// ***Panics*** if the starting point is greater than the end point or if
     /// the end point is greater than the length of the set.
+    #[track_caller]
     pub fn drain<R>(&mut self, range: R) -> Drain<'_, T>
     where
         R: RangeBounds<usize>,
@@ -264,6 +265,7 @@ impl<T, S> IndexSet<T, S> {
     /// the elements `[0, at)` with its previous capacity unchanged.
     ///
     /// ***Panics*** if `at > len`.
+    #[track_caller]
     pub fn split_off(&mut self, at: usize) -> Self
     where
         S: Clone,
@@ -426,6 +428,7 @@ where
     /// assert_eq!(set.get_index_of(&'+'), Some(27));
     /// assert_eq!(set.len(), 28);
     /// ```
+    #[track_caller]
     pub fn insert_before(&mut self, index: usize, value: T) -> (usize, bool) {
         let (index, existing) = self.map.insert_before(index, value, ());
         (index, existing.is_none())
@@ -483,6 +486,7 @@ where
     /// // This is an invalid index for moving an existing value!
     /// set.shift_insert(set.len(), 'a');
     /// ```
+    #[track_caller]
     pub fn shift_insert(&mut self, index: usize, value: T) -> bool {
         self.map.shift_insert(index, value, ()).is_none()
     }
@@ -584,6 +588,7 @@ where
     /// assert!(set.into_iter().eq([0, 1, 5, 3, 2, 4]));
     /// assert_eq!(removed, &[2, 3]);
     /// ```
+    #[track_caller]
     pub fn splice<R, I>(&mut self, range: R, replace_with: I) -> Splice<'_, I::IntoIter, T, S>
     where
         R: RangeBounds<usize>,
@@ -1050,6 +1055,7 @@ impl<T, S> IndexSet<T, S> {
     /// ***Panics*** if `from` or `to` are out of bounds.
     ///
     /// Computes in **O(n)** time (average).
+    #[track_caller]
     pub fn move_index(&mut self, from: usize, to: usize) {
         self.map.move_index(from, to)
     }
@@ -1059,6 +1065,7 @@ impl<T, S> IndexSet<T, S> {
     /// ***Panics*** if `a` or `b` are out of bounds.
     ///
     /// Computes in **O(1)** time (average).
+    #[track_caller]
     pub fn swap_indices(&mut self, a: usize, b: usize) {
         self.map.swap_indices(a, b)
     }
@@ -1099,8 +1106,12 @@ impl<T, S> Index<usize> for IndexSet<T, S> {
     ///
     /// ***Panics*** if `index` is out of bounds.
     fn index(&self, index: usize) -> &T {
-        self.get_index(index)
-            .expect("IndexSet: index out of bounds")
+        self.get_index(index).unwrap_or_else(|| {
+            panic!(
+                "index out of bounds: the len is {len} but the index is {index}",
+                len = self.len()
+            );
+        })
     }
 }
 

diff --git a/src/set/iter.rs b/src/set/iter.rs
@@ -542,6 +542,7 @@ where
     T: Hash + Eq,
     S: BuildHasher,
 {
+    #[track_caller]
     pub(super) fn new<R>(set: &'a mut IndexSet<T, S>, range: R, replace_with: I) -> Self
     where
         R: RangeBounds<usize>,

diff --git a/src/util.rs b/src/util.rs
@@ -4,6 +4,7 @@ pub(crate) fn third<A, B, C>(t: (A, B, C)) -> C {
     t.2
 }
 
+#[track_caller]
 pub(crate) fn simplify_range<R>(range: R, len: usize) -> Range<usize>
 where
     R: RangeBounds<usize>,
@@ -12,17 +13,21 @@ where
         Bound::Unbounded => 0,
         Bound::Included(&i) if i <= len => i,
         Bound::Excluded(&i) if i < len => i + 1,
-        bound => panic!("range start {:?} should be <= length {}", bound, len),
+        Bound::Included(i) | Bound::Excluded(i) => {
+            panic!("range start index {i} out of range for slice of length {len}")
+        }
     };
     let end = match range.end_bound() {
         Bound::Unbounded => len,
         Bound::Excluded(&i) if i <= len => i,
         Bound::Included(&i) if i < len => i + 1,
-        bound => panic!("range end {:?} should be <= length {}", bound, len),
+        Bound::Included(i) | Bound::Excluded(i) => {
+            panic!("range end index {i} out of range for slice of length {len}")
+        }
     };
     if start > end {
         panic!(
-            "range start {:?} should be <= range end {:?}",
+            "range start index {:?} should be <= range end index {:?}",
             range.start_bound(),
             range.end_bound()
         );