Skip to content

Use assert_index_le and #[track_caller] in shift_insert_unique - #451

Open
youdie006 wants to merge 1 commit into
indexmap-rs:mainfrom
youdie006:track-caller-shift-insert-unique
Open

youdie006 wants to merge 1 commit into
indexmap-rs:mainfrom
youdie006:track-caller-shift-insert-unique

Conversation

@youdie006

Copy link
Copy Markdown

#448 unified the index assertions in src/inner.rs, but Core::shift_insert_unique kept its bare assert!(index <= end) (src/inner.rs:531) and is the only index-taking Core method that panics without #[track_caller]drain:173, split_off:196, split_splice:208, replace_index_unique:508, move_index:648 and swap_indices:677 all have it. (shift_remove_index:551 and swap_remove_index:574 take an index too, but return Option and never panic.)

That leaks through the public VacantEntry::shift_insert, whose doc at src/inner/entry.rs:363 promises ***Panics*** if index is out of bounds. Its neighbour in the same impl block, VacantEntry::replace_index (src/inner/entry.rs:381), carries the same doc line and does it right.

On a map of len 1, inserting at index 5:

call message location
IndexMap::shift_insert index out of bounds: the len is 1 but the index is 5. Expected index <= len the caller
IndexMap::insert_before same the caller
VacantEntry::shift_insert assertion failed: index <= end indexmap/src/inner.rs:531
std Vec::insert (reference) insertion index (is 5) should be <= len (is 1) the caller

RawVacantEntryMut::shift_insert and shift_insert_hashed_nocheck (src/map/raw_entry_v1.rs:596,612) reach the same assertion and are fixed by the same two lines. IndexMap::shift_insert and IndexMap::insert_before already assert before delegating, so they are unaffected.

Verification

Base 41a8708. Three tests: the message (vacant_entry_shift_insert_oob), the guard row where index == len must still succeed (vacant_entry_shift_insert_at_len), and the location. The location test replaces the global panic hook, so it lives in its own test binary (tests/track_caller.rs) rather than in the shared --lib harness.

row src/inner.rs md5 --lib vacant_entry --test track_caller
RED (base) 725aaef7… 2 ran, 1 failed ("assertion failed: index <= end") 1 ran, failed (panic reported at src/inner.rs)
GREEN d8c71a57… 2 ran, 2 passed 1 ran, 1 passed
MUT-A revert to assert!(index <= end) f64c4378… 2 ran, …_oob failed 1 ran, passed
MUT-B over-correct to assert_index_lt 1bc23f6c… 2 ran, …_oob and …_at_len failed 1 ran, passed
MUT-C drop #[track_caller] 87eff44f… 2 ran, 2 passed 1 ran, failed

Each mutant was killed by a different test, so the two halves of the change are independently covered. Every mutated md5 differs from GREEN's, and each anchor's occurrence count was asserted as 1 before the rewrite.

CI gates, run as .github/workflows/ci.yml writes them:

gate result
cargo test / cargo test --release, features "", arbitrary, quickcheck, rayon, serde, sval, borsh, std 16/16 exit 0; 227-238 passed per row, 7 test binaries
MSRV 1.85.0: build, doc, test, test --release exit 0; 227 passed
cargo clippy --all-features exit 0, no warnings (run on stable; CI uses beta)
cargo doc --all-features exit 0
cargo build --target=thumbv6m-none-eabi --no-default-features exit 0
cargo miri test --doc 41 ran, 41 passed
cargo miri test on both new binaries 2 passed, 1 passed

If you would rather not carry the panic-hook test, the #[track_caller] line stands on #367 and #389 on its own and that file can be dropped.


Disclosure: written with the help of Claude (an AI assistant). The verification above is from runs on my machine; I have reviewed the change and stand behind it.

PR indexmap-rs#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.
@youdie006
youdie006 force-pushed the track-caller-shift-insert-unique branch from 4f991e2 to bef5fff Compare September 15, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant