Merge pull request RustPython#3748 from youknowone/getattr

Fix getattr to use `__getattribute__` correctly

Author: youknowone

Lib/test/test_enum.py

@@ -429,8 +429,6 @@ def red(self):
                 green = 2
                 blue = 3
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_enum_with_value_name(self):
         class Huh(Enum):
             name = 1
@@ -1668,8 +1666,6 @@ class Test(Base):
             test = 1
         self.assertIs(type(Test.test), Test)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_subclass_duplicate_name_dynamic(self):
         from types import DynamicClassAttribute
         class Base(Enum):

vm/src/builtins/weakproxy.rs

@@ -2,7 +2,7 @@ use super::{PyStrRef, PyType, PyTypeRef, PyWeak};
 use crate::{
     class::PyClassImpl,
     function::OptionalArg,
-    types::{Constructor, SetAttr},
+    types::{Constructor, GetAttr, SetAttr},
     Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
 };
 
@@ -57,14 +57,8 @@ crate::common::static_cell! {
     static WEAK_SUBCLASS: PyTypeRef;
 }
 
-#[pyimpl(with(SetAttr, Constructor))]
+#[pyimpl(with(GetAttr, SetAttr, Constructor))]
 impl PyWeakProxy {
-    // TODO: callbacks
-    #[pymethod(magic)]
-    fn getattr(&self, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let obj = self.weak.upgrade().ok_or_else(|| new_reference_error(vm))?;
-        obj.get_attr(attr_name, vm)
-    }
     #[pymethod(magic)]
     fn str(&self, vm: &VirtualMachine) -> PyResult<PyStrRef> {
         match self.weak.upgrade() {
@@ -81,6 +75,14 @@ fn new_reference_error(vm: &VirtualMachine) -> PyRef<super::PyBaseException> {
     )
 }
 
+impl GetAttr for PyWeakProxy {
+    // TODO: callbacks
+    fn getattro(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let obj = zelf.weak.upgrade().ok_or_else(|| new_reference_error(vm))?;
+        obj.get_attr(name, vm)
+    }
+}
+
 impl SetAttr for PyWeakProxy {
     fn setattro(
         zelf: &crate::Py<Self>,

vm/src/protocol/object.rs

@@ -233,9 +233,6 @@ impl PyObject {
                 }
                 None => Ok(Some(attr)),
             }
-        } else if let Some(getter) = obj_cls.get_attr(identifier!(vm, __getattr__)) {
-            drop(obj_cls);
-            vm.invoke(&getter, (self.to_owned(), name_str)).map(Some)
         } else {
             Ok(None)
         }

vm/src/types/slot.rs

@@ -414,7 +414,15 @@ fn call_wrapper(zelf: &PyObject, args: FuncArgs, vm: &VirtualMachine) -> PyResul
 }
 
 fn getattro_wrapper(zelf: &PyObject, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-    vm.call_special_method(zelf.to_owned(), identifier!(vm, __getattribute__), (name,))
+    let __getattribute__ = identifier!(vm, __getattribute__);
+    let __getattr__ = identifier!(vm, __getattr__);
+    match vm.call_special_method(zelf.to_owned(), __getattribute__, (name.clone(),)) {
+        Ok(r) => Ok(r),
+        Err(_) if zelf.class().has_attr(__getattr__) => {
+            vm.call_special_method(zelf.to_owned(), __getattr__, (name,))
+        }
+        Err(e) => Err(e),
+    }
 }
 
 fn setattro_wrapper(
@@ -522,7 +530,7 @@ impl PyType {
             "__call__" => {
                 update_slot!(call, call_wrapper);
             }
-            "__getattribute__" => {
+            "__getattr__" | "__getattribute__" => {
                 update_slot!(getattro, getattro_wrapper);
             }
             "__setattr__" | "__delattr__" => {