Removal of ToRust trait to prevent incorrect str representation

windelbouwman · windelbouwman · commit b7a0b0f2f30f · 2018-11-02T20:27:05.000+01:00
diff --git a/parser/src/ast.rs b/parser/src/ast.rs
@@ -1,6 +1,6 @@
-/*
- * Implement abstract syntax tree nodes for the python language.
- */
+//! Implement abstract syntax tree nodes for the python language.
+//!
+//! Roughly equivalent to this: https://docs.python.org/3/library/ast.html
 
 pub use super::lexer::Location;
 /*
@@ -40,6 +40,7 @@ pub struct Located<T> {
 
 pub type LocatedStatement = Located<Statement>;
 
+/// Abstract syntax tree nodes for python statements.
 #[derive(Debug, PartialEq)]
 pub enum Statement {
     Break,
diff --git a/parser/src/lexer.rs b/parser/src/lexer.rs
@@ -1,3 +1,6 @@
+//! This module takes care of lexing python source text. This means source
+//! code is translated into seperate tokens.
+
 pub use super::token::Tok;
 use std::collections::HashMap;
 
diff --git a/parser/src/parser.rs b/parser/src/parser.rs
@@ -69,6 +69,25 @@ pub fn parse_statement(source: &str) -> Result<ast::LocatedStatement, String> {
     do_lalr_parsing!(source, Statement, StartStatement)
 }
 
+/// Parses a python expression
+///
+/// # Example
+/// ```
+/// use rustpython_parser::{parser, ast};
+/// let expr = parser::parse_expression("1+2").unwrap();
+///
+/// assert_eq!(ast::Expression::Binop {
+///         a: Box::new(ast::Expression::Number {
+///             value: ast::Number::Integer { value: 1 }
+///         }),
+///         op: ast::Operator::Add,
+///         b: Box::new(ast::Expression::Number {
+///             value: ast::Number::Integer { value: 2 }
+///         })
+///     },
+///     expr);
+///
+/// ```
 pub fn parse_expression(source: &str) -> Result<ast::Expression, String> {
     do_lalr_parsing!(source, Expression, StartExpression)
 }
diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs
@@ -1,3 +1,7 @@
+//! Builtin function definitions.
+//!
+//! Implements functions listed here: https://docs.python.org/3/library/builtins.html
+
 // use std::ops::Deref;
 use std::char;
 use std::collections::HashMap;
@@ -463,10 +467,8 @@ fn builtin_pow(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 pub fn builtin_print(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     trace!("print called with {:?}", args);
     for a in args.args {
-        let s = match vm.to_str(a) {
-            Ok(v) => objstr::get_value(&v),
-            Err(err) => return Err(err),
-        };
+        let v = vm.to_str(&a)?;
+        let s = objstr::get_value(&v);
         print!("{}", s);
     }
     println!();
diff --git a/vm/src/compile.rs b/vm/src/compile.rs
@@ -1,10 +1,10 @@
-/*
- * Take an AST and transform it into bytecode
- *
- * Inspirational code:
- *   https://github.com/python/cpython/blob/master/Python/compile.c
- *   https://github.com/micropython/micropython/blob/master/py/compile.c
- */
+//!
+//!
+//! Take an AST and transform it into bytecode
+//!
+//! Inspirational code:
+//!   https://github.com/python/cpython/blob/master/Python/compile.c
+//!   https://github.com/micropython/micropython/blob/master/py/compile.c
 
 use super::bytecode::{self, CallType, CodeObject, Instruction};
 use super::pyobject::{PyObject, PyObjectKind, PyResult};
@@ -19,6 +19,7 @@ struct Compiler {
     current_source_location: ast::Location,
 }
 
+/// Compile a given sourcecode into a bytecode object.
 pub fn compile(
     vm: &mut VirtualMachine,
     source: &str,
diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs
@@ -30,19 +30,19 @@ pub fn print_exception(vm: &mut VirtualMachine, exc: &PyObjectRef) {
             for element in elements.iter() {
                 if objtype::isinstance(&element, &vm.ctx.tuple_type()) {
                     let element = objtuple::get_elements(&element);
-                    let filename = if let Ok(x) = vm.to_str(element[0].clone()) {
+                    let filename = if let Ok(x) = vm.to_str(&element[0]) {
                         objstr::get_value(&x)
                     } else {
                         "<error>".to_string()
                     };
 
-                    let lineno = if let Ok(x) = vm.to_str(element[1].clone()) {
+                    let lineno = if let Ok(x) = vm.to_str(&element[1]) {
                         objstr::get_value(&x)
                     } else {
                         "<error>".to_string()
                     };
 
-                    let obj_name = if let Ok(x) = vm.to_str(element[2].clone()) {
+                    let obj_name = if let Ok(x) = vm.to_str(&element[2]) {
                         objstr::get_value(&x)
                     } else {
                         "<error>".to_string()
@@ -58,7 +58,7 @@ pub fn print_exception(vm: &mut VirtualMachine, exc: &PyObjectRef) {
         println!("No traceback set on exception");
     }
 
-    match vm.to_str(exc.clone()) {
+    match vm.to_str(exc) {
         Ok(txt) => println!("{}", objstr::get_value(&txt)),
         Err(err) => println!("Error during error {:?}", err),
     }
diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -16,7 +16,7 @@ use super::obj::objstr;
 use super::obj::objtype;
 use super::pyobject::{
     AttributeProtocol, DictProtocol, IdProtocol, ParentProtocol, PyFuncArgs, PyObject,
-    PyObjectKind, PyObjectRef, PyResult, ToRust, TypeProtocol,
+    PyObjectKind, PyObjectRef, PyResult, TypeProtocol,
 };
 use super::vm::VirtualMachine;
 use num_bigint::ToBigInt;
@@ -461,9 +461,8 @@ impl Frame {
                         let kwarg_names = self.pop_value();
                         let args: Vec<PyObjectRef> = self.pop_multiple(*count);
 
-                        let kwarg_names = kwarg_names
-                            .to_vec()
-                            .unwrap()
+                        let kwarg_names = vm
+                            .extract_elements(&kwarg_names)?
                             .iter()
                             .map(|pyobj| objstr::get_value(pyobj))
                             .collect();
diff --git a/vm/src/lib.rs b/vm/src/lib.rs
@@ -1,3 +1,10 @@
+//! This crate contains most python logic.
+//!
+//! - Compilation
+//! - Bytecode
+//! - Import mechanics
+//! - Base objects
+
 #[macro_use]
 extern crate bitflags;
 #[macro_use]
diff --git a/vm/src/macros.rs b/vm/src/macros.rs
@@ -64,11 +64,12 @@ macro_rules! arg_check {
             match expected_type {
                 Some(expected_type) => {
                     if !objtype::isinstance(arg, &expected_type) {
-                        let arg_typ = arg.typ().clone();
-                        let actual_type = arg_typ.borrow().str().clone();
+                        let arg_typ = arg.typ();
+                        let expected_type_name = $vm.to_pystr(expected_type)?;
+                        let actual_type = $vm.to_pystr(&arg_typ)?;
                         return Err($vm.new_type_error(format!(
                             "argument of type {} is required for parameter {} ({}) (got: {})",
-                            expected_type.borrow().str(),
+                            expected_type_name,
                             arg_position + 1,
                             arg_name,
                             actual_type
diff --git a/vm/src/obj/mod.rs b/vm/src/obj/mod.rs
@@ -1,3 +1,5 @@
+//! This package contains the python basic/builtin types
+
 pub mod objbool;
 pub mod objbytes;
 pub mod objcomplex;
diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs
@@ -246,7 +246,7 @@ fn str_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         panic!("str expects exactly one parameter");
     };
 
-    vm.to_str(args.args[1].clone())
+    vm.to_str(&args.args[1])
 }
 
 impl PySliceableSequence for String {
diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs
@@ -62,10 +62,8 @@ fn tuple_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 
     let mut str_parts = vec![];
     for elem in elements.iter() {
-        match vm.to_repr(elem) {
-            Ok(s) => str_parts.push(objstr::get_value(&s)),
-            Err(err) => return Err(err),
-        }
+        let s = vm.to_repr(elem)?;
+        str_parts.push(objstr::get_value(&s));
     }
 
     let s = if str_parts.len() == 1 {
diff --git a/vm/src/obj/objtype.rs b/vm/src/obj/objtype.rs
@@ -1,6 +1,6 @@
 use super::super::pyobject::{
     AttributeProtocol, IdProtocol, PyContext, PyFuncArgs, PyObject, PyObjectKind, PyObjectRef,
-    PyResult, ToRust, TypeProtocol,
+    PyResult, TypeProtocol,
 };
 use super::super::vm::VirtualMachine;
 use super::objdict;
@@ -99,16 +99,14 @@ pub fn type_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
             required = [
                 (typ, Some(vm.ctx.type_type())),
                 (name, Some(vm.ctx.str_type())),
-                // bases needs to be mutable, which arg_check! doesn't support, so we just check
-                // the type and extract it again below
-                // TODO: arg_check! should support specifying iterables
-                (_bases, None),
+                (bases, None),
                 (dict, Some(vm.ctx.dict_type()))
             ]
         );
-        let mut bases = args.args[2].to_vec().unwrap();
+        let mut bases = vm.extract_elements(bases)?;
         bases.push(vm.context().object());
-        new(typ.clone(), &name.to_str().unwrap(), bases, dict.clone())
+        let name = objstr::get_value(name);
+        new(typ.clone(), &name, bases, dict.clone())
     } else {
         Err(vm.new_type_error(format!(": type_new: {:?}", args)))
     }
@@ -124,15 +122,11 @@ pub fn type_call(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> PyResult {
     };
 
     if let Some(init) = obj.typ().get_attr("__init__") {
-        match vm.invoke(init, args.insert(obj.clone())) {
-            Ok(res) => {
-                // TODO: assert that return is none?
-                if !isinstance(&res, &vm.get_none()) {
-                    // panic!("__init__ must return none");
-                    // return Err(vm.new_type_error("__init__ must return None".to_string()));
-                }
-            }
-            Err(err) => return Err(err),
+        let res = vm.invoke(init, args.insert(obj.clone()))?;
+        // TODO: assert that return is none?
+        if !isinstance(&res, &vm.get_none()) {
+            // panic!("__init__ must return none");
+            // return Err(vm.new_type_error("__init__ must return None".to_string()));
         }
     }
     Ok(obj)
diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs
@@ -579,25 +579,6 @@ impl DictProtocol for PyObjectRef {
     }
 }
 
-pub trait ToRust {
-    fn to_vec(&self) -> Option<Vec<PyObjectRef>>;
-    fn to_str(&self) -> Option<String>;
-}
-
-impl ToRust for PyObjectRef {
-    fn to_vec(&self) -> Option<Vec<PyObjectRef>> {
-        match self.borrow().kind {
-            PyObjectKind::Tuple { ref elements } => Some(elements.clone()),
-            PyObjectKind::List { ref elements } => Some(elements.clone()),
-            _ => None,
-        }
-    }
-
-    fn to_str(&self) -> Option<String> {
-        Some(self.borrow().str())
-    }
-}
-
 impl fmt::Debug for PyObject {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "[PyObj {:?}]", self.kind)
@@ -772,6 +753,7 @@ impl PyObject {
         .into_ref()
     }
 
+    /// Deprecated method, please call `vm.to_pystr`
     pub fn str(&self) -> String {
         match self.kind {
             PyObjectKind::String { ref value } => value.clone(),
diff --git a/vm/src/vm.rs b/vm/src/vm.rs
@@ -1,8 +1,8 @@
-/*
- * Implement virtual machine to run instructions.
- * See also:
- *   https://github.com/ProgVal/pythonvm-rust/blob/master/src/processor/mod.rs
- */
+//! Implement virtual machine to run instructions.
+//!
+//! See also:
+//!   https://github.com/ProgVal/pythonvm-rust/blob/master/src/processor/mod.rs
+//!
 
 extern crate rustpython_parser;
 
@@ -14,6 +14,7 @@ use super::frame::{copy_code, Frame};
 use super::obj::objgenerator;
 use super::obj::objiter;
 use super::obj::objlist;
+use super::obj::objstr;
 use super::obj::objtuple;
 use super::obj::objtype;
 use super::pyobject::{
@@ -40,6 +41,7 @@ impl VirtualMachine {
         frame.run_frame_full(self)
     }
 
+    /// Create a new python string object.
     pub fn new_str(&self, s: String) -> PyObjectRef {
         self.ctx.new_str(s)
     }
@@ -141,10 +143,15 @@ impl VirtualMachine {
     }
 
     // Container of the virtual machine state:
-    pub fn to_str(&mut self, obj: PyObjectRef) -> PyResult {
+    pub fn to_str(&mut self, obj: &PyObjectRef) -> PyResult {
         self.call_method(&obj, "__str__", vec![])
     }
 
+    pub fn to_pystr(&mut self, obj: &PyObjectRef) -> Result<String, PyObjectRef> {
+        let py_str_obj = self.to_str(obj)?;
+        Ok(objstr::get_value(&py_str_obj))
+    }
+
     pub fn to_repr(&mut self, obj: &PyObjectRef) -> PyResult {
         self.call_method(obj, "__repr__", vec![])
     }

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+//! This package contains the python basic/builtin types`
	`2`	`+`
`1`	`3`	`pub mod objbool;`
`2`	`4`	`pub mod objbytes;`
`3`	`5`	`pub mod objcomplex;`