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.