Merge pull request #1684 from youknowone/refactor-things

youknowone · web-flow · commit c07e9323bcc9 · 2020-01-15T14:13:02.000+09:00
Refactor small things
diff --git a/parser/src/parser.rs b/parser/src/parser.rs
@@ -145,7 +145,7 @@ mod tests {
 
     #[test]
     fn test_parse_empty() {
-        let parse_ast = parse_program(&String::from(""));
+        let parse_ast = parse_program("");
         assert_eq!(parse_ast, Ok(ast::Program { statements: vec![] }))
     }
 
diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs
@@ -951,11 +951,11 @@ pub fn builtin_build_class_(
     let cells = vm.ctx.new_dict();
 
     let scope = function
-        .scope
+        .scope()
         .new_child_scope_with_locals(cells.clone())
         .new_child_scope_with_locals(namespace.clone());
 
-    vm.invoke_python_function_with_scope(&function, vec![].into(), &scope)?;
+    function.invoke_with_scope(vec![].into(), &scope, vm)?;
 
     let class = vm.call_method(
         metaclass.as_object(),
diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs
@@ -369,29 +369,16 @@ impl DictKey for &str {
 }
 
 impl DictKey for &String {
-    fn do_hash(self, _vm: &VirtualMachine) -> PyResult<HashValue> {
-        // follow a similar route as the hashing of PyStringRef
-        let raw_hash = pyhash::hash_value(self).to_bigint().unwrap();
-        let raw_hash = pyhash::hash_bigint(&raw_hash);
-        let mut hasher = DefaultHasher::new();
-        raw_hash.hash(&mut hasher);
-        Ok(hasher.finish() as HashValue)
+    fn do_hash(self, vm: &VirtualMachine) -> PyResult<HashValue> {
+        self.as_str().do_hash(vm)
     }
 
-    fn do_is(self, _other: &PyObjectRef) -> bool {
-        // No matter who the other pyobject is, we are never the same thing, since
-        // we are a str, not a pyobject.
-        false
+    fn do_is(self, other: &PyObjectRef) -> bool {
+        self.as_str().do_is(other)
     }
 
     fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
-        if let Some(py_str_value) = other_key.payload::<PyString>() {
-            Ok(py_str_value.as_str() == self)
-        } else {
-            // Fall back to PyString implementation.
-            let s = vm.new_str(self.to_string());
-            s.do_eq(vm, other_key)
-        }
+        self.as_str().do_eq(vm, other_key)
     }
 }
 
diff --git a/vm/src/obj/objclassmethod.rs b/vm/src/obj/objclassmethod.rs
@@ -29,10 +29,16 @@ use crate::vm::VirtualMachine;
 #[pyclass]
 #[derive(Clone, Debug)]
 pub struct PyClassMethod {
-    pub callable: PyObjectRef,
+    callable: PyObjectRef,
 }
 pub type PyClassMethodRef = PyRef<PyClassMethod>;
 
+impl PyClassMethod {
+    pub fn new(value: PyObjectRef) -> Self {
+        Self { callable: value }
+    }
+}
+
 impl PyValue for PyClassMethod {
     const HAVE_DICT: bool = true;
 
@@ -73,7 +79,7 @@ impl PyClassMethod {
     }
 }
 
-pub fn init(context: &PyContext) {
+pub(crate) fn init(context: &PyContext) {
     PyClassMethod::extend_class(context, &context.types.classmethod_type);
     extend_class!(context, context.types.classmethod_type, {
         "__get__" => context.new_method(PyClassMethod::get),
diff --git a/vm/src/obj/objfunction.rs b/vm/src/obj/objfunction.rs
@@ -3,21 +3,28 @@ use super::objdict::PyDictRef;
 use super::objstr::PyStringRef;
 use super::objtuple::PyTupleRef;
 use super::objtype::PyClassRef;
+use crate::bytecode;
 use crate::descriptor::PyBuiltinDescriptor;
+use crate::frame::Frame;
 use crate::function::{OptionalArg, PyFuncArgs};
-use crate::pyobject::{IdProtocol, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol};
+use crate::obj::objcoroutine::PyCoroutine;
+use crate::obj::objgenerator::PyGenerator;
+use crate::pyobject::{
+    IdProtocol, ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
+    TypeProtocol,
+};
 use crate::scope::Scope;
 use crate::vm::VirtualMachine;
 
 pub type PyFunctionRef = PyRef<PyFunction>;
 
+#[pyclass]
 #[derive(Debug)]
 pub struct PyFunction {
-    // TODO: these shouldn't be public
-    pub code: PyCodeRef,
-    pub scope: Scope,
-    pub defaults: Option<PyTupleRef>,
-    pub kw_only_defaults: Option<PyDictRef>,
+    code: PyCodeRef,
+    scope: Scope,
+    defaults: Option<PyTupleRef>,
+    kw_only_defaults: Option<PyDictRef>,
 }
 
 impl PyBuiltinDescriptor for PyFunction {
@@ -49,6 +56,182 @@ impl PyFunction {
             kw_only_defaults,
         }
     }
+
+    pub fn scope(&self) -> &Scope {
+        &self.scope
+    }
+
+    fn fill_locals_from_args(
+        &self,
+        code_object: &bytecode::CodeObject,
+        locals: &PyDictRef,
+        func_args: PyFuncArgs,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        let nargs = func_args.args.len();
+        let nexpected_args = code_object.arg_names.len();
+
+        // This parses the arguments from args and kwargs into
+        // the proper variables keeping into account default values
+        // and starargs and kwargs.
+        // See also: PyEval_EvalCodeWithName in cpython:
+        // https://github.com/python/cpython/blob/master/Python/ceval.c#L3681
+
+        let n = if nargs > nexpected_args {
+            nexpected_args
+        } else {
+            nargs
+        };
+
+        // Copy positional arguments into local variables
+        for i in 0..n {
+            let arg_name = &code_object.arg_names[i];
+            let arg = &func_args.args[i];
+            locals.set_item(arg_name, arg.clone(), vm)?;
+        }
+
+        // Pack other positional arguments in to *args:
+        match code_object.varargs {
+            bytecode::Varargs::Named(ref vararg_name) => {
+                let mut last_args = vec![];
+                for i in n..nargs {
+                    let arg = &func_args.args[i];
+                    last_args.push(arg.clone());
+                }
+                let vararg_value = vm.ctx.new_tuple(last_args);
+
+                locals.set_item(vararg_name, vararg_value, vm)?;
+            }
+            bytecode::Varargs::Unnamed | bytecode::Varargs::None => {
+                // Check the number of positional arguments
+                if nargs > nexpected_args {
+                    return Err(vm.new_type_error(format!(
+                        "Expected {} arguments (got: {})",
+                        nexpected_args, nargs
+                    )));
+                }
+            }
+        }
+
+        // Do we support `**kwargs` ?
+        let kwargs = match code_object.varkeywords {
+            bytecode::Varargs::Named(ref kwargs_name) => {
+                let d = vm.ctx.new_dict();
+                locals.set_item(kwargs_name, d.as_object().clone(), vm)?;
+                Some(d)
+            }
+            bytecode::Varargs::Unnamed => Some(vm.ctx.new_dict()),
+            bytecode::Varargs::None => None,
+        };
+
+        // Handle keyword arguments
+        for (name, value) in func_args.kwargs {
+            // Check if we have a parameter with this name:
+            if code_object.arg_names.contains(&name) || code_object.kwonlyarg_names.contains(&name)
+            {
+                if locals.contains_key(&name, vm) {
+                    return Err(
+                        vm.new_type_error(format!("Got multiple values for argument '{}'", name))
+                    );
+                }
+
+                locals.set_item(&name, value, vm)?;
+            } else if let Some(d) = &kwargs {
+                d.set_item(&name, value, vm)?;
+            } else {
+                return Err(
+                    vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name))
+                );
+            }
+        }
+
+        // Add missing positional arguments, if we have fewer positional arguments than the
+        // function definition calls for
+        if nargs < nexpected_args {
+            let num_defaults_available = self.defaults.as_ref().map_or(0, |d| d.as_slice().len());
+
+            // Given the number of defaults available, check all the arguments for which we
+            // _don't_ have defaults; if any are missing, raise an exception
+            let required_args = nexpected_args - num_defaults_available;
+            let mut missing = vec![];
+            for i in 0..required_args {
+                let variable_name = &code_object.arg_names[i];
+                if !locals.contains_key(variable_name, vm) {
+                    missing.push(variable_name)
+                }
+            }
+            if !missing.is_empty() {
+                return Err(vm.new_type_error(format!(
+                    "Missing {} required positional arguments: {:?}",
+                    missing.len(),
+                    missing
+                )));
+            }
+            if let Some(defaults) = &self.defaults {
+                let defaults = defaults.as_slice();
+                // We have sufficient defaults, so iterate over the corresponding names and use
+                // the default if we don't already have a value
+                for (default_index, i) in (required_args..nexpected_args).enumerate() {
+                    let arg_name = &code_object.arg_names[i];
+                    if !locals.contains_key(arg_name, vm) {
+                        locals.set_item(arg_name, defaults[default_index].clone(), vm)?;
+                    }
+                }
+            }
+        };
+
+        // Check if kw only arguments are all present:
+        for arg_name in &code_object.kwonlyarg_names {
+            if !locals.contains_key(arg_name, vm) {
+                if let Some(kw_only_defaults) = &self.kw_only_defaults {
+                    if let Some(default) = kw_only_defaults.get_item_option(arg_name, vm)? {
+                        locals.set_item(arg_name, default, vm)?;
+                        continue;
+                    }
+                }
+
+                // No default value and not specified.
+                return Err(
+                    vm.new_type_error(format!("Missing required kw only argument: '{}'", arg_name))
+                );
+            }
+        }
+
+        Ok(())
+    }
+
+    pub fn invoke_with_scope(
+        &self,
+        func_args: PyFuncArgs,
+        scope: &Scope,
+        vm: &VirtualMachine,
+    ) -> PyResult {
+        let code = &self.code;
+
+        let scope = if self.code.flags.contains(bytecode::CodeFlags::NEW_LOCALS) {
+            scope.new_child_scope(&vm.ctx)
+        } else {
+            scope.clone()
+        };
+
+        self.fill_locals_from_args(&code, &scope.get_locals(), func_args, vm)?;
+
+        // Construct frame:
+        let frame = Frame::new(code.clone(), scope).into_ref(vm);
+
+        // If we have a generator, create a new generator
+        if code.flags.contains(bytecode::CodeFlags::IS_GENERATOR) {
+            Ok(PyGenerator::new(frame, vm).into_object())
+        } else if code.flags.contains(bytecode::CodeFlags::IS_COROUTINE) {
+            Ok(PyCoroutine::new(frame, vm).into_object())
+        } else {
+            vm.run_frame_full(frame)
+        }
+    }
+
+    pub fn invoke(&self, func_args: PyFuncArgs, vm: &VirtualMachine) -> PyResult {
+        self.invoke_with_scope(func_args, &self.scope, vm)
+    }
 }
 
 impl PyValue for PyFunction {
@@ -57,20 +240,25 @@ impl PyValue for PyFunction {
     }
 }
 
-impl PyFunctionRef {
-    fn call(func: PyObjectRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult {
-        vm.invoke(&func, args)
+#[pyimpl]
+impl PyFunction {
+    #[pymethod(name = "__call__")]
+    fn call(zelf: PyObjectRef, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult {
+        vm.invoke(&zelf, args)
     }
 
-    fn code(self, _vm: &VirtualMachine) -> PyCodeRef {
+    #[pyproperty(name = "__code__")]
+    fn code(&self, _vm: &VirtualMachine) -> PyCodeRef {
         self.code.clone()
     }
 
-    fn defaults(self, _vm: &VirtualMachine) -> Option<PyTupleRef> {
+    #[pyproperty(name = "__defaults__")]
+    fn defaults(&self, _vm: &VirtualMachine) -> Option<PyTupleRef> {
         self.defaults.clone()
     }
 
-    fn kwdefaults(self, _vm: &VirtualMachine) -> Option<PyDictRef> {
+    #[pyproperty(name = "__kwdefaults__")]
+    fn kwdefaults(&self, _vm: &VirtualMachine) -> Option<PyDictRef> {
         self.kw_only_defaults.clone()
     }
 }
@@ -100,13 +288,10 @@ impl PyValue for PyBoundMethod {
 
 pub fn init(context: &PyContext) {
     let function_type = &context.types.function_type;
+    PyFunction::extend_class(context, function_type);
     extend_class!(context, function_type, {
         "__get__" => context.new_method(PyFunction::get),
         (slot descr_get) => PyFunction::get,
-        "__call__" => context.new_method(PyFunctionRef::call),
-        "__code__" => context.new_property(PyFunctionRef::code),
-        "__defaults__" => context.new_property(PyFunctionRef::defaults),
-        "__kwdefaults__" => context.new_property(PyFunctionRef::kwdefaults),
     });
 
     let method_type = &context.types.bound_method_type;
diff --git a/vm/src/obj/objstaticmethod.rs b/vm/src/obj/objstaticmethod.rs
@@ -29,7 +29,7 @@ impl PyBuiltinDescriptor for PyStaticMethod {
 }
 
 #[pyimpl]
-impl PyStaticMethodRef {
+impl PyStaticMethod {
     #[pyslot]
     fn tp_new(
         cls: PyClassRef,
@@ -44,7 +44,7 @@ impl PyStaticMethodRef {
 }
 
 pub fn init(context: &PyContext) {
-    PyStaticMethodRef::extend_class(context, &context.types.staticmethod_type);
+    PyStaticMethod::extend_class(context, &context.types.staticmethod_type);
     extend_class!(context, context.types.staticmethod_type, {
         "__get__" => context.new_method(PyStaticMethod::get),
         (slot descr_get) => PyStaticMethod::get,
diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs
diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs
diff --git a/vm/src/vm.rs b/vm/src/vm.rs

Original file line number	Diff line number	Diff line change
`@@ -145,7 +145,7 @@ mod tests {`
`145`	`145`
`146`	`146`	`#[test]`
`147`	`147`	`fn test_parse_empty() {`
`148`		`- let parse_ast = parse_program(&String::from(""));`
	`148`	`+ let parse_ast = parse_program("");`
`149`	`149`	`assert_eq!(parse_ast, Ok(ast::Program { statements: vec![] }))`
`150`	`150`	`}`
`151`	`151`