add args attribute to exceptions, make __str__ and __repr__ compatible with CPython

mkurnikov · mkurnikov · commit cd1d7b1a4395 · 2019-06-24T17:30:20.000+03:00
diff --git a/tests/snippets/exceptions.py b/tests/snippets/exceptions.py
@@ -0,0 +1,25 @@
+empty_exc = KeyError()
+assert str(empty_exc) == ''
+assert repr(empty_exc) == 'KeyError()'
+assert len(empty_exc.args) == 0
+assert type(empty_exc.args) == tuple
+
+exc = KeyError('message')
+assert str(exc) == "'message'"
+assert repr(exc) == "KeyError('message')"
+
+exc = KeyError('message', 'another message')
+assert str(exc) == "('message', 'another message')"
+assert repr(exc) == "KeyError('message', 'another message')"
+assert exc.args[0] == 'message'
+assert exc.args[1] == 'another message'
+
+class A:
+    def __repr__(self):
+        return 'repr'
+    def __str__(self):
+        return 'str'
+
+exc = KeyError(A())
+assert str(exc) == 'repr'
+assert repr(exc) == 'KeyError(repr)'
diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs
@@ -1,5 +1,6 @@
 use crate::function::PyFuncArgs;
 use crate::obj::objsequence;
+use crate::obj::objtuple::{PyTuple, PyTupleRef};
 use crate::obj::objtype;
 use crate::obj::objtype::PyClassRef;
 use crate::pyobject::{create_type, IdProtocol, PyContext, PyObjectRef, PyResult, TypeProtocol};
@@ -8,16 +9,12 @@ use std::fs::File;
 use std::io::{BufRead, BufReader};
 
 fn exception_init(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    let zelf = args.args[0].clone();
-    let msg = if args.args.len() > 1 {
-        args.args[1].clone()
-    } else {
-        let empty_string = String::default();
-        vm.new_str(empty_string)
-    };
+    let exc_self = args.args[0].clone();
+    let exc_args = vm.ctx.new_tuple(args.args[1..].to_vec());
+    vm.set_attr(&exc_self, "args", exc_args)?;
+
     let traceback = vm.ctx.new_list(Vec::new());
-    vm.set_attr(&zelf, "msg", msg)?;
-    vm.set_attr(&zelf, "__traceback__", traceback)?;
+    vm.set_attr(&exc_self, "__traceback__", traceback)?;
     Ok(vm.get_none())
 }
 
@@ -119,25 +116,70 @@ pub fn print_exception_inner(vm: &VirtualMachine, exc: &PyObjectRef) {
     }
 }
 
+fn exception_args_as_string(vm: &VirtualMachine, varargs: PyTupleRef) -> Vec<String> {
+    match varargs.elements.len() {
+        0 => vec![],
+        1 => {
+            let args0_repr = match vm.to_repr(&varargs.elements[0]) {
+                Ok(args0_repr) => args0_repr.value.clone(),
+                Err(_) => "<element repr() failed>".to_string(),
+            };
+            vec![args0_repr]
+        }
+        _ => {
+            let mut args_vec = Vec::with_capacity(varargs.elements.len());
+            for vararg in &varargs.elements {
+                let arg_repr = match vm.to_repr(vararg) {
+                    Ok(arg_repr) => arg_repr.value.clone(),
+                    Err(_) => "<element repr() failed>".to_string(),
+                };
+                args_vec.push(arg_repr);
+            }
+            args_vec
+        }
+    }
+}
+
 fn exception_str(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(
         vm,
         args,
         required = [(exc, Some(vm.ctx.exceptions.exception_type.clone()))]
     );
-    let msg = if let Ok(m) = vm.get_attribute(exc.clone(), "msg") {
-        match vm.to_pystr(&m) {
-            Ok(msg) => msg,
-            _ => "<exception str() failed>".to_string(),
-        }
-    } else {
-        panic!("Error message must be set");
+    let args = vm
+        .get_attribute(exc.clone(), "args")
+        .unwrap()
+        .downcast::<PyTuple>()
+        .expect("'args' must be a tuple");
+    let args_str = exception_args_as_string(vm, args);
+    let joined_str = match args_str.len() {
+        0 => "".to_string(),
+        1 => args_str[0].to_string(),
+        _ => format!("({})", args_str.join(", ")),
     };
-    let mut exc_repr = exc.class().name.clone();
-    if !msg.is_empty() {
-        &exc_repr.push_str(&format!(": {}", msg));
-    }
-    Ok(vm.new_str(exc_repr))
+    Ok(vm.new_str(joined_str))
+}
+
+fn exception_repr(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
+    arg_check!(
+        vm,
+        args,
+        required = [(exc, Some(vm.ctx.exceptions.exception_type.clone()))]
+    );
+    let args = vm
+        .get_attribute(exc.clone(), "args")
+        .unwrap()
+        .downcast::<PyTuple>()
+        .expect("'args' must be a tuple");
+    let args_repr = exception_args_as_string(vm, args);
+
+    let exc_repr = exc.class().name.clone();
+    let joined_str = match args_repr.len() {
+        0 => format!("{}()", exc_repr),
+        1 => format!("{}({})", exc_repr, args_repr[0].to_string()),
+        _ => format!("{}({})", exc_repr, args_repr.join(", ")),
+    };
+    Ok(vm.new_str(joined_str))
 }
 
 #[derive(Debug)]
@@ -266,6 +308,7 @@ pub fn init(context: &PyContext) {
 
     let exception_type = &context.exceptions.exception_type;
     extend_class!(context, exception_type, {
-        "__str__" => context.new_rustfunc(exception_str)
+        "__str__" => context.new_rustfunc(exception_str),
+        "__repr__" => context.new_rustfunc(exception_repr),
     });
 }
diff --git a/vm/src/vm.rs b/vm/src/vm.rs
@@ -173,7 +173,7 @@ impl VirtualMachine {
         self.invoke(exc_type.into_object(), args)
     }
 
-    /// Create Python instance of `exc_type` with message
+    /// 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:

Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ impl VirtualMachine {`
`173`	`173`	`self.invoke(exc_type.into_object(), args)`
`174`	`174`	`}`
`175`	`175`
`176`		- /// Create Python instance of `exc_type` with message
	`176`	+ /// Create Python instance of `exc_type` with message as first element of `args` tuple
`177`	`177`	`pub fn new_exception(&self, exc_type: PyClassRef, msg: String) -> PyObjectRef {`
`178`	`178`	`// TODO: exc_type may be user-defined exception, so we should return PyResult`
`179`	`179`	`// TODO: maybe there is a clearer way to create an instance:`