Add a getattro slot

vm/src/obj/objfunction.rs

@@ -19,7 +19,7 @@ use crate::pyobject::{
     PyRef, PyResult, PyValue, TypeProtocol,
 };
 use crate::scope::Scope;
-use crate::slots::{Callable, Comparable, PyComparisonOp, SlotDescriptor};
+use crate::slots::{Callable, Comparable, PyComparisonOp, SlotDescriptor, SlotGetattro};
 use crate::VirtualMachine;
 use itertools::Itertools;
 #[cfg(feature = "jit")]
@@ -357,7 +357,16 @@ impl Comparable for PyBoundMethod {
     }
 }
 
-#[pyimpl(with(Callable, Comparable), flags(HAS_DICT))]
+impl SlotGetattro for PyBoundMethod {
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        if let Some(obj) = zelf.get_class_attr(name.borrow_value()) {
+            return vm.call_if_get_descriptor(obj, zelf.into_object());
+        }
+        vm.get_attribute(zelf.function.clone(), name)
+    }
+}
+
+#[pyimpl(with(Callable, Comparable, SlotGetattro), flags(HAS_DICT))]
 impl PyBoundMethod {
     pub fn new(object: PyObjectRef, function: PyObjectRef) -> Self {
         PyBoundMethod { object, function }
@@ -376,14 +385,6 @@ impl PyBoundMethod {
         vm.get_attribute(self.function.clone(), "__doc__")
     }
 
-    #[pymethod(magic)]
-    fn getattribute(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(obj) = zelf.get_class_attr(name.borrow_value()) {
-            return vm.call_if_get_descriptor(obj, zelf.into_object());
-        }
-        vm.get_attribute(zelf.function.clone(), name)
-    }
-
     #[pyproperty(magic)]
     fn func(&self) -> PyObjectRef {
         self.function.clone()

vm/src/obj/objmodule.rs

@@ -6,6 +6,7 @@ use crate::pyobject::{
     BorrowValue, IntoPyObject, ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult,
     PyValue,
 };
+use crate::slots::SlotGetattro;
 use crate::vm::VirtualMachine;
 
 #[pyclass(module = false, name = "module")]
@@ -42,8 +43,8 @@ pub fn init_module_dict(
         .expect("Failed to set __spec__ on module");
 }
 
-#[pyimpl(flags(BASETYPE, HAS_DICT))]
-impl PyModuleRef {
+#[pyimpl(with(SlotGetattro), flags(BASETYPE, HAS_DICT))]
+impl PyModule {
     #[pyslot]
     fn tp_new(cls: PyTypeRef, _args: PyFuncArgs, vm: &VirtualMachine) -> PyResult<PyModuleRef> {
         PyModule {}.into_ref_with_type(vm, cls)
@@ -51,27 +52,27 @@ impl PyModuleRef {
 
     #[pymethod(magic)]
     fn init(
-        self,
+        zelf: PyRef<Self>,
         name: PyStrRef,
         doc: OptionalOption<PyStrRef>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        debug_assert!(crate::pyobject::TypeProtocol::lease_class(self.as_object())
+        debug_assert!(crate::pyobject::TypeProtocol::lease_class(zelf.as_object())
             .slots
             .flags
             .has_feature(crate::slots::PyTpFlags::HAS_DICT));
         init_module_dict(
             vm,
-            &self.as_object().dict().unwrap(),
+            &zelf.as_object().dict().unwrap(),
             name.into_object(),
             doc.flatten().into_pyobject(vm),
         );
         Ok(())
     }
 
-    fn name(self, vm: &VirtualMachine) -> Option<String> {
+    fn name(zelf: PyRef<Self>, vm: &VirtualMachine) -> Option<String> {
         vm.generic_getattribute_opt(
-            self.as_object().clone(),
+            zelf.as_object().clone(),
             PyStr::from("__name__").into_ref(vm),
             None,
         )
@@ -80,10 +81,28 @@ impl PyModuleRef {
     }
 
     #[pymethod(magic)]
-    fn getattribute(self, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        vm.generic_getattribute_opt(self.as_object().clone(), name.clone(), None)?
+    fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
+        let importlib = vm.import("_frozen_importlib", &[], 0)?;
+        let module_repr = vm.get_attribute(importlib, "_module_repr")?;
+        vm.invoke(&module_repr, vec![zelf.into_object()])
+    }
+
+    #[pymethod(magic)]
+    fn dir(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
+        let dict = zelf
+            .as_object()
+            .dict()
+            .ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
+        let attrs = dict.into_iter().map(|(k, _v)| k).collect();
+        Ok(vm.ctx.new_list(attrs))
+    }
+}
+
+impl SlotGetattro for PyModule {
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        vm.generic_getattribute_opt(zelf.as_object().clone(), name.clone(), None)?
             .ok_or_else(|| {
-                let module_name = if let Some(name) = self.name(vm) {
+                let module_name = if let Some(name) = Self::name(zelf, vm) {
                     format!(" '{}'", name)
                 } else {
                     "".to_owned()
@@ -93,25 +112,8 @@ impl PyModuleRef {
                 )
             })
     }
-
-    #[pymethod(magic)]
-    fn repr(self, vm: &VirtualMachine) -> PyResult {
-        let importlib = vm.import("_frozen_importlib", &[], 0)?;
-        let module_repr = vm.get_attribute(importlib, "_module_repr")?;
-        vm.invoke(&module_repr, vec![self.into_object()])
-    }
-
-    #[pymethod(magic)]
-    fn dir(self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
-        let dict = self
-            .as_object()
-            .dict()
-            .ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
-        let attrs = dict.into_iter().map(|(k, _v)| k).collect();
-        Ok(vm.ctx.new_list(attrs))
-    }
 }
 
 pub(crate) fn init(context: &PyContext) {
-    PyModuleRef::extend_class(&context, &context.types.module_type);
+    PyModule::extend_class(&context, &context.types.module_type);
 }

vm/src/obj/objobject.rs

@@ -242,8 +242,9 @@ impl PyBaseObject {
         }
     }
 
-    #[pymethod(magic)]
-    fn getattribute(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+    #[pymethod(name = "__getattribute__")]
+    #[pyslot]
+    fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
         vm_trace!("object.__getattribute__({:?}, {:?})", obj, name);
         vm.generic_getattribute(obj, name)
     }

vm/src/obj/objsuper.rs

@@ -14,7 +14,7 @@ use crate::pyobject::{
     TryFromObject, TypeProtocol,
 };
 use crate::scope::NameProtocol;
-use crate::slots::SlotDescriptor;
+use crate::slots::{SlotDescriptor, SlotGetattro};
 use crate::vm::VirtualMachine;
 
 use itertools::Itertools;
@@ -34,7 +34,7 @@ impl PyValue for PySuper {
     }
 }
 
-#[pyimpl(with(SlotDescriptor))]
+#[pyimpl(with(SlotGetattro, SlotDescriptor))]
 impl PySuper {
     fn new(typ: PyTypeRef, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
         let obj = if vm.is_none(&obj) {
@@ -55,29 +55,6 @@ impl PySuper {
         }
     }
 
-    #[pymethod(name = "__getattribute__")]
-    fn getattribute(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let (inst, obj_type) = match zelf.obj.clone() {
-            Some(o) => o,
-            None => return vm.generic_getattribute(zelf.into_object(), name),
-        };
-        // skip the classes in obj_type.mro up to and including zelf.typ
-        let mut it = obj_type.iter_mro().peekable();
-        for _ in it.peeking_take_while(|cls| !cls.is(&zelf.typ)) {}
-        for cls in it.skip(1) {
-            if let Some(descr) = cls.get_attr(name.borrow_value()) {
-                return vm
-                    .call_get_descriptor_specific(
-                        descr.clone(),
-                        if inst.is(&obj_type) { None } else { Some(inst) },
-                        Some(obj_type.clone().into_object()),
-                    )
-                    .unwrap_or(Ok(descr));
-            }
-        }
-        vm.generic_getattribute(zelf.into_object(), name)
-    }
-
     #[pyslot]
     fn tp_new(
         cls: PyTypeRef,
@@ -130,6 +107,30 @@ impl PySuper {
     }
 }
 
+impl SlotGetattro for PySuper {
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let (inst, obj_type) = match zelf.obj.clone() {
+            Some(o) => o,
+            None => return vm.generic_getattribute(zelf.into_object(), name),
+        };
+        // skip the classes in obj_type.mro up to and including zelf.typ
+        let mut it = obj_type.iter_mro().peekable();
+        for _ in it.peeking_take_while(|cls| !cls.is(&zelf.typ)) {}
+        for cls in it.skip(1) {
+            if let Some(descr) = cls.get_attr(name.borrow_value()) {
+                return vm
+                    .call_get_descriptor_specific(
+                        descr.clone(),
+                        if inst.is(&obj_type) { None } else { Some(inst) },
+                        Some(obj_type.clone().into_object()),
+                    )
+                    .unwrap_or(Ok(descr));
+            }
+        }
+        vm.generic_getattribute(zelf.into_object(), name)
+    }
+}
+
 impl SlotDescriptor for PySuper {
     fn descr_get(
         zelf: PyObjectRef,

vm/src/obj/objtype.rs

@@ -17,7 +17,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyLease,
     PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
-use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots};
+use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots, SlotGetattro};
 use crate::vm::VirtualMachine;
 use itertools::Itertools;
 use std::ops::Deref;
@@ -165,7 +165,14 @@ impl PyTypeRef {
                 } as _;
                 self.slots.cmp.store(Some(func))
             }
-            _ => (),
+            "__getattribute__" => {
+                let func: slots::GetattroFunc = |zelf, name, vm| {
+                    let magic = get_class_magic(&zelf, "__getattribute__");
+                    vm.invoke(&magic, vec![zelf, name.into_object()])
+                };
+                self.slots.getattro.store(Some(func))
+            }
+            _ => {}
         }
     }
 }
@@ -180,7 +187,7 @@ fn get_class_magic(zelf: &PyObjectRef, name: &str) -> PyObjectRef {
     // attrs.get(name).unwrap().clone()
 }
 
-#[pyimpl(with(Callable), flags(BASETYPE))]
+#[pyimpl(with(SlotGetattro, Callable), flags(BASETYPE))]
 impl PyType {
     #[pyproperty(name = "__mro__")]
     fn get_mro(zelf: PyRef<Self>) -> PyTuple {
@@ -266,53 +273,6 @@ impl PyType {
         vm.ctx.new_dict()
     }
 
-    #[pymethod(magic)]
-    fn getattribute(zelf: PyRef<Self>, name_ref: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let name = name_ref.borrow_value();
-        vm_trace!("type.__getattribute__({:?}, {:?})", zelf, name);
-        let mcl = zelf.lease_class();
-
-        if let Some(attr) = mcl.get_attr(&name) {
-            let attr_class = attr.lease_class();
-            if attr_class.has_attr("__set__") {
-                if let Some(ref descr_get) =
-                    PyLease::into_pyref(attr_class).first_in_mro(|cls| cls.slots.descr_get.load())
-                {
-                    let mcl = PyLease::into_pyref(mcl).into_object();
-                    return descr_get(attr, Some(zelf.into_object()), Some(mcl), vm);
-                }
-            }
-        }
-
-        if let Some(attr) = zelf.get_attr(&name) {
-            let attr_class = attr.class();
-            if let Some(ref descr_get) = attr_class.first_in_mro(|cls| cls.slots.descr_get.load()) {
-                drop(mcl);
-                return descr_get(attr, None, Some(zelf.into_object()), vm);
-            }
-        }
-
-        if let Some(cls_attr) = zelf.get_attr(&name) {
-            Ok(cls_attr)
-        } else if let Some(attr) = mcl.get_attr(&name) {
-            drop(mcl);
-            vm.call_if_get_descriptor(attr, zelf.into_object())
-        } else if let Some(ref getter) = zelf.get_attr("__getattr__") {
-            vm.invoke(
-                getter,
-                vec![
-                    PyLease::into_pyref(mcl).into_object(),
-                    name_ref.into_object(),
-                ],
-            )
-        } else {
-            Err(vm.new_attribute_error(format!(
-                "type object '{}' has no attribute '{}'",
-                zelf, name
-            )))
-        }
-    }
-
     #[pymethod(magic)]
     fn setattr(
         zelf: PyRef<Self>,
@@ -510,6 +470,58 @@ impl PyType {
     }
 }
 
+impl SlotGetattro for PyType {
+    fn getattro(zelf: PyRef<Self>, name_ref: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let name = name_ref.borrow_value();
+        vm_trace!("type.__getattribute__({:?}, {:?})", zelf, name);
+        let mcl = zelf.lease_class();
+
+        let mcl_attr = mcl.get_attr(name);
+
+        if let Some(ref attr) = mcl_attr {
+            let attr_class = attr.lease_class();
+            if attr_class.has_attr("__set__") {
+                if let Some(ref descr_get) =
+                    PyLease::into_pyref(attr_class).first_in_mro(|cls| cls.slots.descr_get.load())
+                {
+                    let mcl = PyLease::into_pyref(mcl).into_object();
+                    return descr_get(attr.clone(), Some(zelf.into_object()), Some(mcl), vm);
+                }
+            }
+        }
+
+        let zelf_attr = zelf.get_attr(name);
+
+        if let Some(ref attr) = zelf_attr {
+            let attr_class = attr.class();
+            if let Some(descr_get) = attr_class.first_in_mro(|cls| cls.slots.descr_get.load()) {
+                drop(mcl);
+                return descr_get(attr.clone(), None, Some(zelf.into_object()), vm);
+            }
+        }
+
+        if let Some(cls_attr) = zelf_attr {
+            Ok(cls_attr)
+        } else if let Some(attr) = mcl_attr {
+            drop(mcl);
+            vm.call_if_get_descriptor(attr, zelf.into_object())
+        } else if let Some(ref getter) = zelf.get_attr("__getattr__") {
+            vm.invoke(
+                getter,
+                vec![
+                    PyLease::into_pyref(mcl).into_object(),
+                    name_ref.into_object(),
+                ],
+            )
+        } else {
+            Err(vm.new_attribute_error(format!(
+                "type object '{}' has no attribute '{}'",
+                zelf, name
+            )))
+        }
+    }
+}
+
 impl Callable for PyType {
     fn call(zelf: &PyRef<Self>, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult {
         vm_trace!("type_call: {:?}", zelf);

vm/src/slots.rs

@@ -3,6 +3,7 @@ use std::cmp::Ordering;
 use crate::common::cell::PyRwLock;
 use crate::common::hash::PyHash;
 use crate::function::{OptionalArg, PyFuncArgs, PyNativeFunc};
+use crate::obj::objstr::PyStrRef;
 use crate::pyobject::{
     Either, IdProtocol, PyComparisonValue, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
 };
@@ -51,6 +52,7 @@ pub(crate) type CmpFunc = fn(
     PyComparisonOp,
     &VirtualMachine,
 ) -> PyResult<Either<PyObjectRef, PyComparisonValue>>;
+pub(crate) type GetattroFunc = fn(PyObjectRef, PyStrRef, &VirtualMachine) -> PyResult;
 
 #[derive(Default)]
 pub struct PyTypeSlots {
@@ -62,6 +64,7 @@ pub struct PyTypeSlots {
     pub descr_get: AtomicCell<Option<DescrGetFunc>>,
     pub hash: AtomicCell<Option<HashFunc>>,
     pub cmp: AtomicCell<Option<CmpFunc>>,
+    pub getattro: AtomicCell<Option<GetattroFunc>>,
 }
 
 impl PyTypeSlots {
@@ -379,3 +382,23 @@ impl PyComparisonOp {
         }
     }
 }
+
+#[pyimpl]
+pub trait SlotGetattro: PyValue {
+    #[pyslot]
+    fn tp_getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        if let Ok(zelf) = obj.downcast::<Self>() {
+            Self::getattro(zelf, name, vm)
+        } else {
+            Err(vm.new_type_error("unexpected payload for __getattribute__".to_owned()))
+        }
+    }
+
+    // TODO: make zelf: &PyRef<Self>
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult;
+
+    #[pymethod]
+    fn __getattribute__(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        Self::getattro(zelf, name, vm)
+    }
+}

vm/src/stdlib/thread.rs

@@ -9,6 +9,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, ItemProtocol, PyCallable, PyClassImpl, PyObjectRef, PyRef,
     PyResult, PyValue, TypeProtocol,
 };
+use crate::slots::SlotGetattro;
 use crate::vm::VirtualMachine;
 
 use parking_lot::{
@@ -296,7 +297,7 @@ impl PyValue for PyLocal {
     }
 }
 
-#[pyimpl(flags(BASETYPE))]
+#[pyimpl(with(SlotGetattro), flags(BASETYPE))]
 impl PyLocal {
     fn ldict(&self, vm: &VirtualMachine) -> PyDictRef {
         self.data.get_or(|| vm.ctx.new_dict()).clone()
@@ -310,20 +311,6 @@ impl PyLocal {
         .into_ref_with_type(vm, cls)
     }
 
-    #[pymethod(magic)]
-    fn getattribute(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let ldict = zelf.ldict(vm);
-        if attr.borrow_value() == "__dict__" {
-            Ok(ldict.into_object())
-        } else {
-            let zelf = zelf.into_object();
-            vm.generic_getattribute_opt(zelf.clone(), attr.clone(), Some(ldict))?
-                .ok_or_else(|| {
-                    vm.new_attribute_error(format!("{} has no attribute '{}'", zelf, attr))
-                })
-        }
-    }
-
     #[pymethod(magic)]
     fn setattr(
         zelf: PyRef<Self>,
@@ -356,6 +343,21 @@ impl PyLocal {
     }
 }
 
+impl SlotGetattro for PyLocal {
+    fn getattro(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let ldict = zelf.ldict(vm);
+        if attr.borrow_value() == "__dict__" {
+            Ok(ldict.into_object())
+        } else {
+            let zelf = zelf.into_object();
+            vm.generic_getattribute_opt(zelf.clone(), attr.clone(), Some(ldict))?
+                .ok_or_else(|| {
+                    vm.new_attribute_error(format!("{} has no attribute '{}'", zelf, attr))
+                })
+        }
+    }
+}
+
 pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
     let ctx = &vm.ctx;
 

vm/src/types.rs

@@ -180,7 +180,7 @@ impl TypeZoo {
             map_type: create_type!(objmap::PyMap),
             mappingproxy_type: create_type!(objmappingproxy::PyMappingProxy),
             memoryview_type: create_type!(objmemory::PyMemoryView),
-            module_type: create_type!(objmodule::PyModuleRef),
+            module_type: create_type!(objmodule::PyModule),
             namespace_type: create_type!(objnamespace::PyNamespace),
             property_type: create_type!(objproperty::PyProperty),
             range_type: create_type!(objrange::PyRange),

vm/src/vm.rs

@@ -971,7 +971,11 @@ impl VirtualMachine {
     {
         let attr_name = attr_name.try_into_ref(self)?;
         vm_trace!("vm.__getattribute__: {:?} {:?}", obj, attr_name);
-        self.call_method(&obj, "__getattribute__", vec![attr_name.into_object()])
+        let getattro = obj
+            .class()
+            .first_in_mro(|cls| cls.slots.getattro.load())
+            .unwrap();
+        getattro(obj, attr_name, self)
     }
 
     pub fn set_attr<K, V>(&self, obj: &PyObjectRef, attr_name: K, attr_value: V) -> PyResult
@@ -1083,9 +1087,11 @@ impl VirtualMachine {
         let name = name_str.borrow_value();
         let cls = obj.class();
 
-        if let Some(attr) = cls.get_attr(&name) {
+        let cls_attr = cls.get_attr(name);
+
+        if let Some(ref attr) = cls_attr {
             if attr.lease_class().has_attr("__set__") {
-                if let Some(r) = self.call_get_descriptor(attr, obj.clone()) {
+                if let Some(r) = self.call_get_descriptor(attr.clone(), obj.clone()) {
                     return r.map(Some);
                 }
             }
@@ -1101,7 +1107,7 @@ impl VirtualMachine {
 
         if let Some(obj_attr) = attr {
             Ok(Some(obj_attr))
-        } else if let Some(attr) = cls.get_attr(&name) {
+        } else if let Some(attr) = cls_attr {
             self.call_if_get_descriptor(attr, obj).map(Some)
         } else if let Some(getter) = cls.get_attr("__getattr__") {
             self.invoke(&getter, vec![obj, name_str.into_object()])