Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Proxy super().x = y and del super().x
This patch was originally contributed by Daniel Urban, whose summary
follows:

> I'm attaching a patch implementing super.__setattr__ (and
> __delattr__).

> The implementation in the patch only works, if super can find a data
> descriptor in the MRO, otherwise it throws an AttributeError. As it
> can be seen in the tests, in some cases this may result in
> counter-intuitive behaviour. But I wasn't able to find another
> behaviour, that is consistent with both super.__getattr__ and normal
> __setattr__ semantics.

Co-authored-by: hab <_@habnab.it>
Co-authored-by: Markus Kitsinger (SwooshyCueb) <root@swooshalicio.us>
  • Loading branch information
3 people committed Dec 7, 2021
commit b955e79d6f0b777de9a2b69b452b64d9f4f3e6f5
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
Copy Markdown
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