From aa1e00f711dbdd032dc8d2967893410ba3e2e9e9 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 13 Apr 2026 00:12:19 +0300 Subject: [PATCH 1/2] [3.13] gh-105936: Properly update closure cells for `__setattr__` and `__delattr__` in frozen dataclasses with slots (GH-144021) 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). Co-authored-by: Prometheus3375 Co-authored-by: Sviataslau <35541026+Prometheus3375@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- Lib/dataclasses.py | 64 ++++++++-- Lib/test/test_dataclasses/__init__.py | 114 ++++++++++-------- ...-01-19-21-23-18.gh-issue-105936.dGrzjM.rst | 5 + 3 files changed, 121 insertions(+), 62 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 7883ce78e57d50..22794a51a5d365 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -665,10 +665,10 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, 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) + '}' @@ -676,14 +676,14 @@ def _frozen_get_del_attr(cls, fields, func_builder): ('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) @@ -1141,7 +1141,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, 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), @@ -1219,6 +1219,27 @@ def _get_slots(cls): 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. I'm not convinced that the "is oldcls" test + # is needed, but other than performance can't hurt. + closure = f.__closure__[idx] + if closure.cell_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. @@ -1260,18 +1281,37 @@ def _add_slots(cls, is_frozen, weakref_slot): # 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, diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index ec70f9ceda5d5e..70afffeef5a21b 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -2865,29 +2865,41 @@ class C(base): 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) @@ -3083,41 +3095,43 @@ class D(I): 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) diff --git a/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst b/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst new file mode 100644 index 00000000000000..c1d3ec806e597c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst @@ -0,0 +1,5 @@ +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. From 678cc46cb7c82f5f9b641b9870286aed0a6f6730 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 25 Apr 2026 11:35:59 -0700 Subject: [PATCH 2/2] [3.13] gh-148947: dataclasses: fix error on empty __class__ cell (GH-148948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Consolidates the news entries given GH-148947 bug was never present in the 3.13 branch Co-authored-by: Jelle Zijlstra Co-authored-by: Bartosz Sławecki Co-Authored-By: Claude Opus 4.7 (1M context) --- Lib/dataclasses.py | 14 ++++-- Lib/test/test_dataclasses/__init__.py | 44 +++++++++++++++++++ ...-01-19-21-23-18.gh-issue-105936.dGrzjM.rst | 4 +- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 22794a51a5d365..8703e0763892fe 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1231,10 +1231,18 @@ def _update_func_cell_for__class__(f, oldcls, newcls): # 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. I'm not convinced that the "is oldcls" test - # is needed, but other than performance can't hurt. + # at the old class. closure = f.__closure__[idx] - if closure.cell_contents is oldcls: + + 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 diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 70afffeef5a21b..3b7a5dbc3202fe 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -3771,6 +3771,50 @@ class WithCorrectSuper(CorrectSuper): # 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): diff --git a/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst b/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst index c1d3ec806e597c..076574ce8599dc 100644 --- a/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst +++ b/Misc/NEWS.d/next/Library/2026-01-19-21-23-18.gh-issue-105936.dGrzjM.rst @@ -2,4 +2,6 @@ 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. +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).