Cleanup exceptions a bit

2019-12-17 11:43:23 -06:00
parent f22a12d555
commit cd726a2818
7 changed files with 65 additions and 106 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -598,7 +598,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"

 [[package]]
 name = "itertools"
-version = "0.8.0"
+version = "0.8.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
 "either 1.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -633,7 +633,7 @@ dependencies = [
 "diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
 "docopt 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "ena 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "lalrpop-util 0.17.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "petgraph 0.4.13 (registry+https://github.com/rust-lang/crates.io-index)",
 "regex 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1255,7 +1255,7 @@ version = "0.1.1"
 dependencies = [
 "arrayvec 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "indexmap 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
 "num-complex 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
 "rustpython-bytecode 0.1.1",
@@ -1316,7 +1316,7 @@ dependencies = [
 "hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "hexf-parse 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "indexmap 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "lexical 4.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2226,7 +2226,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum hexf-parse 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "79296f72d53a89096cbc9a88c9547ee8dfe793388674620e2207593d370550ac"
 "checksum humantime 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "df004cfca50ef23c36850aaaa59ad52cc70d0e90243c3c7737a4dd32dc7a3c4f"
 "checksum indexmap 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a61202fbe46c4a951e9404a720a0180bcf3212c750d735cb5c4ba4dc551299f3"
-"checksum itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5b8467d9c1cebe26feb08c640139247fac215782d35371ade9a2136ed6085358"
+"checksum itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f56a2d0bc861f9165be4eb3442afd3c236d8a98afd426f65d92324ae1091a484"
 "checksum itoa 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "501266b7edd0174f8530248f87f99c88fbe60ca4ef3dd486835b8d8d53136f7f"
 "checksum js-sys 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)" = "2cc9a97d7cec30128fd8b28a7c1f9df1c001ceb9b441e2b755e24130a6b43c79"
 "checksum keccak 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "67c21572b4949434e4fc1e1978b99c5f77064153c59d998bf13ecd96fb5ecba7"
--- a/src/main.rs
+++ b/src/main.rs
@@ -7,9 +7,9 @@ extern crate log;
 use clap::{App, AppSettings, Arg, ArgMatches};
 use rustpython_compiler::compile;
 use rustpython_vm::{
+    exceptions::{print_exception, PyBaseExceptionRef},
    match_class,
-    obj::{objint::PyInt, objtuple::PyTuple, objtype},
-    print_exception,
+    obj::{objint::PyInt, objtype},
    pyobject::{ItemProtocol, PyResult},
    scope::Scope,
    util, InitParameter, PySettings, VirtualMachine,
@@ -51,8 +51,8 @@ fn main() {
    // See if any exception leaked out:
    if let Err(err) = res {
        if objtype::isinstance(&err, &vm.ctx.exceptions.system_exit) {
-            let args = vm.get_attribute(err.clone(), "args").unwrap();
-            let args = args.downcast::<PyTuple>().expect("'args' must be a tuple");
+            let err: PyBaseExceptionRef = err.downcast().unwrap();
+            let args = err.args();
            match args.elements.len() {
                0 => return,
                1 => match_class!(match args.elements[0].clone() {
--- a/src/shell.rs
+++ b/src/shell.rs
@@ -5,8 +5,8 @@ mod rustyline_helper;
 use rustpython_compiler::{compile, error::CompileError, error::CompileErrorType};
 use rustpython_parser::error::ParseErrorType;
 use rustpython_vm::{
+    exceptions::print_exception,
    obj::objtype,
-    print_exception,
    pyobject::{ItemProtocol, PyObjectRef, PyResult},
    scope::Scope,
    VirtualMachine,
--- a/vm/Cargo.toml
+++ b/vm/Cargo.toml
@@ -49,7 +49,7 @@ chrono = { version = "=0.4.9", features = ["wasmbind"] }
 unicode-xid = "0.2.0"
 lazy_static = "^1.0.1"
 lexical = "4"
-itertools = "^0.8.0"
+itertools = "0.8"
 hex = "0.4.0"
 hexf-parse = "0.1.0"
 indexmap = "1.0.2"
--- a/vm/src/exceptions.rs
+++ b/vm/src/exceptions.rs
@@ -1,8 +1,7 @@
 use crate::function::PyFuncArgs;
-use crate::obj::objiter;
+use crate::obj::objstr::{PyString, PyStringRef};
 use crate::obj::objtraceback::PyTracebackRef;
 use crate::obj::objtuple::{PyTuple, PyTupleRef};
-use crate::obj::objtype;
 use crate::obj::objtype::PyClassRef;
 use crate::pyobject::{
    IdProtocol, PyClassImpl, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue,
@@ -47,7 +46,7 @@ impl PyBaseException {
    #[pyslot(new)]
    fn tp_new(
        cls: PyClassRef,
-        _args: PyFuncArgs,
+        args: PyFuncArgs,
        vm: &VirtualMachine,
    ) -> PyResult<PyBaseExceptionRef> {
        PyBaseException {
@@ -55,7 +54,7 @@ impl PyBaseException {
            cause: RefCell::new(None),
            context: RefCell::new(None),
            suppress_context: Cell::new(false),
-            args: RefCell::new(PyTuple::from(vec![]).into_ref(vm)),
+            args: RefCell::new(PyTuple::from(args.args).into_ref(vm)),
        }
        .into_ref_with_type(vm, cls)
    }
@@ -66,8 +65,8 @@ impl PyBaseException {
        Ok(())
    }

-    #[pyproperty]
-    fn args(&self, _vm: &VirtualMachine) -> PyTupleRef {
+    #[pyproperty(name = "args")]
+    fn get_args(&self, _vm: &VirtualMachine) -> PyTupleRef {
        self.args.borrow().clone()
    }

@@ -127,6 +126,30 @@ impl PyBaseException {
        zelf.traceback.replace(tb);
        Ok(zelf.as_object().clone())
    }
+
+    #[pymethod(name = "__str__")]
+    fn str(&self, vm: &VirtualMachine) -> PyStringRef {
+        let str_args = exception_args_as_string(vm, self.args(), false);
+        match str_args.into_iter().exactly_one() {
+            Err(i) if i.len() == 0 => PyString::from("").into_ref(vm),
+            Ok(s) => s,
+            Err(i) => PyString::from(format!("({})", i.format(", "))).into_ref(vm),
+        }
+    }
+
+    #[pymethod(name = "__repr__")]
+    fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> String {
+        let repr_args = exception_args_as_string(vm, zelf.args(), false);
+        let cls = zelf.class();
+        match repr_args.into_iter().exactly_one() {
+            Ok(one) => format!("{}({},)", cls.name, one),
+            Err(i) => format!("{}({})", cls.name, i.format(", ")),
+        }
+    }
+
+    pub fn args(&self) -> PyTupleRef {
+        self.args.borrow().clone()
+    }
 }

 /// Print exception chain
@@ -207,27 +230,20 @@ pub fn print_exception_inner<W: Write>(
    vm: &VirtualMachine,
    exc: &PyObjectRef,
 ) -> io::Result<()> {
-    if let Ok(tb) = vm.get_attribute(exc.clone(), "__traceback__") {
-        if objtype::isinstance(&tb, &vm.ctx.traceback_type()) {
-            writeln!(output, "Traceback (most recent call last):")?;
-            let mut tb: PyTracebackRef = tb.downcast().expect(" must be a traceback object");
-            loop {
-                print_traceback_entry(&mut output, &tb)?;
-                tb = match &tb.next {
-                    Some(tb) => tb.clone(),
-                    None => break,
-                };
-            }
+    let exc: PyBaseExceptionRef = exc.clone().downcast().unwrap();
+
+    if let Some(tb) = exc.traceback.borrow().clone() {
+        writeln!(output, "Traceback (most recent call last):")?;
+        let mut tb = &Some(tb);
+        while let Some(traceback) = tb {
+            print_traceback_entry(&mut output, traceback)?;
+            tb = &traceback.next;
        }
    } else {
        writeln!(output, "No traceback set on exception")?;
    }

-    let varargs = vm
-        .get_attribute(exc.clone(), "args")
-        .unwrap()
-        .downcast::<PyTuple>()
-        .expect("'args' must be a tuple");
+    let varargs = exc.args();
    let args_repr = exception_args_as_string(vm, varargs, true);

    let exc_name = exc.class().name.clone();
@@ -247,73 +263,30 @@ fn exception_args_as_string(
    vm: &VirtualMachine,
    varargs: PyTupleRef,
    str_single: bool,
-) -> Vec<String> {
+) -> Vec<PyStringRef> {
    match varargs.elements.len() {
        0 => vec![],
        1 => {
            let args0_repr = if str_single {
-                vm.to_pystr(&varargs.elements[0])
-                    .unwrap_or_else(|_| "<element str() failed>".to_string())
+                vm.to_str(&varargs.elements[0])
+                    .unwrap_or_else(|_| PyString::from("<element str() failed>").into_ref(vm))
            } else {
                vm.to_repr(&varargs.elements[0])
-                    .map(|s| s.as_str().to_owned())
-                    .unwrap_or_else(|_| "<element repr() failed>".to_string())
+                    .unwrap_or_else(|_| PyString::from("<element repr() failed>").into_ref(vm))
            };
            vec![args0_repr]
        }
        _ => varargs
            .elements
            .iter()
-            .map(|vararg| match vm.to_repr(vararg) {
-                Ok(arg_repr) => arg_repr.as_str().to_string(),
-                Err(_) => "<element repr() failed>".to_string(),
+            .map(|vararg| {
+                vm.to_repr(vararg)
+                    .unwrap_or_else(|_| PyString::from("<element repr() failed>").into_ref(vm))
            })
            .collect(),
    }
 }

-fn exception_str(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_str = exception_args_as_string(vm, args, false);
-    let joined_str = match args_str.len() {
-        0 => "".to_string(),
-        1 => args_str.into_iter().next().unwrap(),
-        _ => format!("({})", args_str.into_iter().format(", ")),
-    };
-    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, false);
-
-    let exc_name = exc.class().name.clone();
-    let joined_str = match args_repr.len() {
-        0 => format!("{}()", exc_name),
-        1 => format!("{}({},)", exc_name, args_repr[0]),
-        _ => format!("{}({})", exc_name, args_repr.join(", ")),
-    };
-    Ok(vm.new_str(joined_str))
-}
-
 #[derive(Debug)]
 pub struct ExceptionZoo {
    pub arithmetic_error: PyClassRef,
@@ -508,10 +481,7 @@ impl ExceptionZoo {
    }
 }

-fn import_error_init(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    let exc_self = args.args[0].clone();
-
-    vm.set_attr(&exc_self, "args", vm.ctx.new_tuple(args.args[1..].to_vec()))?;
+fn import_error_init(exc_self: PyObjectRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult<()> {
    vm.set_attr(
        &exc_self,
        "name",
@@ -528,12 +498,7 @@ fn import_error_init(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
            .cloned()
            .unwrap_or_else(|| vm.get_none()),
    )?;
-    vm.set_attr(
-        &exc_self,
-        "msg",
-        args.args.get(1).cloned().unwrap_or_else(|| vm.get_none()),
-    )?;
-    Ok(vm.get_none())
+    Ok(())
 }

 fn none_getter(_obj: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef {
@@ -556,11 +521,6 @@ pub fn init(ctx: &PyContext) {

    PyBaseException::extend_class(ctx, &excs.base_exception_type);

-    extend_class!(ctx, &excs.exception_type, {
-        "__str__" => ctx.new_rustfunc(exception_str),
-        "__repr__" => ctx.new_rustfunc(exception_repr),
-    });
-
    extend_class!(ctx, &excs.syntax_error, {
        "msg" => ctx.new_property(make_arg_getter(0)),
        "filename" => ctx.new_property(make_arg_getter(1)),
@@ -570,13 +530,12 @@ pub fn init(ctx: &PyContext) {
    });

    extend_class!(ctx, &excs.import_error, {
-        "__init__" => ctx.new_rustfunc(import_error_init)
+        "__init__" => ctx.new_rustfunc(import_error_init),
+        "msg" => ctx.new_property(make_arg_getter(0)),
    });

    extend_class!(ctx, &excs.stop_iteration, {
-        "value" => ctx.new_rustfunc(|obj: PyObjectRef, vm: &VirtualMachine| {
-            objiter::stop_iter_value(vm, &obj)
-        }),
+        "value" => ctx.new_property(make_arg_getter(0)),
    });

    extend_class!(ctx, &excs.unicode_decode_error, {
--- a/vm/src/lib.rs
+++ b/vm/src/lib.rs
@@ -53,7 +53,7 @@ pub mod cformat;
 mod dictdatatype;
 #[cfg(feature = "rustpython-compiler")]
 pub mod eval;
-mod exceptions;
+pub mod exceptions;
 pub mod format;
 mod frame;
 mod frozen;
--- a/vm/src/obj/objiter.rs
+++ b/vm/src/obj/objiter.rs
@@ -4,8 +4,8 @@

 use std::cell::Cell;

-use super::objtuple::PyTuple;
 use super::objtype::{self, PyClassRef};
+use crate::exceptions::PyBaseExceptionRef;
 use crate::pyobject::{
    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
@@ -74,8 +74,8 @@ pub fn new_stop_iteration(vm: &VirtualMachine) -> PyObjectRef {
 }

 pub fn stop_iter_value(vm: &VirtualMachine, exc: &PyObjectRef) -> PyResult {
-    let args = vm.get_attribute(exc.clone(), "args")?;
-    let args: &PyTuple = args.payload().unwrap();
+    let exc = PyBaseExceptionRef::try_from_object(vm, exc.clone())?;
+    let args = exc.args();
    let val = args
        .elements
        .first()