From bef5fff2156e7fea191f134f4989a1d53e242ea1 Mon Sep 17 00:00:00 2001 From: youdie006 Date: Mon, 14 Sep 2026 16:29:26 +0900 Subject: [PATCH] Use assert_index_le and #[track_caller] in shift_insert_unique PR #448 unified the index assertions in this file, but Core::shift_insert_unique kept a bare assert!(index <= end) and is the only index-taking Core method that panics without #[track_caller]. That leaks through the public VacantEntry::shift_insert, which panics with "assertion failed: index <= end" located in indexmap's internals instead of the caller. Also covers RawVacantEntryMut::shift_insert and shift_insert_hashed_nocheck, which reach the same assertion. --- src/inner.rs | 3 ++- src/map/tests.rs | 23 +++++++++++++++++++++++ tests/track_caller.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 tests/track_caller.rs diff --git a/src/inner.rs b/src/inner.rs index bb4c77ea..48aa9207 100644 --- a/src/inner.rs +++ b/src/inner.rs @@ -520,6 +520,7 @@ impl Core { /// Insert a key-value pair in `entries` at a particular index, /// *without* checking whether it already exists. + #[track_caller] pub(crate) fn shift_insert_unique( &mut self, index: usize, @@ -528,7 +529,7 @@ impl Core { value: V, ) -> &mut Bucket { let end = self.indices.len(); - assert!(index <= end); + assert_index_le(index, end); // Increment others first so we don't have duplicate indices. self.increment_indices(index, end); let entries = &*self.entries; diff --git a/src/map/tests.rs b/src/map/tests.rs index 5b7a174c..c919a183 100644 --- a/src/map/tests.rs +++ b/src/map/tests.rs @@ -789,6 +789,29 @@ fn get_range_mut() { assert_eq!(slice, &mut [(3, 31), (4, 41)]); } +#[test] +#[should_panic = "index out of bounds: the len is 1 but the index is 5. Expected index <= len"] +fn vacant_entry_shift_insert_oob() { + let mut map: IndexMap = IndexMap::new(); + map.insert(0, 0); + let Entry::Vacant(entry) = map.entry(1) else { + unreachable!() + }; + entry.shift_insert(5, 10); +} + +#[test] +fn vacant_entry_shift_insert_at_len() { + let mut map: IndexMap = IndexMap::new(); + map.insert(0, 0); + let len = map.len(); + let Entry::Vacant(entry) = map.entry(1) else { + unreachable!() + }; + entry.shift_insert(len, 10); + assert_eq!(map.get_index(1), Some((&1, &10))); +} + #[test] #[should_panic = "index out of bounds"] fn shift_insert_oob() { diff --git a/tests/track_caller.rs b/tests/track_caller.rs new file mode 100644 index 00000000..a25abee3 --- /dev/null +++ b/tests/track_caller.rs @@ -0,0 +1,33 @@ +//! Panics from the index-taking `IndexMap` entry points must name the caller. +//! +//! This is its own test binary because it replaces the global panic hook. + +use indexmap::IndexMap; +use indexmap::map::Entry; + +#[test] +fn vacant_entry_shift_insert_oob_reports_the_caller() { + let file = std::sync::Arc::new(std::sync::Mutex::new(String::new())); + let hook_file = std::sync::Arc::clone(&file); + let prev = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + if let Some(loc) = info.location() { + *hook_file.lock().unwrap() = String::from(loc.file()); + } + })); + let caught = std::panic::catch_unwind(|| { + let mut map: IndexMap = IndexMap::new(); + map.insert(0, 0); + let Entry::Vacant(entry) = map.entry(1) else { + unreachable!() + }; + entry.shift_insert(5, 10); + }); + std::panic::set_hook(prev); + assert!(caught.is_err()); + let file = file.lock().unwrap().clone(); + assert!( + file.ends_with("track_caller.rs"), + "panic reported at {file}" + ); +}