save passed object into the first .args parameter for dict/mappingproxy KeyError

Author: mkurnikov

tests/snippets/dict.py

@@ -52,6 +52,10 @@
 with assertRaises(StopIteration):
     next(it)
 
+with assertRaises(KeyError) as cm:
+    del x[10]
+assert cm.exception.args[0] == 10
+
 # Iterating a dictionary is just its keys:
 assert ['a', 'b', 'd'] == list(x)
 
@@ -95,9 +99,6 @@
 x[2] = 2
 x[(5, 6)] = 5
 
-with assertRaises(KeyError):
-    x["not here"]
-
 with assertRaises(TypeError):
     x[[]] # Unhashable type.
 
@@ -167,19 +168,42 @@ def __eq__(self, other):
 y.update(y)
 assert y == {'a': 2, 'b': 12, 'c': 19, 'd': -1}  # hasn't changed
 
+# KeyError has object that used as key as an .args[0]
+with assertRaises(KeyError) as cm:
+    x['not here']
+assert cm.exception.args[0] == "not here"
+with assertRaises(KeyError) as cm:
+    x.pop('not here')
+assert cm.exception.args[0] == "not here"
+
+with assertRaises(KeyError) as cm:
+    x[10]
+assert cm.exception.args[0] == 10
+with assertRaises(KeyError) as cm:
+    x.pop(10)
+assert cm.exception.args[0] == 10
+
+class MyClass: pass
+obj = MyClass()
+
+with assertRaises(KeyError) as cm:
+    x[obj]
+assert cm.exception.args[0] == obj
+with assertRaises(KeyError) as cm:
+    x.pop(obj)
+assert cm.exception.args[0] == obj
+
 x = {1: 'a', '1': None}
 assert x.pop(1) == 'a'
 assert x.pop('1') is None
 assert x == {}
 
-with assertRaises(KeyError):
-    x.pop("not here")
-
 x = {1: 'a'}
 assert (1, 'a') == x.popitem()
-with assertRaises(KeyError):
-    x.popitem()
 assert x == {}
+with assertRaises(KeyError) as cm:
+    x.popitem()
+assert cm.exception.args == ('popitem(): dictionary is empty',)
 
 x = {'a': 4}
 assert 4 == x.setdefault('a', 0)

tests/snippets/mappingproxy.py

@@ -9,8 +9,10 @@ def b():
 
 
 assert A.__dict__['a'] == A.a
-with assertRaises(KeyError):
-    A.__dict__['not_here']
+with assertRaises(KeyError) as cm:
+    A.__dict__['not here']
+
+assert cm.exception.args[0] == "not here"
 
 assert 'b' in A.__dict__
 assert 'c' not in A.__dict__

tests/snippets/testutils.py

@@ -23,6 +23,7 @@ def assert_raises(exc_type, expr, msg=None):
 class assertRaises:
     def __init__(self, expected):
         self.expected = expected
+        self.exception = None
 
     def __enter__(self):
         return self
@@ -33,6 +34,8 @@ def __exit__(self, exc_type, exc_val, exc_tb):
             assert False, failmsg
         if not issubclass(exc_type, self.expected):
             return False
+
+        self.exception = exc_val
         return True
 
 

vm/src/dictdatatype.rs

@@ -129,8 +129,7 @@ impl<T: Clone> Dict<T> {
         if self.delete_if_exists(vm, key)? {
             Ok(())
         } else {
-            let key_repr = vm.to_pystr(key)?;
-            Err(vm.new_key_error(format!("Key not found: {}", key_repr)))
+            Err(vm.new_key_error(key.clone()))
         }
     }
 
@@ -247,8 +246,7 @@ impl<T: Clone> Dict<T> {
             self.unchecked_delete(index);
             Ok(value)
         } else {
-            let key_repr = vm.to_pystr(key)?;
-            Err(vm.new_key_error(format!("Key not found: {}", key_repr)))
+            Err(vm.new_key_error(key.clone()))
         }
     }
 

vm/src/obj/objdict.rs

@@ -210,7 +210,7 @@ impl PyDictRef {
             let method = method_or_err?;
             return vm.invoke(method, vec![key]);
         }
-        Err(vm.new_key_error(format!("Key not found: {}", vm.to_pystr(&key)?)))
+        Err(vm.new_key_error(key.clone()))
     }
 
     fn get(
@@ -266,7 +266,8 @@ impl PyDictRef {
         if let Some((key, value)) = entries.pop_front() {
             Ok(vm.ctx.new_tuple(vec![key, value]))
         } else {
-            Err(vm.new_key_error("popitem(): dictionary is empty".to_string()))
+            let err_msg = vm.new_str("popitem(): dictionary is empty".to_string());
+            Err(vm.new_key_error(err_msg))
         }
     }
 

vm/src/obj/objmappingproxy.rs

@@ -28,7 +28,7 @@ impl PyMappingProxy {
         if let Some(value) = objtype::class_get_attr(&self.class, key.as_str()) {
             return Ok(value);
         }
-        Err(vm.new_key_error(format!("Key not found: {}", key)))
+        Err(vm.new_key_error(key.into_object()))
     }
 
     #[pymethod(name = "__contains__")]

vm/src/obj/objset.rs

@@ -269,7 +269,8 @@ impl PySetInner {
         if let Some((key, _)) = self.content.pop_front() {
             Ok(key)
         } else {
-            Err(vm.new_key_error("pop from an empty set".to_string()))
+            let err_msg = vm.new_str("pop from an empty set".to_string());
+            Err(vm.new_key_error(err_msg))
         }
     }
 

vm/src/obj/objstr.rs

@@ -1335,7 +1335,7 @@ fn perform_format(
                 let result = match arguments.get_optional_kwarg(&keyword) {
                     Some(argument) => call_object_format(vm, argument.clone(), &format_spec)?,
                     None => {
-                        return Err(vm.new_key_error(format!("'{}'", keyword)));
+                        return Err(vm.new_key_error(vm.new_str(keyword.to_string())));
                     }
                 };
                 get_value(&result)

vm/src/stdlib/pwd.rs

@@ -48,7 +48,7 @@ fn pwd_getpwnam(name: PyStringRef, vm: &VirtualMachine) -> PyResult<Passwd> {
         Ok(Some(passwd)) => Ok(passwd),
         _ => {
             let name_repr = vm.to_repr(name.as_object())?;
-            let message = format!("getpwnam(): name not found: {}", name_repr);
+            let message = vm.new_str(format!("getpwnam(): name not found: {}", name_repr));
             Err(vm.new_key_error(message))
         }
     }
@@ -58,7 +58,7 @@ fn pwd_getpwuid(uid: u32, vm: &VirtualMachine) -> PyResult<Passwd> {
     match Passwd::from_uid(uid) {
         Some(passwd) => Ok(passwd),
         _ => {
-            let message = format!("getpwuid(): uid not found: {}", uid);
+            let message = vm.new_str(format!("getpwuid(): uid not found: {}", uid));
             Err(vm.new_key_error(message))
         }
     }

vm/src/vm.rs

@@ -167,20 +167,20 @@ impl VirtualMachine {
         self.ctx.new_bool(b)
     }
 
-    pub fn new_empty_exception(&self, exc_type: PyClassRef) -> PyResult {
+    fn new_exception_obj(&self, exc_type: PyClassRef, args: Vec<PyObjectRef>) -> PyResult {
+        // TODO: add repr of args into logging?
         info!("New exception created: {}", exc_type.name);
-        let args = PyFuncArgs::default();
         self.invoke(exc_type.into_object(), args)
     }
 
+    pub fn new_empty_exception(&self, exc_type: PyClassRef) -> PyResult {
+        self.new_exception_obj(exc_type, vec![])
+    }
+
     /// Create Python instance of `exc_type` with message as first element of `args` tuple
     pub fn new_exception(&self, exc_type: PyClassRef, msg: String) -> PyObjectRef {
-        // TODO: exc_type may be user-defined exception, so we should return PyResult
-        // TODO: maybe there is a clearer way to create an instance:
-        info!("New exception created: {}('{}')", exc_type.name, msg);
-        let pymsg = self.new_str(msg);
-        let args: Vec<PyObjectRef> = vec![pymsg];
-        self.invoke(exc_type.into_object(), args).unwrap()
+        let pystr_msg = self.new_str(msg);
+        self.new_exception_obj(exc_type, vec![pystr_msg]).unwrap()
     }
 
     pub fn new_attribute_error(&self, msg: String) -> PyObjectRef {
@@ -224,9 +224,9 @@ impl VirtualMachine {
         self.new_exception(value_error, msg)
     }
 
-    pub fn new_key_error(&self, msg: String) -> PyObjectRef {
+    pub fn new_key_error(&self, obj: PyObjectRef) -> PyObjectRef {
         let key_error = self.ctx.exceptions.key_error.clone();
-        self.new_exception(key_error, msg)
+        self.new_exception_obj(key_error, vec![obj]).unwrap()
     }
 
     pub fn new_index_error(&self, msg: String) -> PyObjectRef {