Make HashMap INDEX_GET op fallible and fix Vm bug with invoking it

crates/runestick/src/key.rs

@@ -10,7 +10,7 @@ use std::sync::Arc;
 use std::vec;
 
 /// A key that can be used as an anonymous object key.
-#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum Key {
     /// A constant unit.
     Unit,
@@ -151,6 +151,23 @@ impl Key {
     }
 }
 
+impl fmt::Debug for Key {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Key::Unit => write!(f, "()"),
+            Key::Byte(b) => write!(f, "{:?}", b),
+            Key::Char(c) => write!(f, "{:?}", c),
+            Key::Bool(b) => write!(f, "{}", b),
+            Key::Integer(n) => write!(f, "{}", n),
+            Key::String(s) => write!(f, "{:?}", s),
+            Key::Bytes(b) => write!(f, "{:?}", b),
+            Key::Vec(vec) => write!(f, "{:?}", vec),
+            Key::Tuple(tuple) => write!(f, "{:?}", tuple),
+            Key::Option(opt) => write!(f, "{:?}", opt),
+        }
+    }
+}
+
 impl FromValue for Key {
     fn from_value(value: Value) -> Result<Self, VmError> {
         Key::from_value(&value)

crates/runestick/src/modules/collections.rs

@@ -1,6 +1,8 @@
 //! `std::collections` module.
 
-use crate::{Any, ContextError, Interface, Iterator, Key, Module, Ref, Value, VmError};
+use crate::{
+    Any, ContextError, Interface, Iterator, Key, Module, Ref, Value, VmError, VmErrorKind,
+};
 
 #[derive(Any)]
 #[rune(module = "crate")]
@@ -48,6 +50,20 @@ impl HashMap {
         self.map.get(&key).cloned()
     }
 
+    #[inline]
+    fn fallible_get(&self, key: Key) -> Result<Value, VmError> {
+        use crate::TypeOf as _;
+
+        let value = self.map.get(&key).ok_or_else(|| {
+            VmError::from(VmErrorKind::MissingIndexKey {
+                target: Self::type_info(),
+                index: key,
+            })
+        })?;
+
+        Ok(value.clone())
+    }
+
     #[inline]
     fn is_empty(&self) -> bool {
         self.map.is_empty()
@@ -248,7 +264,7 @@ pub fn module() -> Result<Module, ContextError> {
     module.inst_fn("clear", HashMap::clear)?;
     module.inst_fn(crate::Protocol::INTO_ITER, HashMap::iter)?;
     module.inst_fn(crate::Protocol::INDEX_SET, HashMap::insert)?;
-    module.inst_fn(crate::Protocol::INDEX_GET, HashMap::get)?;
+    module.inst_fn(crate::Protocol::INDEX_GET, HashMap::fallible_get)?;
 
     module.ty::<HashSet>()?;
     module.function(&["HashSet", "new"], HashSet::new)?;

crates/runestick/src/vm.rs

@@ -1960,47 +1960,41 @@ impl Vm {
         let index = self.stack.address(index)?;
         let target = self.stack.address_ref(target)?;
 
-        // This is a useful pattern.
-        #[allow(clippy::never_loop)]
-        loop {
-            match &index {
-                Value::String(string) => {
-                    let string_ref = string.borrow_ref()?;
+        match &index {
+            Value::String(string) => {
+                let string_ref = string.borrow_ref()?;
 
-                    if let Some(value) =
-                        Self::try_object_like_index_get(&target, string_ref.as_str())?
-                    {
-                        self.stack.push(value);
-                        return Ok(());
-                    }
+                if let Some(value) = Self::try_object_like_index_get(&target, string_ref.as_str())?
+                {
+                    self.stack.push(value);
+                    return Ok(());
                 }
-                Value::StaticString(string) => {
-                    if let Some(value) = Self::try_object_like_index_get(&target, string.as_ref())?
-                    {
-                        self.stack.push(value);
-                        return Ok(());
-                    }
+            }
+            Value::StaticString(string) => {
+                if let Some(value) = Self::try_object_like_index_get(&target, string.as_ref())? {
+                    self.stack.push(value);
+                    return Ok(());
                 }
-                Value::Integer(index) => {
-                    use std::convert::TryInto as _;
-
-                    let index = match (*index).try_into() {
-                        Ok(index) => index,
-                        Err(..) => {
-                            return Err(VmError::from(VmErrorKind::MissingIndex {
-                                target: target.type_info()?,
-                                index: VmIntegerRepr::from(*index),
-                            }));
-                        }
-                    };
-
-                    if let Some(value) = Self::try_tuple_like_index_get(&target, index)? {
-                        self.stack.push(value);
-                        return Ok(());
+            }
+            Value::Integer(index) => {
+                use std::convert::TryInto as _;
+
+                let index = match (*index).try_into() {
+                    Ok(index) => index,
+                    Err(..) => {
+                        return Err(VmError::from(VmErrorKind::MissingIndex {
+                            target: target.type_info()?,
+                            index: VmIntegerRepr::from(*index),
+                        }));
                     }
+                };
+
+                if let Some(value) = Self::try_tuple_like_index_get(&target, index)? {
+                    self.stack.push(value);
+                    return Ok(());
                 }
-                _ => break,
-            };
+            }
+            _ => (),
         }
 
         let target = target.into_owned();

crates/runestick/src/vm_error.rs

@@ -1,6 +1,7 @@
 use crate::panic::BoxedPanic;
 use crate::{
-    AccessError, Hash, Item, Panic, Protocol, StackError, TypeInfo, TypeOf, Unit, Value, VmHaltInfo,
+    AccessError, Hash, Item, Key, Panic, Protocol, StackError, TypeInfo, TypeOf, Unit, Value,
+    VmHaltInfo,
 };
 use std::fmt;
 use std::sync::Arc;
@@ -240,11 +241,13 @@ pub enum VmErrorKind {
     UnsupportedCallFn { actual_type: TypeInfo },
     #[error("missing index by static string slot `{slot}` in object")]
     ObjectIndexMissing { slot: usize },
-    #[error("missing index `{index}` on `{target}`")]
+    #[error("`{target}` missing index `{index}`")]
     MissingIndex {
         target: TypeInfo,
         index: VmIntegerRepr,
     },
+    #[error("`{target}` missing index `{index:?}`")]
+    MissingIndexKey { target: TypeInfo, index: Key },
     #[error("index out of bounds: the len is ${len} but the index is {index}")]
     OutOfRange {
         index: VmIntegerRepr,