[pypy-commit] pypy cpyext-tp_dictoffset: test, fixes for when tp_dictoffset is non-zero, when PyObject_*Attr* can run

[mattip]

Author: Matti Picus <matti.picus at gmail.com>
Branch: cpyext-tp_dictoffset
Changeset: r92370:e2b3bea05c2b
Date: 2017-09-11 18:06 +0300
http://bitbucket.org/pypy/pypy/changeset/e2b3bea05c2b/

Log:	test, fixes for when tp_dictoffset is non-zero, when PyObject_*Attr*
	can run

diff --git a/pypy/module/cpyext/api.py b/pypy/module/cpyext/api.py
--- a/pypy/module/cpyext/api.py
+++ b/pypy/module/cpyext/api.py
@@ -1183,6 +1183,7 @@
 def attach_recursively(space, static_pyobjs, static_objs_w, attached_objs, i):
     # Start at i but make sure all the base classes are already attached
     from pypy.module.cpyext.pyobject import get_typedescr, make_ref
+    from pypy.module.cpyext.typeobject import type_attach
     if i in attached_objs:
         return
     py_obj = static_pyobjs[i]
@@ -1205,9 +1206,18 @@
     typedescr = get_typedescr(w_type.layout.typedef)
     py_obj.c_ob_type = rffi.cast(PyTypeObjectPtr,
                                  make_ref(space, w_type))
-    typedescr.attach(space, py_obj, w_obj)
-    # prevent PyObject_SetAttr on built-ins
-    rffi.cast(PyTypeObjectPtr, py_obj).c_tp_dictoffset = 0
+    if isinstance(w_obj, W_TypeObject):
+        # type and module objects need to have tp_dictoffset != 0
+        # so that, like user-defined python classes, they can have
+        # attributes set via PyObject_SetAttr()
+        set_tp_dictoffset = False
+        if w_obj is space.w_type:
+            set_tp_dictoffset = True
+        if space.is_w(w_obj, space.gettypeobject(Module.typedef)):
+            set_tp_dictoffset = True
+        type_attach(space, py_obj, w_obj, set_tp_dictoffset=set_tp_dictoffset)
+    else:
+        typedescr.attach(space, py_obj, w_obj) 
     attached_objs.append(i)
 
 
diff --git a/pypy/module/cpyext/object.py b/pypy/module/cpyext/object.py
--- a/pypy/module/cpyext/object.py
+++ b/pypy/module/cpyext/object.py
@@ -123,7 +123,9 @@
 @cpython_api([PyObject, PyObject, PyObject], rffi.INT_real, error=-1)
 def PyObject_SetAttr(space, w_obj, w_name, w_value):
     pyobj = as_pyobj(space, w_obj) # assumes w_obj is kept alive for the call
-    if (pyobj.c_ob_type.c_tp_dictoffset == 0):
+    if (pyobj.c_ob_type.c_tp_dictoffset == 0 and
+            not pyobj.c_ob_type.c_tp_setattro and
+            not pyobj.c_ob_type.c_tp_setattr):
         raise oefmt(space.w_AttributeError,
              "object has no attribute %s", space.text_w(w_name))
     operation.setattr(space, w_obj, w_name, w_value)
@@ -133,7 +135,9 @@
 def PyObject_SetAttrString(space, w_obj, name_ptr, w_value):
     w_name = space.newtext(rffi.charp2str(name_ptr))
     pyobj = as_pyobj(space, w_obj) # assumes w_obj is kept alive for the call
-    if (pyobj.c_ob_type.c_tp_dictoffset == 0):
+    if (pyobj.c_ob_type.c_tp_dictoffset == 0 and
+            not pyobj.c_ob_type.c_tp_setattro and
+            not pyobj.c_ob_type.c_tp_setattr):
         raise oefmt(space.w_AttributeError,
              "object has no attribute %s", space.text_w(w_name))
     operation.setattr(space, w_obj, w_name, w_value)
@@ -144,7 +148,9 @@
     """Delete attribute named attr_name, for object o. Returns -1 on failure.
     This is the equivalent of the Python statement del o.attr_name."""
     pyobj = as_pyobj(space, w_obj) # assumes w_obj is kept alive for the call
-    if (pyobj.c_ob_type.c_tp_dictoffset == 0):
+    if (pyobj.c_ob_type.c_tp_dictoffset == 0 and
+            not pyobj.c_ob_type.c_tp_setattro and
+            not pyobj.c_ob_type.c_tp_setattr):
         raise oefmt(space.w_AttributeError,
              "object has no attribute %s", space.text_w(w_name))
     space.delattr(w_obj, w_name)
@@ -156,7 +162,9 @@
     This is the equivalent of the Python statement del o.attr_name."""
     w_name = space.newtext(rffi.charp2str(name_ptr))
     pyobj = as_pyobj(space, w_obj) # assumes w_obj is kept alive for the call
-    if (pyobj.c_ob_type.c_tp_dictoffset == 0):
+    if (pyobj.c_ob_type.c_tp_dictoffset == 0 and
+            not pyobj.c_ob_type.c_tp_setattro and
+            not pyobj.c_ob_type.c_tp_setattr):
         raise oefmt(space.w_AttributeError,
              "object has no attribute %s", space.text_w(w_name))
     space.delattr(w_obj, w_name)
diff --git a/pypy/module/cpyext/test/test_cpyext.py b/pypy/module/cpyext/test/test_cpyext.py
--- a/pypy/module/cpyext/test/test_cpyext.py
+++ b/pypy/module/cpyext/test/test_cpyext.py
@@ -584,12 +584,17 @@
             Py_INCREF(t);
             return t;
         }
+        static PyObject * foo_get_dictoffset(PyObject *self, PyObject * args) {
+            return PyLong_FromLong(args->ob_type->tp_dictoffset);
+        }
         static PyMethodDef methods[] = {
             { "test", foo_test, METH_VARARGS },
+            { "get_dictoffset",   foo_get_dictoffset, METH_O },
             { NULL }
         };
         """
         module = self.import_module(name='foo', init=init, body=body)
+        assert module.get_dictoffset(module) > 0
         assert module.test(True, True) == True
 
     def test_exception(self):
diff --git a/pypy/module/cpyext/test/test_eval.py b/pypy/module/cpyext/test/test_eval.py
--- a/pypy/module/cpyext/test/test_eval.py
+++ b/pypy/module/cpyext/test/test_eval.py
@@ -388,6 +388,10 @@
                         return NULL;
                     }
                     return args->ob_type->tp_iternext(args);
+                 '''),
+                ('get_dictoffset', "METH_O",
+                 '''
+                    return PyLong_FromLong(args->ob_type->tp_dictoffset);
                  '''),])
             def __init__(self, N):
                 self.N = N
@@ -415,3 +419,4 @@
             except StopIteration:
                 pass
             assert out == [0, 1, 2, 3, 4]
+            assert module.get_dictoffset(c) > 0
diff --git a/pypy/module/cpyext/typeobject.py b/pypy/module/cpyext/typeobject.py
--- a/pypy/module/cpyext/typeobject.py
+++ b/pypy/module/cpyext/typeobject.py
@@ -569,18 +569,18 @@
     # w_obj is an instance of w_A or one of its subclasses. So climb up the
     # inheritance chain until base.c_tp_dealloc is exactly this_func, and then
     # continue on up until they differ.
-    #print 'subtype_dealloc, start from', rffi.charp2str(base.c_tp_name)
+    # print 'subtype_dealloc, start from', rffi.charp2str(base.c_tp_name)
     while base.c_tp_dealloc != this_func_ptr:
         base = base.c_tp_base
         assert base
-        #print '                 ne move to', rffi.charp2str(base.c_tp_name)
+        # print '                 ne move to', rffi.charp2str(base.c_tp_name)
         w_obj = from_ref(space, rffi.cast(PyObject, base))
     while base.c_tp_dealloc == this_func_ptr:
         base = base.c_tp_base
         assert base
-        #print '                 eq move to', rffi.charp2str(base.c_tp_name)
+        # print '                 eq move to', rffi.charp2str(base.c_tp_name)
         w_obj = from_ref(space, rffi.cast(PyObject, base))
-    #print '                   end with', rffi.charp2str(base.c_tp_name)
+    # print '                   end with', rffi.charp2str(base.c_tp_name)
     dealloc = base.c_tp_dealloc
     # XXX call tp_del if necessary
     generic_cpy_call(space, dealloc, obj)
@@ -740,7 +740,7 @@
 
     return rffi.cast(PyObject, heaptype)
 
-def type_attach(space, py_obj, w_type, w_userdata=None):
+def type_attach(space, py_obj, w_type, w_userdata=None, set_tp_dictoffset=True):
     """
     Fills a newly allocated PyTypeObject from an existing type.
     """
@@ -815,7 +815,8 @@
         if pto.c_tp_base != base_object_pto or flags & Py_TPFLAGS_HEAPTYPE:
                 pto.c_tp_new = pto.c_tp_base.c_tp_new
         Py_DecRef(space, base_object_pyo)
-    pto.c_tp_dictoffset = rffi.offsetof(lltype.typeOf(pto).TO, 'c_tp_dictoffset')
+    if set_tp_dictoffset:
+        pto.c_tp_dictoffset = rffi.offsetof(lltype.typeOf(pto).TO, 'c_tp_dictoffset')
     pto.c_tp_flags |= Py_TPFLAGS_READY
     return pto