Merge pull request RustPython#3662 from Snowapril/object_protocol

Relocate `PyObject_Generic*` into object protocol

Author: youknowone

vm/src/builtins/genericalias.rs

@@ -369,7 +369,7 @@ impl GetAttr for PyGenericAlias {
     fn getattro(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
         for exc in &ATTR_EXCEPTIONS {
             if *(*exc) == attr.to_string() {
-                return vm.generic_getattribute(zelf.as_object().to_owned(), attr);
+                return zelf.as_object().generic_getattr(attr, vm);
             }
         }
         zelf.origin().get_attr(attr, vm)

vm/src/builtins/module.rs

@@ -49,8 +49,9 @@ impl PyModule {
     }
 
     fn getattr_inner(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(attr) =
-            vm.generic_getattribute_opt(zelf.to_owned().into(), name.clone(), None)?
+        if let Some(attr) = zelf
+            .as_object()
+            .generic_getattr_opt(name.clone(), None, vm)?
         {
             return Ok(attr);
         }
@@ -66,7 +67,8 @@ impl PyModule {
     }
 
     fn name(zelf: PyRef<Self>, vm: &VirtualMachine) -> Option<PyStrRef> {
-        vm.generic_getattribute_opt(zelf.into(), PyStr::from("__name__").into_ref(vm), None)
+        zelf.as_object()
+            .generic_getattr_opt(PyStr::from("__name__").into_ref(vm), None, vm)
             .unwrap_or(None)
             .and_then(|obj| obj.downcast::<PyStr>().ok())
     }

vm/src/builtins/object.rs

@@ -162,13 +162,13 @@ impl PyBaseObject {
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        generic_setattr(&obj, name, Some(value), vm)
+        obj.generic_setattr(name, Some(value), vm)
     }
 
     /// Implement delattr(self, name).
     #[pymethod]
     fn __delattr__(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        generic_setattr(&obj, name, None, vm)
+        obj.generic_setattr(name, None, vm)
     }
 
     #[pyslot]
@@ -178,7 +178,7 @@ impl PyBaseObject {
         value: Option<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        generic_setattr(&*obj, attr_name, value, vm)
+        obj.generic_setattr(attr_name, value, vm)
     }
 
     /// Return str(self).
@@ -288,7 +288,7 @@ impl PyBaseObject {
     #[pyslot]
     pub(crate) fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
         vm_trace!("object.__getattribute__({:?}, {:?})", obj, name);
-        vm.generic_getattribute(obj, name)
+        obj.as_object().generic_getattr(name, vm)
     }
 
     #[pymethod(magic)]
@@ -330,52 +330,6 @@ pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) -
         .map_err(|_| vm.new_attribute_error("This object has no __dict__".to_owned()))
 }
 
-pub fn generic_getattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-    vm.generic_getattribute(obj, attr_name)
-}
-
-#[cfg_attr(feature = "flame-it", flame)]
-pub fn generic_setattr(
-    obj: &PyObject,
-    attr_name: PyStrRef,
-    value: Option<PyObjectRef>,
-    vm: &VirtualMachine,
-) -> PyResult<()> {
-    vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);
-
-    if let Some(attr) = obj.get_class_attr(attr_name.as_str()) {
-        let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load());
-        if let Some(descriptor) = descr_set {
-            return descriptor(attr, obj.to_owned(), value, vm);
-        }
-    }
-
-    if let Some(dict) = obj.dict() {
-        if let Some(value) = value {
-            dict.set_item(attr_name, value, vm)?;
-        } else {
-            dict.del_item(attr_name.clone(), vm).map_err(|e| {
-                if e.fast_isinstance(&vm.ctx.exceptions.key_error) {
-                    vm.new_attribute_error(format!(
-                        "'{}' object has no attribute '{}'",
-                        obj.class().name(),
-                        attr_name,
-                    ))
-                } else {
-                    e
-                }
-            })?;
-        }
-        Ok(())
-    } else {
-        Err(vm.new_attribute_error(format!(
-            "'{}' object has no attribute '{}'",
-            obj.class().name(),
-            attr_name,
-        )))
-    }
-}
-
 pub fn init(ctx: &Context) {
     PyBaseObject::extend_class(ctx, &ctx.types.object_type);
 }

vm/src/builtins/super.rs

@@ -126,7 +126,7 @@ impl PySuper {
 
 impl GetAttr for PySuper {
     fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let skip = |zelf: PyRef<Self>, name| vm.generic_getattribute(zelf.into(), name);
+        let skip = |zelf: PyRef<Self>, name| zelf.as_object().generic_getattr(name, vm);
         let (obj, start_type): (PyObjectRef, PyTypeRef) = match zelf.obj.clone() {
             Some(o) => o,
             None => return skip(zelf, name),

vm/src/builtins/union.rs

@@ -266,7 +266,7 @@ impl GetAttr for PyUnion {
     fn getattro(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
         for &exc in CLS_ATTRS {
             if *exc == attr.to_string() {
-                return vm.generic_getattribute(zelf.as_object().to_owned(), attr);
+                return zelf.as_object().generic_getattr(attr, vm);
             }
         }
         zelf.as_object().to_pyobject(vm).get_attr(attr, vm)

vm/src/protocol/object.rs

@@ -3,8 +3,8 @@
 
 use crate::{
     builtins::{
-        pystr::IntoPyStrRef, PyBytes, PyDict, PyGenericAlias, PyInt, PyStrRef, PyTupleRef,
-        PyTypeRef,
+        pystr::IntoPyStrRef, PyBytes, PyDict, PyDictRef, PyGenericAlias, PyInt, PyStrRef,
+        PyTupleRef, PyTypeRef,
     },
     bytesinner::ByteInnerNewOptions,
     common::{hash::PyHash, str::to_ascii},
@@ -118,8 +118,6 @@ impl PyObject {
         setattro(self, attr_name, attr_value, vm)
     }
 
-    // PyObject *PyObject_GenericGetAttr(PyObject *o, PyObject *name)
-
     pub fn set_attr(
         &self,
         attr_name: impl IntoPyStrRef,
@@ -131,6 +129,109 @@ impl PyObject {
     }
 
     // int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value)
+    #[cfg_attr(feature = "flame-it", flame)]
+    pub fn generic_setattr(
+        &self,
+        attr_name: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);
+
+        if let Some(attr) = self.get_class_attr(attr_name.as_str()) {
+            let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load());
+            if let Some(descriptor) = descr_set {
+                return descriptor(attr, self.to_owned(), value, vm);
+            }
+        }
+
+        if let Some(dict) = self.dict() {
+            if let Some(value) = value {
+                dict.set_item(attr_name, value, vm)?;
+            } else {
+                dict.del_item(attr_name.clone(), vm).map_err(|e| {
+                    if e.fast_isinstance(&vm.ctx.exceptions.key_error) {
+                        vm.new_attribute_error(format!(
+                            "'{}' object has no attribute '{}'",
+                            self.class().name(),
+                            attr_name,
+                        ))
+                    } else {
+                        e
+                    }
+                })?;
+            }
+            Ok(())
+        } else {
+            Err(vm.new_attribute_error(format!(
+                "'{}' object has no attribute '{}'",
+                self.class().name(),
+                attr_name,
+            )))
+        }
+    }
+
+    pub fn generic_getattr(&self, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        self.generic_getattr_opt(name.clone(), None, vm)?
+            .ok_or_else(|| vm.new_attribute_error(format!("{} has no attribute '{}'", self, name)))
+    }
+
+    /// CPython _PyObject_GenericGetAttrWithDict
+    pub fn generic_getattr_opt(
+        &self,
+        name_str: PyStrRef,
+        dict: Option<PyDictRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<Option<PyObjectRef>> {
+        let name = name_str.as_str();
+        let obj_cls = self.class();
+        let cls_attr = match obj_cls.get_attr(name) {
+            Some(descr) => {
+                let descr_cls = descr.class();
+                let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load());
+                if let Some(descr_get) = descr_get {
+                    if descr_cls
+                        .mro_find_map(|cls| cls.slots.descr_set.load())
+                        .is_some()
+                    {
+                        drop(descr_cls);
+                        let cls = obj_cls.into_owned().into();
+                        return descr_get(descr, Some(self.to_pyobject(vm)), Some(cls), vm)
+                            .map(Some);
+                    }
+                }
+                drop(descr_cls);
+                Some((descr, descr_get))
+            }
+            None => None,
+        };
+
+        let dict = dict.or_else(|| self.dict());
+
+        let attr = if let Some(dict) = dict {
+            dict.get_item_opt(name, vm)?
+        } else {
+            None
+        };
+
+        if let Some(obj_attr) = attr {
+            Ok(Some(obj_attr))
+        } else if let Some((attr, descr_get)) = cls_attr {
+            match descr_get {
+                Some(descr_get) => {
+                    let cls = obj_cls.into_owned().into();
+                    descr_get(attr, Some(self.to_pyobject(vm)), Some(cls), vm).map(Some)
+                }
+                None => Ok(Some(attr)),
+            }
+        } else if let Some(getter) = obj_cls.get_attr("__getattr__") {
+            drop(obj_cls);
+            vm.invoke(&getter, (self.to_pyobject(vm), name_str))
+                .map(Some)
+        } else {
+            Ok(None)
+        }
+    }
 
     pub fn del_attr(&self, attr_name: impl IntoPyStrRef, vm: &VirtualMachine) -> PyResult<()> {
         let attr_name = attr_name.into_pystr_ref(vm);

vm/src/stdlib/thread.rs

@@ -326,7 +326,8 @@ pub(crate) mod _thread {
             if attr.as_str() == "__dict__" {
                 Ok(ldict.into())
             } else {
-                vm.generic_getattribute_opt(zelf.clone().into(), attr.clone(), Some(ldict))?
+                zelf.as_object()
+                    .generic_getattr_opt(attr.clone(), Some(ldict), vm)?
                     .ok_or_else(|| {
                         vm.new_attribute_error(format!(
                             "{} has no attribute '{}'",

vm/src/types/slot.rs

@@ -1,4 +1,3 @@
-pub use crate::builtins::object::{generic_getattr, generic_setattr};
 use crate::common::{hash::PyHash, lock::PyRwLock};
 use crate::{
     builtins::{PyInt, PyStrRef, PyType, PyTypeRef},

vm/src/vm/mod.rs

@@ -17,7 +17,6 @@ mod vm_ops;
 use crate::{
     builtins::{
         code::{self, PyCode},
-        object,
         pystr::IntoPyStrRef,
         tuple::{PyTuple, PyTupleTyped},
         PyBaseExceptionRef, PyDictRef, PyList, PyModule, PyStrRef, PyTypeRef,
@@ -594,66 +593,6 @@ impl VirtualMachine {
         Some(self.call_if_get_descriptor(method, obj))
     }
 
-    pub fn generic_getattribute(&self, obj: PyObjectRef, name: PyStrRef) -> PyResult {
-        self.generic_getattribute_opt(obj.clone(), name.clone(), None)?
-            .ok_or_else(|| self.new_attribute_error(format!("{} has no attribute '{}'", obj, name)))
-    }
-
-    /// CPython _PyObject_GenericGetAttrWithDict
-    pub fn generic_getattribute_opt(
-        &self,
-        obj: PyObjectRef,
-        name_str: PyStrRef,
-        dict: Option<PyDictRef>,
-    ) -> PyResult<Option<PyObjectRef>> {
-        let name = name_str.as_str();
-        let obj_cls = obj.class();
-        let cls_attr = match obj_cls.get_attr(name) {
-            Some(descr) => {
-                let descr_cls = descr.class();
-                let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load());
-                if let Some(descr_get) = descr_get {
-                    if descr_cls
-                        .mro_find_map(|cls| cls.slots.descr_set.load())
-                        .is_some()
-                    {
-                        drop(descr_cls);
-                        let cls = obj_cls.into_owned().into();
-                        return descr_get(descr, Some(obj), Some(cls), self).map(Some);
-                    }
-                }
-                drop(descr_cls);
-                Some((descr, descr_get))
-            }
-            None => None,
-        };
-
-        let dict = dict.or_else(|| obj.dict());
-
-        let attr = if let Some(dict) = dict {
-            dict.get_item_opt(name, self)?
-        } else {
-            None
-        };
-
-        if let Some(obj_attr) = attr {
-            Ok(Some(obj_attr))
-        } else if let Some((attr, descr_get)) = cls_attr {
-            match descr_get {
-                Some(descr_get) => {
-                    let cls = obj_cls.into_owned().into();
-                    descr_get(attr, Some(obj), Some(cls), self).map(Some)
-                }
-                None => Ok(Some(attr)),
-            }
-        } else if let Some(getter) = obj_cls.get_attr("__getattr__") {
-            drop(obj_cls);
-            self.invoke(&getter, (obj, name_str)).map(Some)
-        } else {
-            Ok(None)
-        }
-    }
-
     pub fn is_callable(&self, obj: &PyObject) -> bool {
         obj.class()
             .mro_find_map(|cls| cls.slots.call.load())
@@ -741,6 +680,6 @@ impl VirtualMachine {
         attr_value: impl Into<PyObjectRef>,
     ) -> PyResult<()> {
         let val = attr_value.into();
-        object::generic_setattr(module, attr_name.into_pystr_ref(self), Some(val), self)
+        module.generic_setattr(attr_name.into_pystr_ref(self), Some(val), self)
     }
 }