From 4f3318b1d937b68f9ffecb3dea805a8e1e4588c3 Mon Sep 17 00:00:00 2001 From: hxperl Date: Tue, 15 Sep 2026 14:25:04 +0900 Subject: [PATCH] Make out-of-bounds panics report the caller's location `insert` is annotated `#[track_caller]`, but it raises its out-of-bounds panic from inside `try_insert`, which is not annotated. The annotation is therefore defeated on that path: a full-vector `insert` blames the caller, while an out-of-bounds `insert` blames src/arrayvec.rs. `remove` and `swap_remove` were never annotated either. #236 added `#[track_caller]` for the capacity-overflow panics and #212 was closed as fixed by it, but the `panic_oob!` family was left behind. Annotate `try_insert`, `remove` and `swap_remove`. `remove` and `swap_remove` raised their panic inside an `unwrap_or_else` closure, and closures do not inherit `#[track_caller]`, so both are rewritten as a `match` that panics directly in the annotated function body. Co-Authored-By: Claude Opus 5 (1M context) --- src/arrayvec.rs | 23 ++++++++++++++--------- tests/tests.rs | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/arrayvec.rs b/src/arrayvec.rs index f646b08..e584176 100644 --- a/src/arrayvec.rs +++ b/src/arrayvec.rs @@ -293,7 +293,7 @@ impl ArrayVec { /// /// Returns an error if vector is already at full capacity. /// - /// ***Panics*** `index` is out of bounds. + /// ***Panics*** if the `index` is out of bounds. /// /// ``` /// use arrayvec::ArrayVec; @@ -306,6 +306,7 @@ impl ArrayVec { /// assert_eq!(&array[..], &["y", "x"]); /// /// ``` + #[track_caller] pub fn try_insert(&mut self, index: usize, element: T) -> Result<(), CapacityError> { if index > self.len() { panic_oob!("try_insert", index, self.len()) @@ -369,11 +370,13 @@ impl ArrayVec { /// assert_eq!(array.swap_remove(1), 2); /// assert_eq!(&array[..], &[3]); /// ``` + #[track_caller] pub fn swap_remove(&mut self, index: usize) -> T { - self.swap_pop(index) - .unwrap_or_else(|| { - panic_oob!("swap_remove", index, self.len()) - }) + let len = self.len(); + match self.swap_pop(index) { + Some(element) => element, + None => panic_oob!("swap_remove", index, len), + } } /// Remove the element at `index` and swap the last element into its place. @@ -417,11 +420,13 @@ impl ArrayVec { /// assert_eq!(removed_elt, 1); /// assert_eq!(&array[..], &[2, 3]); /// ``` + #[track_caller] pub fn remove(&mut self, index: usize) -> T { - self.pop_at(index) - .unwrap_or_else(|| { - panic_oob!("remove", index, self.len()) - }) + let len = self.len(); + match self.pop_at(index) { + Some(element) => element, + None => panic_oob!("remove", index, len), + } } /// Remove the element at `index` and shift down the following elements. diff --git a/tests/tests.rs b/tests/tests.rs index 309ceb8..553ea55 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -821,3 +821,53 @@ fn test_arraystring_zero_filled_has_some_sanity_checks() { assert_eq!(string.as_str(), "\0\0\0\0"); assert_eq!(string.len(), 4); } + +#[test] +fn test_track_caller_out_of_bounds() { + // The out-of-bounds panics of `insert`/`try_insert`/`remove`/`swap_remove` + // must report the caller's location, not a line inside arrayvec. + use std::panic::{self, AssertUnwindSafe}; + use std::sync::{Arc, Mutex}; + + fn panic_file(f: F) -> String { + let slot = Arc::new(Mutex::new(String::new())); + let sink = Arc::clone(&slot); + let prev = panic::take_hook(); + panic::set_hook(Box::new(move |info| { + if let Some(loc) = info.location() { + *sink.lock().unwrap() = loc.file().to_string(); + } + })); + let result = panic::catch_unwind(AssertUnwindSafe(f)); + panic::set_hook(prev); + assert!(result.is_err(), "expected a panic"); + let file = slot.lock().unwrap().clone(); + file + } + + let here = file!(); + + assert_eq!(here, panic_file(|| { + let mut v = ArrayVec::::new(); + v.insert(3, 1); + })); + assert_eq!(here, panic_file(|| { + let mut v = ArrayVec::::new(); + let _ = v.try_insert(3, 1); + })); + assert_eq!(here, panic_file(|| { + let mut v = ArrayVec::::new(); + v.remove(3); + })); + assert_eq!(here, panic_file(|| { + let mut v = ArrayVec::::new(); + v.swap_remove(3); + })); + + // The capacity panic of `insert` already reported the caller; keep it that way. + assert_eq!(here, panic_file(|| { + let mut v = ArrayVec::::new(); + v.push(0); + v.insert(0, 1); + })); +}