diff --git a/crates/moon-collections/src/vecmap.rs b/crates/moon-collections/src/vecmap.rs index bec6596..885fbff 100644 --- a/crates/moon-collections/src/vecmap.rs +++ b/crates/moon-collections/src/vecmap.rs @@ -157,8 +157,13 @@ where match self { EntryRef::Occupied(entry) => entry.value, EntryRef::Vacant(entry) => { - entry.map.values.push(default(entry.key)); - entry.map.keys.push(entry.key.clone()); + // Build both halves before pushing either: a panic in `default` or in the key's + // `Clone` must leave `keys` and `values` the same length, or every later key + // pairs with its predecessor's value. + let value = default(entry.key); + let key = entry.key.clone(); + entry.map.values.push(value); + entry.map.keys.push(key); match entry.map.values.last_mut() { Some(value) => value, None => unreachable!("vec empty after pushing to it"), diff --git a/crates/moon-collections/src/vecmap_tests.rs b/crates/moon-collections/src/vecmap_tests.rs index 1f698f8..c4f5e13 100644 --- a/crates/moon-collections/src/vecmap_tests.rs +++ b/crates/moon-collections/src/vecmap_tests.rs @@ -209,3 +209,32 @@ fn test_entry_ref_mut_ref_can_be_updated() { *value = 5; assert_eq!(map.iter().collect::>(), vec![(&key, &5)]); } + +/// A key whose `Clone` panics, standing in for any fallible clone (an allocation, a refcount). +#[derive(PartialEq, Eq, Debug)] +struct PanickingCloneKey(u32); + +impl Clone for PanickingCloneKey { + fn clone(&self) -> Self { + panic!("clone of key {} failed", self.0); + } +} + +/// Catches `EntryRef::or_insert_with_key` pushing the value before cloning the key: a clone that +/// panics then leaves one value with no key, so every key inserted afterwards reads the value +/// stored beside the one before it. Like `HashMap`, a panic in caller code must leave the map as +/// it was: after the failed insert, key 2 maps to "two", exactly as if the insert never ran. +#[test] +fn test_entry_ref_panicking_key_clone_leaves_map_unchanged() { + let mut map: VecMap = VecMap::new(); + map.entry(PanickingCloneKey(1)).or_insert("one"); + + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + map.entry_ref(&PanickingCloneKey(99)).or_insert("lost"); + })); + assert!(result.is_err()); + + map.entry(PanickingCloneKey(2)).or_insert("two"); + let pairs: Vec<_> = map.iter().map(|(key, value)| (key.0, *value)).collect(); + assert_eq!(pairs, vec![(1, "one"), (2, "two")]); +}