Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-14965: Proxy super().x = y and del super().x (updated) #29950

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions Lib/test/test_super.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,26 @@ class F(E):
class G(A):
pass

class P:
@property
def x(self):
return self._x
@x.setter
def x(self, value):
self._x = value
@x.deleter
def x(self):
del self._x

class Q(P):
pass

class R(P):
x = None

class S(Q, R):
pass


class TestSuper(unittest.TestCase):

Expand Down Expand Up @@ -317,6 +337,60 @@ def test_super_init_leaks(self):
for i in range(1000):
super.__init__(sp, int, i)

def test_super_setattr(self):
# See issue14965
val = object()
p = P()
with self.assertRaises(AttributeError):
super(P, p).x = val
with self.assertRaises(AttributeError):
del super(P, p).x

q = Q()
super(Q, q).x = val
self.assertIs(q.__dict__['_x'], val)
self.assertIs(super(Q, q).x, val)
del super(Q, q).x
self.assertNotIn('_x', q.__dict__)

r = R()
super(R, r).x = val
self.assertIs(r.__dict__['_x'], val)
self.assertIs(super(R, r).x, val)
del super(R, r).x
self.assertNotIn('_x', r.__dict__)

s = S()
with self.assertRaises(AttributeError):
super(S, s).x = val
with self.assertRaises(AttributeError):
del super(S, s).x
self.assertIs(super(S, s).x, None)

def test_super_property(self):
# See issue14965
class T(P):
def __init__(self, x):
self.x = x
@property
def x(self):
return super().x / 2
@x.setter
def x(self, value):
super().x = value * 2
@x.deleter
def x(self):
del super().x
t = T(1)
self.assertEqual(t.x, 1)
self.assertEqual(t._x, 2)
t.x = 10
self.assertEqual(t.x, 10)
self.assertEqual(t._x, 20)
del t.x
self.assertFalse(hasattr(t, 'x'))
self.assertFalse(hasattr(t, '_x'))


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Proxying data descriptor setters with ``super().x = y`` and deleters with ``del
super().x`` will now follow getter use with ``super().x``. Previously, these
would unconditionally fail. Patch by Daniel Urban.
67 changes: 66 additions & 1 deletion Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -8831,6 +8831,71 @@ super_getattro(PyObject *self, PyObject *name)
return PyObject_GenericGetAttr(self, name);
}

static int
super_setattro(PyObject *self, PyObject *name, PyObject *value)
{
superobject *su = (superobject *)self;
int skip = (su->obj_type == NULL);

if (!skip) {
/* We don't allow overwriting __class__. */
skip = (PyUnicode_Check(name) &&
PyUnicode_GET_LENGTH(name) == 9 &&
PyUnicode_CompareWithASCIIString(name, "__class__") == 0);
}

if (!skip) {
PyObject *mro, *descr, *tmp, *dict;
PyTypeObject *starttype;
descrsetfunc f;
Py_ssize_t i, n;
int done = 0, result = 0;

starttype = su->obj_type;
mro = starttype->tp_mro;

if (mro == NULL)
n = 0;
else {
assert(PyTuple_Check(mro));
n = PyTuple_GET_SIZE(mro);
}
for (i = 0; i < n; i++) {
if ((PyObject *)(su->type) == PyTuple_GET_ITEM(mro, i))
break;
}
i++;
descr = NULL;
/* keep a strong reference to mro because starttype->tp_mro can be
replaced during PyDict_GetItem(dict, name) */
Py_INCREF(mro);
for (; i < n; i++) {
tmp = PyTuple_GET_ITEM(mro, i);
if (PyType_Check(tmp))
dict = ((PyTypeObject *)tmp)->tp_dict;
else
continue;
descr = PyDict_GetItem(dict, name);
if (descr != NULL) {
Py_INCREF(descr);
f = Py_TYPE(descr)->tp_descr_set;
if ((f != NULL) && PyDescr_IsData(descr)) {
/* We found a data descriptor: */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the behavior if there isn't a data descriptor is that we fall back to PyObject_GenericSetAttr, which will fail because super() doesn't allow setting any attributes. Shouldn't we instead set the attr on the object super is proxying for?

Use case to think about: We're using super().x = ... to set a @property on a base class. Then the base class is refactored so that x is a regular attribute, not a @property. As a user, I would expect super().x = ... to continue to work as before.

result = f(descr, su->obj, value);
done = 1;
}
Py_DECREF(descr);
break;
}
}
Py_DECREF(mro);
if (done) {
return result;
}
}
return PyObject_GenericSetAttr(self, name, value);
}

static PyTypeObject *
supercheck(PyTypeObject *type, PyObject *obj)
{
Expand Down Expand Up @@ -9082,7 +9147,7 @@ PyTypeObject PySuper_Type = {
0, /* tp_call */
0, /* tp_str */
super_getattro, /* tp_getattro */
0, /* tp_setattro */
super_setattro, /* tp_setattro */
0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
Py_TPFLAGS_BASETYPE, /* tp_flags */
Expand Down