gh-105936: Properly update closure cells for `__setattr__` and `__delattr__` in frozen dataclasses with slots (GH-144021)
(cherry picked from commit
8a398bfbbc6769f6cabb3177702e7a506e203d61)
The cherry-pick required additional changes beyond the original commit
because 3.13 lacks the `__class__` closure cell fixup machinery that
was added in 3.14 by GH-124455 (gh-90562). Specifically:
- Backported `_update_func_cell_for__class__()` helper function and the
closure fixup loop in `_add_slots()` from GH-124455. Without these,
renaming the closure variable from `cls` to `__class__` has no effect
because nothing updates the cell when the class is recreated with slots.
- Changed `_add_slots()` to use `newcls` instead of reusing `cls` for the
recreated class, so both old and new class references are available for
the fixup loop.
- Replaced `assertNotHasAttr` with `assertFalse(hasattr(...))` in tests
(assertNotHasAttr was added in 3.14).
- Dropped `test_original_class_is_gced` additions (that test does not
exist on 3.13; it was added by GH-137047 for gh-135228 which was not
backported to 3.13).
gh-148947: dataclasses: fix error on empty __class__ cell (GH-148948)
Also add a test demonstrating the need for the existing "is oldcls" check.
(cherry picked from commit
6d7bbee1d5714a345dca5a7e4089de3c2fc0fb59)
---------
Co-authored-by: Prometheus3375 <prometheus3375@gmail.com>
Co-authored-by: Sviataslau <35541026+Prometheus3375@users.noreply.github.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
return_type=None)
-def _frozen_get_del_attr(cls, fields, func_builder):
- locals = {'cls': cls,
+def _frozen_set_del_attr(cls, fields, func_builder):
+ locals = {'__class__': cls,
'FrozenInstanceError': FrozenInstanceError}
- condition = 'type(self) is cls'
+ condition = 'type(self) is __class__'
if fields:
condition += ' or name in {' + ', '.join(repr(f.name) for f in fields) + '}'
('self', 'name', 'value'),
(f' if {condition}:',
' raise FrozenInstanceError(f"cannot assign to field {name!r}")',
- f' super(cls, self).__setattr__(name, value)'),
+ f' super(__class__, self).__setattr__(name, value)'),
locals=locals,
overwrite_error=True)
func_builder.add_fn('__delattr__',
('self', 'name'),
(f' if {condition}:',
' raise FrozenInstanceError(f"cannot delete field {name!r}")',
- f' super(cls, self).__delattr__(name)'),
+ f' super(__class__, self).__delattr__(name)'),
locals=locals,
overwrite_error=True)
overwrite_error='Consider using functools.total_ordering')
if frozen:
- _frozen_get_del_attr(cls, field_list, func_builder)
+ _frozen_set_del_attr(cls, field_list, func_builder)
# Decide if/how we're going to create a hash function.
hash_action = _hash_action[bool(unsafe_hash),
raise TypeError(f"Slots of '{cls.__name__}' cannot be determined")
+def _update_func_cell_for__class__(f, oldcls, newcls):
+ # Returns True if we update a cell, else False.
+ if f is None:
+ # f will be None in the case of a property where not all of
+ # fget, fset, and fdel are used. Nothing to do in that case.
+ return False
+ try:
+ idx = f.__code__.co_freevars.index("__class__")
+ except ValueError:
+ # This function doesn't reference __class__, so nothing to do.
+ return False
+ # Fix the cell to point to the new class, if it's already pointing
+ # at the old class.
+ closure = f.__closure__[idx]
+
+ try:
+ contents = closure.cell_contents
+ except ValueError:
+ # Cell is empty
+ return False
+
+ # This check makes it so we avoid updating an incorrect cell if the
+ # class body contains a function that was defined in a different class.
+ if contents is oldcls:
+ closure.cell_contents = newcls
+ return True
+ return False
+
+
def _add_slots(cls, is_frozen, weakref_slot):
# Need to create a new class, since we can't set __slots__
# after a class has been created.
# And finally create the class.
qualname = getattr(cls, '__qualname__', None)
- cls = type(cls)(cls.__name__, cls.__bases__, cls_dict)
+ newcls = type(cls)(cls.__name__, cls.__bases__, cls_dict)
if qualname is not None:
- cls.__qualname__ = qualname
+ newcls.__qualname__ = qualname
if is_frozen:
# Need this for pickling frozen classes with slots.
if '__getstate__' not in cls_dict:
- cls.__getstate__ = _dataclass_getstate
+ newcls.__getstate__ = _dataclass_getstate
if '__setstate__' not in cls_dict:
- cls.__setstate__ = _dataclass_setstate
-
- return cls
+ newcls.__setstate__ = _dataclass_setstate
+
+ # Fix up any closures which reference __class__. This is used to
+ # fix zero argument super so that it points to the correct class
+ # (the newly created one, which we're returning) and not the
+ # original class. We can break out of this loop as soon as we
+ # make an update, since all closures for a class will share a
+ # given cell.
+ for member in newcls.__dict__.values():
+ # If this is a wrapped function, unwrap it.
+ member = inspect.unwrap(member)
+
+ if isinstance(member, types.FunctionType):
+ if _update_func_cell_for__class__(member, cls, newcls):
+ break
+ elif isinstance(member, property):
+ if (_update_func_cell_for__class__(member.fget, cls, newcls)
+ or _update_func_cell_for__class__(member.fset, cls, newcls)
+ or _update_func_cell_for__class__(member.fdel, cls, newcls)):
+ break
+
+ return newcls
def dataclass(cls=None, /, *, init=True, repr=True, eq=True, order=False,
class TestFrozen(unittest.TestCase):
+ # Some tests have a subtest with a slotted dataclass.
+ # See https://github.com/python/cpython/issues/105936 for the reasons.
+
def test_frozen(self):
- @dataclass(frozen=True)
- class C:
- i: int
+ for slots in (False, True):
+ with self.subTest(slots=slots):
- c = C(10)
- self.assertEqual(c.i, 10)
- with self.assertRaises(FrozenInstanceError):
- c.i = 5
- self.assertEqual(c.i, 10)
+ @dataclass(frozen=True, slots=slots)
+ class C:
+ i: int
+
+ c = C(10)
+ self.assertEqual(c.i, 10)
+ with self.assertRaises(FrozenInstanceError):
+ c.i = 5
+ self.assertEqual(c.i, 10)
+ with self.assertRaises(FrozenInstanceError):
+ del c.i
+ self.assertEqual(c.i, 10)
def test_frozen_empty(self):
- @dataclass(frozen=True)
- class C:
- pass
+ for slots in (False, True):
+ with self.subTest(slots=slots):
- c = C()
- self.assertFalse(hasattr(c, 'i'))
- with self.assertRaises(FrozenInstanceError):
- c.i = 5
- self.assertFalse(hasattr(c, 'i'))
- with self.assertRaises(FrozenInstanceError):
- del c.i
+ @dataclass(frozen=True, slots=slots)
+ class C:
+ pass
+
+ c = C()
+ self.assertFalse(hasattr(c, 'i'))
+ with self.assertRaises(FrozenInstanceError):
+ c.i = 5
+ self.assertFalse(hasattr(c, 'i'))
+ with self.assertRaises(FrozenInstanceError):
+ del c.i
def test_inherit(self):
@dataclass(frozen=True)
d.i = 5
def test_non_frozen_normal_derived(self):
- # See bpo-32953.
-
- @dataclass(frozen=True)
- class D:
- x: int
- y: int = 10
-
- class S(D):
- pass
+ # See bpo-32953 and https://github.com/python/cpython/issues/105936
+ for slots in (False, True):
+ with self.subTest(slots=slots):
- s = S(3)
- self.assertEqual(s.x, 3)
- self.assertEqual(s.y, 10)
- s.cached = True
+ @dataclass(frozen=True, slots=slots)
+ class D:
+ x: int
+ y: int = 10
- # But can't change the frozen attributes.
- with self.assertRaises(FrozenInstanceError):
- s.x = 5
- with self.assertRaises(FrozenInstanceError):
- s.y = 5
- self.assertEqual(s.x, 3)
- self.assertEqual(s.y, 10)
- self.assertEqual(s.cached, True)
+ class S(D):
+ pass
- with self.assertRaises(FrozenInstanceError):
- del s.x
- self.assertEqual(s.x, 3)
- with self.assertRaises(FrozenInstanceError):
- del s.y
- self.assertEqual(s.y, 10)
- del s.cached
- self.assertFalse(hasattr(s, 'cached'))
- with self.assertRaises(AttributeError) as cm:
- del s.cached
- self.assertNotIsInstance(cm.exception, FrozenInstanceError)
+ s = S(3)
+ self.assertEqual(s.x, 3)
+ self.assertEqual(s.y, 10)
+ s.cached = True
+
+ # But can't change the frozen attributes.
+ with self.assertRaises(FrozenInstanceError):
+ s.x = 5
+ with self.assertRaises(FrozenInstanceError):
+ s.y = 5
+ self.assertEqual(s.x, 3)
+ self.assertEqual(s.y, 10)
+ self.assertEqual(s.cached, True)
+
+ with self.assertRaises(FrozenInstanceError):
+ del s.x
+ self.assertEqual(s.x, 3)
+ with self.assertRaises(FrozenInstanceError):
+ del s.y
+ self.assertEqual(s.y, 10)
+ del s.cached
+ self.assertFalse(hasattr(s, 'cached'))
+ with self.assertRaises(AttributeError) as cm:
+ del s.cached
+ self.assertNotIsInstance(cm.exception, FrozenInstanceError)
def test_non_frozen_normal_derived_from_empty_frozen(self):
@dataclass(frozen=True)
# that we create internally.
self.assertEqual(CorrectSuper.args, ["default", "default"])
+ def test_empty_class_cell(self):
+ # gh-148947: Make sure that we explicitly handle the empty class cell.
+ def maker():
+ if False:
+ __class__ = 42
+
+ def method(self):
+ return __class__
+ return method
+
+ from dataclasses import dataclass
+
+ @dataclass(slots=True)
+ class X:
+ a: int
+
+ meth = maker()
+
+ with self.assertRaisesRegex(NameError, '__class__'):
+ X(1).meth()
+
+ def test_class_cell_from_other_class(self):
+ # This test fails without the "is oldcls" check in
+ # _update_func_cell_for__class__.
+ class Base:
+ def meth(self):
+ return "Base"
+
+ class Child(Base):
+ def meth(self):
+ return super().meth() + " Child"
+
+ @dataclass(slots=True)
+ class DC(Child):
+ a: int
+
+ meth = Child.meth
+
+ closure = DC.meth.__closure__
+ self.assertEqual(len(closure), 1)
+ self.assertIs(closure[0].cell_contents, Child)
+
+ self.assertEqual(DC(1).meth(), "Base Child")
+
class TestDescriptors(unittest.TestCase):
def test_set_name(self):
--- /dev/null
+Attempting to mutate non-field attributes of :mod:`dataclasses`
+with both *frozen* and *slots* being ``True`` now raises
+:class:`~dataclasses.FrozenInstanceError` instead of :class:`TypeError`.
+Their non-dataclass subclasses can now freely mutate non-field attributes,
+and the original non-slotted class can be garbage collected. The fix also
+handles the case of an empty ``__class__`` cell on a function found within
+the class (gh-148947).
projects / thirdparty / Python / cpython.git / commitdiff
