Merge pull request #710 from RustPython/class_get_attr_2

Class get attr 2

vm/src/builtins.rs

@@ -19,9 +19,7 @@ use crate::obj::objtype;
 
 use crate::frame::Scope;
 use crate::function::{Args, OptionalArg, PyFuncArgs};
-use crate::pyobject::{
-    AttributeProtocol, DictProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TypeProtocol,
-};
+use crate::pyobject::{DictProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TypeProtocol};
 use crate::vm::VirtualMachine;
 
 #[cfg(not(target_arch = "wasm32"))]
@@ -95,7 +93,7 @@ fn builtin_bin(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
 
 fn builtin_callable(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(vm, args, required = [(obj, None)]);
-    let is_callable = obj.typ().has_attr("__call__");
+    let is_callable = objtype::class_has_attr(&obj.type_pyref(), "__call__");
     Ok(vm.new_bool(is_callable))
 }
 

vm/src/function.rs

@@ -35,6 +35,17 @@ impl From<PyObjectRef> for PyFuncArgs {
     }
 }
 
+impl From<(&Args, &KwArgs)> for PyFuncArgs {
+    fn from(arg: (&Args, &KwArgs)) -> Self {
+        let Args(args) = arg.0;
+        let KwArgs(kwargs) = arg.1;
+        PyFuncArgs {
+            args: args.clone(),
+            kwargs: kwargs.iter().map(|(k, v)| (k.clone(), v.clone())).collect(),
+        }
+    }
+}
+
 impl PyFuncArgs {
     pub fn new(mut args: Vec<PyObjectRef>, kwarg_names: Vec<String>) -> PyFuncArgs {
         let mut kwargs = vec![];

vm/src/obj/objnone.rs

@@ -1,9 +1,9 @@
 use crate::function::PyFuncArgs;
 use crate::obj::objproperty::PyPropertyRef;
 use crate::obj::objstr::PyStringRef;
+use crate::obj::objtype::{class_get_attr, class_has_attr};
 use crate::pyobject::{
-    AttributeProtocol, IntoPyObject, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
-    TryFromObject, TypeProtocol,
+    IntoPyObject, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -45,7 +45,7 @@ impl PyNoneRef {
 
     fn get_attribute(self, name: PyStringRef, vm: &VirtualMachine) -> PyResult {
         trace!("None.__getattribute__({:?}, {:?})", self, name);
-        let cls = self.typ().into_object();
+        let cls = self.typ();
 
         // Properties use a comparision with None to determine if they are either invoked by am
         // instance binding or a class binding. But if the object itself is None then this detection
@@ -69,25 +69,33 @@ impl PyNoneRef {
             }
         }
 
-        if let Some(attr) = cls.get_attr(&name.value) {
-            let attr_class = attr.typ();
-            if attr_class.has_attr("__set__") {
-                if let Some(get_func) = attr_class.get_attr("__get__") {
-                    return call_descriptor(attr, get_func, self.into_object(), cls.clone(), vm);
+        if let Some(attr) = class_get_attr(&cls, &name.value) {
+            let attr_class = attr.type_pyref();
+            if class_has_attr(&attr_class, "__set__") {
+                if let Some(get_func) = class_get_attr(&attr_class, "__get__") {
+                    return call_descriptor(
+                        attr,
+                        get_func,
+                        self.into_object(),
+                        cls.into_object(),
+                        vm,
+                    );
                 }
             }
         }
 
-        if let Some(obj_attr) = self.as_object().get_attr(&name.value) {
-            Ok(obj_attr)
-        } else if let Some(attr) = cls.get_attr(&name.value) {
-            let attr_class = attr.typ();
-            if let Some(get_func) = attr_class.get_attr("__get__") {
-                call_descriptor(attr, get_func, self.into_object(), cls.clone(), vm)
+        // None has no attributes and cannot have attributes set on it.
+        // if let Some(obj_attr) = self.as_object().get_attr(&name.value) {
+        //     Ok(obj_attr)
+        // } else
+        if let Some(attr) = class_get_attr(&cls, &name.value) {
+            let attr_class = attr.type_pyref();
+            if let Some(get_func) = class_get_attr(&attr_class, "__get__") {
+                call_descriptor(attr, get_func, self.into_object(), cls.into_object(), vm)
             } else {
                 Ok(attr)
             }
-        } else if let Some(getter) = cls.get_attr("__getattr__") {
+        } else if let Some(getter) = class_get_attr(&cls, "__getattr__") {
             vm.invoke(getter, vec![self.into_object(), name.into_object()])
         } else {
             Err(vm.new_attribute_error(format!("{} has no attribute '{}'", self.as_object(), name)))

vm/src/obj/objobject.rs

@@ -1,11 +1,12 @@
 use super::objlist::PyList;
+use super::objmodule::PyModule;
 use super::objstr::{self, PyStringRef};
 use super::objtype;
 use crate::function::PyFuncArgs;
 use crate::obj::objproperty::PropertyBuilder;
 use crate::pyobject::{
-    AttributeProtocol, DictProtocol, IdProtocol, PyAttributes, PyContext, PyObject, PyObjectRef,
-    PyRef, PyResult, PyValue, TypeProtocol,
+    DictProtocol, IdProtocol, PyAttributes, PyContext, PyObject, PyObjectRef, PyResult, PyValue,
+    TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -18,8 +19,6 @@ impl PyValue for PyInstance {
     }
 }
 
-pub type PyInstanceRef = PyRef<PyInstance>;
-
 pub fn new_instance(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult {
     // more or less __new__ operator
     let cls = args.shift();
@@ -97,28 +96,27 @@ fn object_hash(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
 }
 
 fn object_setattr(
-    obj: PyInstanceRef,
+    obj: PyObjectRef,
     attr_name: PyStringRef,
     value: PyObjectRef,
     vm: &VirtualMachine,
 ) -> PyResult<()> {
     trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);
-    let cls = obj.as_object().typ();
+    let cls = obj.type_pyref();
 
-    if let Some(attr) = cls.get_attr(&attr_name.value) {
-        let attr_class = attr.typ();
-        if let Some(descriptor) = attr_class.get_attr("__set__") {
+    if let Some(attr) = objtype::class_get_attr(&cls, &attr_name.value) {
+        if let Some(descriptor) = objtype::class_get_attr(&attr.type_pyref(), "__set__") {
             return vm
-                .invoke(descriptor, vec![attr, obj.into_object(), value])
+                .invoke(descriptor, vec![attr, obj.clone(), value])
                 .map(|_| ());
         }
     }
 
-    if let Some(ref dict) = obj.as_object().dict {
+    if let Some(ref dict) = obj.clone().dict {
         dict.borrow_mut().insert(attr_name.value.clone(), value);
         Ok(())
     } else {
-        let type_name = objtype::get_type_name(&obj.as_object().typ());
+        let type_name = objtype::get_type_name(obj.type_ref());
         Err(vm.new_attribute_error(format!(
             "'{}' object has no attribute '{}'",
             type_name, &attr_name.value
@@ -126,23 +124,20 @@ fn object_setattr(
     }
 }
 
-fn object_delattr(obj: PyInstanceRef, attr_name: PyStringRef, vm: &VirtualMachine) -> PyResult<()> {
-    let cls = obj.as_object().typ();
+fn object_delattr(obj: PyObjectRef, attr_name: PyStringRef, vm: &VirtualMachine) -> PyResult<()> {
+    let cls = obj.type_pyref();
 
-    if let Some(attr) = cls.get_attr(&attr_name.value) {
-        let attr_class = attr.typ();
-        if let Some(descriptor) = attr_class.get_attr("__delete__") {
-            return vm
-                .invoke(descriptor, vec![attr, obj.into_object()])
-                .map(|_| ());
+    if let Some(attr) = objtype::class_get_attr(&cls, &attr_name.value) {
+        if let Some(descriptor) = objtype::class_get_attr(&attr.type_pyref(), "__delete__") {
+            return vm.invoke(descriptor, vec![attr, obj.clone()]).map(|_| ());
         }
     }
 
-    if let Some(ref dict) = obj.as_object().dict {
+    if let Some(ref dict) = obj.dict {
         dict.borrow_mut().remove(&attr_name.value);
         Ok(())
     } else {
-        let type_name = objtype::get_type_name(&obj.as_object().typ());
+        let type_name = objtype::get_type_name(obj.type_ref());
         Err(vm.new_attribute_error(format!(
             "'{}' object has no attribute '{}'",
             type_name, &attr_name.value
@@ -259,28 +254,43 @@ fn object_getattribute(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
     );
     let name = objstr::get_value(&name_str);
     trace!("object.__getattribute__({:?}, {:?})", obj, name);
-    let cls = obj.typ();
+    let cls = obj.type_pyref();
 
-    if let Some(attr) = cls.get_attr(&name) {
-        let attr_class = attr.typ();
-        if attr_class.has_attr("__set__") {
-            if let Some(descriptor) = attr_class.get_attr("__get__") {
-                return vm.invoke(descriptor, vec![attr, obj.clone(), cls]);
+    if let Some(attr) = objtype::class_get_attr(&cls, &name) {
+        let attr_class = attr.type_pyref();
+        if objtype::class_has_attr(&attr_class, "__set__") {
+            if let Some(descriptor) = objtype::class_get_attr(&attr_class, "__get__") {
+                return vm.invoke(descriptor, vec![attr, obj.clone(), cls.into_object()]);
             }
         }
     }
 
-    if let Some(obj_attr) = obj.get_attr(&name) {
+    if let Some(obj_attr) = object_getattr(&obj, &name) {
         Ok(obj_attr)
-    } else if let Some(attr) = cls.get_attr(&name) {
+    } else if let Some(attr) = objtype::class_get_attr(&cls, &name) {
         vm.call_get_descriptor(attr, obj.clone())
-    } else if let Some(getter) = cls.get_attr("__getattr__") {
+    } else if let Some(getter) = objtype::class_get_attr(&cls, "__getattr__") {
         vm.invoke(getter, vec![obj.clone(), name_str.clone()])
     } else {
         Err(vm.new_attribute_error(format!("{} has no attribute '{}'", obj, name)))
     }
 }
 
+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()
+    } else {
+        None
+    }
+}
+
 pub fn get_attributes(obj: &PyObjectRef) -> PyAttributes {
     // Get class attributes:
     let mut attributes = objtype::get_attributes(obj.type_pyref());

vm/src/obj/objtype.rs

@@ -2,10 +2,10 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 use std::fmt;
 
-use crate::function::PyFuncArgs;
+use crate::function::{Args, KwArgs, PyFuncArgs};
 use crate::pyobject::{
-    AttributeProtocol, FromPyObjectRef, IdProtocol, PyAttributes, PyContext, PyObject, PyObjectRef,
-    PyRef, PyResult, PyValue, TypeProtocol,
+    FromPyObjectRef, IdProtocol, PyAttributes, PyContext, PyObject, PyObjectRef, PyRef, PyResult,
+    PyValue, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -115,6 +115,42 @@ impl PyClassRef {
     fn prepare(_name: PyStringRef, _bases: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef {
         vm.new_dict()
     }
+
+    fn getattribute(self, name_ref: PyStringRef, vm: &VirtualMachine) -> PyResult {
+        let name = &name_ref.value;
+        trace!("type.__getattribute__({:?}, {:?})", self, name);
+        let mcl = self.type_pyref();
+
+        if let Some(attr) = class_get_attr(&mcl, &name) {
+            let attr_class = attr.type_pyref();
+            if class_has_attr(&attr_class, "__set__") {
+                if let Some(descriptor) = class_get_attr(&attr_class, "__get__") {
+                    return vm.invoke(
+                        descriptor,
+                        vec![attr, self.into_object(), mcl.into_object()],
+                    );
+                }
+            }
+        }
+
+        if let Some(attr) = class_get_attr(&self, &name) {
+            let attr_class = attr.type_pyref();
+            if let Some(descriptor) = class_get_attr(&attr_class, "__get__") {
+                let none = vm.get_none();
+                return vm.invoke(descriptor, vec![attr, none, self.into_object()]);
+            }
+        }
+
+        if let Some(cls_attr) = class_get_attr(&self, &name) {
+            Ok(cls_attr)
+        } else if let Some(attr) = class_get_attr(&mcl, &name) {
+            vm.call_get_descriptor(attr, self.into_object())
+        } else if let Some(getter) = class_get_attr(&self, "__getattr__") {
+            vm.invoke(getter, vec![mcl.into_object(), name_ref.into_object()])
+        } else {
+            Err(vm.new_attribute_error(format!("{} has no attribute '{}'", self, name)))
+        }
+    }
 }
 
 /*
@@ -136,7 +172,7 @@ pub fn init(ctx: &PyContext) {
                 .create(),
         "__repr__" => ctx.new_rustfunc(PyClassRef::repr),
         "__prepare__" => ctx.new_rustfunc(PyClassRef::prepare),
-        "__getattribute__" => ctx.new_rustfunc(type_getattribute),
+        "__getattribute__" => ctx.new_rustfunc(PyClassRef::getattribute),
         "__instancecheck__" => ctx.new_rustfunc(PyClassRef::instance_check),
         "__subclasscheck__" => ctx.new_rustfunc(PyClassRef::subclass_check),
         "__doc__" => ctx.new_str(type_doc.to_string()),
@@ -218,15 +254,14 @@ pub fn type_new_class(
     )
 }
 
-pub fn type_call(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult {
-    debug!("type_call: {:?}", args);
-    let cls = args.shift();
-    let new = cls.get_attr("__new__").unwrap();
-    let new_wrapped = vm.call_get_descriptor(new, cls)?;
-    let obj = vm.invoke(new_wrapped, args.clone())?;
+pub fn type_call(class: PyClassRef, args: Args, kwargs: KwArgs, vm: &VirtualMachine) -> PyResult {
+    debug!("type_call: {:?}", class);
+    let new = class_get_attr(&class, "__new__").expect("All types should have a __new__.");
+    let new_wrapped = vm.call_get_descriptor(new, class.into_object())?;
+    let obj = vm.invoke(new_wrapped, (&args, &kwargs))?;
 
     if let Ok(init) = vm.get_method(obj.clone(), "__init__") {
-        let res = vm.invoke(init, args)?;
+        let res = vm.invoke(init, (&args, &kwargs))?;
         if !res.is(&vm.get_none()) {
             return Err(vm.new_type_error("__init__ must return None".to_string()));
         }
@@ -234,47 +269,47 @@ pub fn type_call(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult {
     Ok(obj)
 }
 
-pub fn type_getattribute(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [
-            (cls, Some(vm.ctx.object())),
-            (name_str, Some(vm.ctx.str_type()))
-        ]
-    );
-    let name = objstr::get_value(&name_str);
-    trace!("type.__getattribute__({:?}, {:?})", cls, name);
-    let mcl = cls.typ();
-
-    if let Some(attr) = mcl.get_attr(&name) {
-        let attr_class = attr.typ();
-        if attr_class.has_attr("__set__") {
-            if let Some(descriptor) = attr_class.get_attr("__get__") {
-                return vm.invoke(descriptor, vec![attr, cls.clone(), mcl]);
-            }
-        }
-    }
-
-    if let Some(attr) = cls.get_attr(&name) {
-        let attr_class = attr.typ();
-        if let Some(descriptor) = attr_class.get_attr("__get__") {
-            let none = vm.get_none();
-            return vm.invoke(descriptor, vec![attr, none, cls.clone()]);
-        }
-    }
-
-    if let Some(cls_attr) = cls.get_attr(&name) {
-        Ok(cls_attr)
-    } else if let Some(attr) = mcl.get_attr(&name) {
-        vm.call_get_descriptor(attr, cls.clone())
-    } else if let Some(getter) = cls.get_attr("__getattr__") {
-        vm.invoke(getter, vec![mcl, name_str.clone()])
+// Very private helper function for class_get_attr
+fn class_get_attr_in_dict(class: &PyClassRef, attr_name: &str) -> Option<PyObjectRef> {
+    if let Some(ref dict) = class.as_object().dict {
+        dict.borrow().get(attr_name).cloned()
     } else {
-        Err(vm.new_attribute_error(format!("{} has no attribute '{}'", cls, name)))
+        panic!("Only classes should be in MRO!");
     }
 }
 
+// Very private helper function for class_has_attr
+fn class_has_attr_in_dict(class: &PyClassRef, attr_name: &str) -> bool {
+    if let Some(ref dict) = class.as_object().dict {
+        dict.borrow().contains_key(attr_name)
+    } else {
+        panic!("All classes are expected to have dicts!");
+    }
+}
+
+// This is the internal get_attr implementation for fast lookup on a class.
+pub fn class_get_attr(zelf: &PyClassRef, attr_name: &str) -> Option<PyObjectRef> {
+    let mro = &zelf.mro;
+    if let Some(item) = class_get_attr_in_dict(zelf, attr_name) {
+        return Some(item);
+    }
+    for class in mro {
+        if let Some(item) = class_get_attr_in_dict(class, attr_name) {
+            return Some(item);
+        }
+    }
+    None
+}
+
+// This is the internal has_attr implementation for fast lookup on a class.
+pub fn class_has_attr(zelf: &PyClassRef, attr_name: &str) -> bool {
+    class_has_attr_in_dict(zelf, attr_name)
+        || zelf
+            .mro
+            .iter()
+            .any(|d| class_has_attr_in_dict(d, attr_name))
+}
+
 pub fn get_attributes(cls: PyClassRef) -> PyAttributes {
     // Gather all members here:
     let mut attributes = PyAttributes::new();

vm/src/vm.rs

@@ -29,8 +29,8 @@ use crate::obj::objstr::{PyString, PyStringRef};
 use crate::obj::objtuple::PyTuple;
 use crate::obj::objtype;
 use crate::pyobject::{
-    AttributeProtocol, DictProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TryFromObject,
-    TryIntoRef, TypeProtocol,
+    DictProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TryFromObject, TryIntoRef,
+    TypeProtocol,
 };
 use crate::stdlib;
 use crate::sysmodule;
@@ -277,8 +277,8 @@ impl VirtualMachine {
     }
 
     pub fn call_get_descriptor(&self, attr: PyObjectRef, obj: PyObjectRef) -> PyResult {
-        let attr_class = attr.typ();
-        if let Some(descriptor) = attr_class.get_attr("__get__") {
+        let attr_class = attr.type_pyref();
+        if let Some(descriptor) = objtype::class_get_attr(&attr_class, "__get__") {
             let cls = obj.typ();
             self.invoke(descriptor, vec![attr, obj.clone(), cls])
         } else {
@@ -291,8 +291,8 @@ impl VirtualMachine {
         T: Into<PyFuncArgs>,
     {
         // This is only used in the vm for magic methods, which use a greatly simplified attribute lookup.
-        let cls = obj.typ();
-        match cls.get_attr(method_name) {
+        let cls = obj.type_pyref();
+        match objtype::class_get_attr(&cls, method_name) {
             Some(func) => {
                 trace!(
                     "vm.call_method {:?} {:?} {:?} -> {:?}",
@@ -575,8 +575,8 @@ impl VirtualMachine {
     // get_method should be used for internal access to magic methods (by-passing
     // the full getattribute look-up.
     pub fn get_method(&self, obj: PyObjectRef, method_name: &str) -> PyResult {
-        let cls = obj.typ();
-        match cls.get_attr(method_name) {
+        let cls = obj.type_pyref();
+        match objtype::class_get_attr(&cls, method_name) {
             Some(method) => self.call_get_descriptor(method, obj.clone()),
             None => Err(self.new_type_error(format!("{} has no method {:?}", obj, method_name))),
         }