RustPython · cthulahoops · Mar 25, 2019 · Mar 25, 2019 · Mar 25, 2019 · Mar 25, 2019
diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs
@@ -11,7 +11,6 @@ use num_traits::{Signed, ToPrimitive};
 use crate::compile;
 use crate::import::import_module;
 use crate::obj::objbool;
-use crate::obj::objdict;
 use crate::obj::objint;
 use crate::obj::objiter;
 use crate::obj::objstr::{self, PyStringRef};
@@ -29,14 +28,7 @@ use crate::obj::objcode::PyCodeRef;
 use crate::stdlib::io::io_open;
 
 fn get_locals(vm: &VirtualMachine) -> PyObjectRef {
-    let d = vm.new_dict();
-    // TODO: implement dict_iter_items?
-    let locals = vm.get_locals();
-    let key_value_pairs = objdict::get_key_value_pairs(&locals);
-    for (key, value) in key_value_pairs {
-        objdict::set_item(&d, vm, &key, &value);
-    }
-    d
+    vm.get_locals()
 }
 
 fn dir_locals(vm: &VirtualMachine) -> PyObjectRef {
@@ -823,9 +815,9 @@ pub fn builtin_build_class_(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResu
     let prepare = vm.get_attribute(metaclass.clone().into_object(), "__prepare__")?;
     let namespace = vm.invoke(prepare, vec![name_arg.clone(), bases.clone()])?;
 
-    let cells = vm.new_dict();
+    let cells = vm.ctx.new_dict();
 
-    vm.invoke_with_locals(function, cells.clone(), namespace.clone())?;
+    vm.invoke_with_locals(function, cells.clone().into_object(), namespace.clone())?;
     let class = vm.call_method(
         metaclass.as_object(),
         "__call__",

diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -119,7 +119,7 @@ impl Scope {
     }
 
     pub fn child_scope(&self, ctx: &PyContext) -> Scope {
-        self.child_scope_with_locals(ctx.new_dict())
+        self.child_scope_with_locals(ctx.new_dict().into_object())
     }
 }
 
@@ -142,7 +142,7 @@ impl NameProtocol for Scope {
             return Some(value);
         }
 
-        vm.builtins.get_item(name)
+        vm.get_attribute(vm.builtins.clone(), name).ok()
     }
 
     fn load_cell(&self, _vm: &VirtualMachine, name: &str) -> Option<PyObjectRef> {
@@ -386,7 +386,7 @@ impl Frame {
                 Ok(None)
             }
             bytecode::Instruction::BuildMap { size, unpack } => {
-                let map_obj = vm.ctx.new_dict();
+                let map_obj = vm.ctx.new_dict().into_object();
                 for _x in 0..*size {
                     let obj = self.pop_value();
                     if *unpack {
@@ -572,7 +572,7 @@ impl Frame {
                 let annotations = if flags.contains(bytecode::FunctionOpArg::HAS_ANNOTATIONS) {
                     self.pop_value()
                 } else {
-                    vm.new_dict()
+                    vm.ctx.new_dict().into_object()
                 };
 
                 let defaults = if flags.contains(bytecode::FunctionOpArg::HAS_DEFAULTS) {
@@ -839,8 +839,10 @@ impl Frame {
         let module = vm.import(module)?;
 
         // Grab all the names from the module and put them in the context
-        for (k, v) in module.get_key_value_pairs().iter() {
-            self.scope.store_name(&vm, &objstr::get_value(k), v.clone());
+        if let Some(dict) = &module.dict {
+            for (k, v) in dict.get_key_value_pairs().iter() {
+                self.scope.store_name(&vm, &objstr::get_value(k), v.clone());
+            }
         }
         Ok(None)
     }

diff --git a/vm/src/function.rs b/vm/src/function.rs
@@ -247,6 +247,15 @@ where
     }
 }
 
+impl<T> IntoIterator for KwArgs<T> {
+    type Item = (String, T);
+    type IntoIter = std::collections::hash_map::IntoIter<String, T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
+
 /// A list of positional argument values.
 ///
 /// A built-in function with a `Args` parameter is analagous to a Python

diff --git a/vm/src/import.rs b/vm/src/import.rs
@@ -40,7 +40,7 @@ fn import_uncached_module(vm: &VirtualMachine, current_path: PathBuf, module: &s
 
     let attrs = vm.ctx.new_dict();
     attrs.set_item(&vm.ctx, "__name__", vm.new_str(module.to_string()));
-    vm.run_code_obj(code_obj, Scope::new(None, attrs.clone()))?;
+    vm.run_code_obj(code_obj, Scope::new(None, attrs.clone().into_object()))?;
     Ok(vm.ctx.new_module(module, attrs))
 }
 

diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs
@@ -3,9 +3,9 @@ use std::collections::HashMap;
 use std::fmt;
 use std::ops::{Deref, DerefMut};
 
-use crate::function::{OptionalArg, PyFuncArgs};
+use crate::function::{KwArgs, OptionalArg};
 use crate::pyobject::{
-    PyAttributes, PyContext, PyIteratorValue, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    DictProtocol, PyAttributes, PyContext, PyIteratorValue, PyObjectRef, PyRef, PyResult, PyValue,
 };
 use crate::vm::{ReprGuard, VirtualMachine};
 
@@ -124,57 +124,48 @@ pub fn py_dict_to_attributes(dict: &PyObjectRef) -> PyAttributes {
     attrs
 }
 
-pub fn attributes_to_py_dict(vm: &VirtualMachine, attributes: PyAttributes) -> PyResult {
-    let dict = vm.ctx.new_dict();
-    for (key, value) in attributes {
-        let key = vm.ctx.new_str(key);
-        set_item(&dict, vm, &key, &value);
-    }
-    Ok(dict)
-}
-
 // Python dict methods:
-fn dict_new(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [(_ty, Some(vm.ctx.type_type()))],
-        optional = [(dict_obj, None)]
-    );
-    let dict = vm.ctx.new_dict();
-    if let Some(dict_obj) = dict_obj {
-        if objtype::isinstance(&dict_obj, &vm.ctx.dict_type()) {
-            for (needle, value) in get_key_value_pairs(&dict_obj) {
-                set_item(&dict, vm, &needle, &value);
-            }
-        } else {
-            let iter = objiter::get_iter(vm, dict_obj)?;
-            loop {
-                fn err(vm: &VirtualMachine) -> PyObjectRef {
-                    vm.new_type_error("Iterator must have exactly two elements".to_string())
+impl PyDictRef {
+    fn new(
+        _class: PyClassRef, // TODO Support subclasses of int.
+        dict_obj: OptionalArg<PyObjectRef>,
+        kwargs: KwArgs,
+        vm: &VirtualMachine,
+    ) -> PyResult<PyDictRef> {
+        let dict = vm.ctx.new_dict();
+        if let OptionalArg::Present(dict_obj) = dict_obj {
+            if objtype::isinstance(&dict_obj, &vm.ctx.dict_type()) {
+                for (needle, value) in get_key_value_pairs(&dict_obj) {
+                    set_item(dict.as_object(), vm, &needle, &value);
                 }
-                let element = match objiter::get_next_object(vm, &iter)? {
-                    Some(obj) => obj,
-                    None => break,
-                };
-                let elem_iter = objiter::get_iter(vm, &element)?;
-                let needle = objiter::get_next_object(vm, &elem_iter)?.ok_or_else(|| err(vm))?;
-                let value = objiter::get_next_object(vm, &elem_iter)?.ok_or_else(|| err(vm))?;
-                if objiter::get_next_object(vm, &elem_iter)?.is_some() {
-                    return Err(err(vm));
+            } else {
+                let iter = objiter::get_iter(vm, &dict_obj)?;
+                loop {
+                    fn err(vm: &VirtualMachine) -> PyObjectRef {
+                        vm.new_type_error("Iterator must have exactly two elements".to_string())
+                    }
+                    let element = match objiter::get_next_object(vm, &iter)? {
+                        Some(obj) => obj,
+                        None => break,
+                    };
+                    let elem_iter = objiter::get_iter(vm, &element)?;
+                    let needle =
+                        objiter::get_next_object(vm, &elem_iter)?.ok_or_else(|| err(vm))?;
+                    let value = objiter::get_next_object(vm, &elem_iter)?.ok_or_else(|| err(vm))?;
+                    if objiter::get_next_object(vm, &elem_iter)?.is_some() {
+                        return Err(err(vm));
+                    }
+                    set_item(dict.as_object(), vm, &needle, &value);
                 }
-                set_item(&dict, vm, &needle, &value);
             }
         }
+        for (needle, value) in kwargs.into_iter() {
+            let py_needle = vm.new_str(needle);
+            set_item(&dict.as_object(), vm, &py_needle, &value);
+        }
+        Ok(dict)
     }
-    for (needle, value) in args.kwargs {
-        let py_needle = vm.new_str(needle);
-        set_item(&dict, vm, &py_needle, &value);
-    }
-    Ok(dict)
-}
 
-impl PyDictRef {
     fn bool(self, _vm: &VirtualMachine) -> bool {
         !self.entries.borrow().is_empty()
     }
@@ -302,6 +293,31 @@ impl PyDictRef {
     }
 }
 
+impl DictProtocol for PyDictRef {
+    fn contains_key(&self, k: &str) -> bool {
+        content_contains_key_str(&self.entries.borrow(), k)
+    }
+
+    fn get_item(&self, k: &str) -> Option<PyObjectRef> {
+        content_get_key_str(&self.entries.borrow(), k)
+    }
+
+    fn get_key_value_pairs(&self) -> Vec<(PyObjectRef, PyObjectRef)> {
+        get_key_value_pairs(self.as_object())
+    }
+
+    // Item set/get:
+    fn set_item(&self, ctx: &PyContext, key: &str, v: PyObjectRef) {
+        let key = ctx.new_str(key.to_string());
+        set_item_in_content(&mut self.entries.borrow_mut(), &key, &v);
+    }
+
+    fn del_item(&self, key: &str) {
+        let mut elements = get_mut_elements(self.as_object());
+        elements.remove(key).unwrap();
+    }
+}
+
 pub fn init(context: &PyContext) {
     extend_class!(context, &context.dict_type, {
         "__bool__" => context.new_rustfunc(PyDictRef::bool),
@@ -310,7 +326,7 @@ pub fn init(context: &PyContext) {
         "__delitem__" => context.new_rustfunc(PyDictRef::delitem),
         "__getitem__" => context.new_rustfunc(PyDictRef::getitem),
         "__iter__" => context.new_rustfunc(PyDictRef::iter),
-        "__new__" => context.new_rustfunc(dict_new),
+        "__new__" => context.new_rustfunc(PyDictRef::new),
         "__repr__" => context.new_rustfunc(PyDictRef::repr),
         "__setitem__" => context.new_rustfunc(PyDictRef::setitem),
         "clear" => context.new_rustfunc(PyDictRef::clear),

diff --git a/vm/src/obj/objmodule.rs b/vm/src/obj/objmodule.rs
@@ -1,12 +1,10 @@
-use crate::obj::objstr::PyStringRef;
 use crate::obj::objtype::PyClassRef;
-use crate::pyobject::{DictProtocol, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{DictProtocol, PyContext, PyRef, PyResult, PyValue};
 use crate::vm::VirtualMachine;
 
 #[derive(Debug)]
 pub struct PyModule {
     pub name: String,
-    pub dict: PyObjectRef,
 }
 pub type PyModuleRef = PyRef<PyModule>;
 
@@ -18,23 +16,21 @@ impl PyValue for PyModule {
 
 impl PyModuleRef {
     fn dir(self: PyModuleRef, vm: &VirtualMachine) -> PyResult {
-        let keys = self
-            .dict
-            .get_key_value_pairs()
-            .iter()
-            .map(|(k, _v)| k.clone())
-            .collect();
-        Ok(vm.ctx.new_list(keys))
-    }
-
-    fn set_attr(self, attr: PyStringRef, value: PyObjectRef, vm: &VirtualMachine) {
-        self.dict.set_item(&vm.ctx, &attr.value, value)
+        if let Some(dict) = &self.into_object().dict {
+            let keys = dict
+                .get_key_value_pairs()
+                .iter()
+                .map(|(k, _v)| k.clone())
+                .collect();
+            Ok(vm.ctx.new_list(keys))
+        } else {
+            panic!("Modules should definitely have a dict.");
+        }
     }
 }
 
 pub fn init(context: &PyContext) {
     extend_class!(&context, &context.module_type, {
         "__dir__" => context.new_rustfunc(PyModuleRef::dir),
-        "__setattr__" => context.new_rustfunc(PyModuleRef::set_attr)
     });
 }
diff --git a/vm/src/obj/objobject.rs b/vm/src/obj/objobject.rs
@@ -1,5 +1,5 @@
+use super::objdict::{self, PyDictRef};
 use super::objlist::PyList;
-use super::objmodule::PyModule;
 use super::objstr::{self, PyStringRef};
 use super::objtype;
 use crate::function::PyFuncArgs;
@@ -23,11 +23,12 @@ impl PyValue for PyInstance {
 pub fn new_instance(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult {
     // more or less __new__ operator
     let cls = PyClassRef::try_from_object(vm, args.shift())?;
-    Ok(if cls.is(&vm.ctx.object) {
-        PyObject::new_without_dict(PyInstance, cls)
+    let dict = if cls.is(&vm.ctx.object) {
+        None
     } else {
-        PyObject::new(PyInstance, cls)
-    })
+        Some(vm.ctx.new_dict())
+    };
+    Ok(PyObject::new(PyInstance, cls, dict))
 }
 
 fn object_eq(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -114,7 +115,7 @@ fn object_setattr(
     }
 
     if let Some(ref dict) = obj.clone().dict {
-        dict.borrow_mut().insert(attr_name.value.clone(), value);
+        dict.set_item(&vm.ctx, &attr_name.value, value);
         Ok(())
     } else {
         let type_name = objtype::get_type_name(obj.type_ref());
@@ -135,7 +136,7 @@ fn object_delattr(obj: PyObjectRef, attr_name: PyStringRef, vm: &VirtualMachine)
     }
 
     if let Some(ref dict) = obj.dict {
-        dict.borrow_mut().remove(&attr_name.value);
+        dict.del_item(&attr_name.value);
         Ok(())
     } else {
         let type_name = objtype::get_type_name(obj.type_ref());
@@ -227,13 +228,9 @@ fn object_class_setter(
     Err(vm.new_type_error(format!("can't change class of type '{}'", type_repr)))
 }
 
-fn object_dict(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    if let Some(ref dict) = args.args[0].dict {
-        let new_dict = vm.new_dict();
-        for (attr, value) in dict.borrow().iter() {
-            new_dict.set_item(&vm.ctx, &attr, value.clone());
-        }
-        Ok(new_dict)
+fn object_dict(object: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyDictRef> {
+    if let Some(ref dict) = object.dict {
+        Ok(dict.clone())
     } else {
         Err(vm.new_type_error("TypeError: no dictionary.".to_string()))
     }
@@ -273,15 +270,8 @@ fn object_getattribute(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
 }
 
 fn object_getattr(obj: &PyObjectRef, attr_name: &str) -> Option<PyObjectRef> {
-    // TODO:
-    // This is an all kinds of wrong work-around for the temporary difference in
-    // shape between modules and object. It will disappear once that is fixed.
-    if let Some(PyModule { ref dict, .. }) = obj.payload::<PyModule>() {
-        return dict.get_item(attr_name);
-    }
-
     if let Some(ref dict) = obj.dict {
-        dict.borrow().get(attr_name).cloned()
+        dict.get_item(attr_name)
     } else {
         None
     }
@@ -293,8 +283,8 @@ pub fn get_attributes(obj: &PyObjectRef) -> PyAttributes {
 
     // Get instance attributes:
     if let Some(dict) = &obj.dict {
-        for (name, value) in dict.borrow().iter() {
-            attributes.insert(name.to_string(), value.clone());
+        for (key, value) in objdict::get_key_value_pairs(dict.as_object()) {
+            attributes.insert(key.to_string(), value.clone());
         }
     }