[GR-34614] Call __getattr__ from tp_getattro wrapper

PullRequest: graalpython/2801

Author: msimacek

graalpython/com.oracle.graal.python.cext/src/typeobject.c

@@ -750,13 +750,13 @@ int type_ready_graalpy_slot_conv(PyTypeObject* cls, PyObject* dict) {
 
     // NOTE: The slots may be called from managed code, i.e., we need to wrap the functions
     // and convert arguments that should be C primitives.
-    ADD_SLOT_CONV("__getattr__", cls->tp_getattr, -2, JWRAPPER_GETATTR);
+    ADD_SLOT_CONV("__getattribute__", cls->tp_getattr, -2, JWRAPPER_GETATTR);
     ADD_SLOT_CONV("__setattr__", cls->tp_setattr, -3, JWRAPPER_SETATTR);
     ADD_SLOT_CONV("__repr__", cls->tp_repr, -1, JWRAPPER_REPR);
     ADD_SLOT_CONV("__hash__", cls->tp_hash, -1, JWRAPPER_HASHFUNC);
     ADD_SLOT_CONV("__call__", cls->tp_call, METH_KEYWORDS | METH_VARARGS, JWRAPPER_CALL);
     ADD_SLOT_CONV("__str__", cls->tp_str, -1, JWRAPPER_STR);
-    ADD_SLOT_CONV("__getattr__", cls->tp_getattro, -2, JWRAPPER_DIRECT);
+    ADD_SLOT_CONV("__getattribute__", cls->tp_getattro, -2, JWRAPPER_DIRECT);
     ADD_SLOT_CONV("__setattr__", cls->tp_setattro, -3, JWRAPPER_SETATTRO);
     ADD_SLOT_CONV("__clear__", cls->tp_clear, -1, JWRAPPER_INQUIRY);
 

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_functions.py

@@ -375,10 +375,15 @@ def richcompare_bool(args):
         argspec="OOi",
         resultspec="i",
     )
-    
+
+    class TypeWithGetattr:
+        def __getattr__(self, item):
+            return item
+
     __PyObject_GetAttrString_ARGS = (
             (MyObject(), "foo"),
             ([], "__len__"),
+            (TypeWithGetattr(), "foo"),
         )
     test_PyObject_GetAttrString = CPyExtFunction(
         lambda args: getattr(*args),
@@ -442,6 +447,7 @@ def _ref_hash_not_implemented(args):
     __PyObject_GetAttr_ARGS = (
             (MyObject(), "foo"),
             ([], "__len__"),
+            (TypeWithGetattr(), "foo"),
         )
     test_PyObject_GetAttr = CPyExtFunction(
         lambda args: getattr(*args),
@@ -480,4 +486,4 @@ def _ref_hash_not_implemented(args):
         arguments=["PyObject* func"],
         argspec="O",
         cmpfunc=lambda x, y: type(x) == staticmethod
-    )
+    )

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_object.py

@@ -334,6 +334,64 @@ def test_getattro(self):
         tester = TestInt()
         assert tester.foo == "foo"
 
+    def test_getattro_inheritance(self):
+        TestWithGetattro = CPyExtType(
+            'TestWithGetattro',
+            '''
+            static PyObject* getattro(PyObject* self, PyObject* key) {
+                if (PyObject_IsTrue(key)) {
+                    Py_INCREF(key);
+                    return key;
+                }
+                return PyErr_Format(PyExc_AttributeError, "Nope");
+            }
+
+            static PyObject* call_getattro_slot(PyObject* unused, PyObject* args) {
+                PyObject* object;
+                PyObject* key;
+                if (!PyArg_ParseTuple(args, "OO", &object, &key))
+                    return NULL;
+                return Py_TYPE(object)->tp_getattro(object, key);
+            }
+            ''',
+            tp_getattro='getattro',
+            tp_methods='{"call_getattro_slot", (PyCFunction)call_getattro_slot, METH_VARARGS | METH_STATIC, ""}'
+        )
+
+        def validate(cls, has_getattro, has_getattr):
+            foo = cls()
+            if has_getattro:
+                assert foo.asdf == 'asdf'
+                assert TestWithGetattro.call_getattro_slot(foo, 'asdf') == 'asdf'
+            elif has_getattr:
+                assert foo.asdf == 3
+                assert TestWithGetattro.call_getattro_slot(foo, 'asdf') == 3
+            if has_getattro and not has_getattr:
+                assert_raises(AttributeError, lambda: getattr(foo, ''))
+                assert_raises(AttributeError, TestWithGetattro.call_getattro_slot, foo, '')
+            if has_getattr:
+                assert getattr(foo, '') == 3
+                assert TestWithGetattro.call_getattro_slot(foo, '') == 3
+
+        validate(TestWithGetattro, True, False)
+
+        class Subclass(TestWithGetattro):
+            pass
+
+        validate(Subclass, True, False)
+
+        class ObjWithGetattr:
+            def __getattr__(self, item):
+                return 3
+
+        validate(ObjWithGetattr, False, True)
+
+        class SubclassWithGetattr(TestWithGetattro):
+            def __getattr__(self, item):
+                return 3
+
+        validate(SubclassWithGetattr, True, True)
+
     def test_dict(self):
         TestDict = CPyExtType("TestDict",
                              """static PyObject* custom_dict = NULL;