mirror of
https://github.com/python/cpython.git
synced 2026-04-20 02:40:59 +00:00
gh-105936: Properly update closure cells for __setattr__ and __delattr__ in frozen dataclasses with slots (GH-144021)
This commit is contained in:
parent
63492628be
commit
8a398bfbbc
3 changed files with 83 additions and 56 deletions
|
|
@ -724,10 +724,10 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init,
|
|||
annotation_fields=annotation_fields)
|
||||
|
||||
|
||||
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) + '}'
|
||||
|
||||
|
|
@ -735,14 +735,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)
|
||||
|
||||
|
|
@ -1205,7 +1205,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),
|
||||
|
|
|
|||
|
|
@ -3052,29 +3052,41 @@ class C(base):
|
|||
|
||||
|
||||
class TestFrozen(unittest.TestCase):
|
||||
def test_frozen(self):
|
||||
@dataclass(frozen=True)
|
||||
class C:
|
||||
i: int
|
||||
# Some tests have a subtest with a slotted dataclass.
|
||||
# See https://github.com/python/cpython/issues/105936 for the reasons.
|
||||
|
||||
c = C(10)
|
||||
self.assertEqual(c.i, 10)
|
||||
with self.assertRaises(FrozenInstanceError):
|
||||
c.i = 5
|
||||
self.assertEqual(c.i, 10)
|
||||
def test_frozen(self):
|
||||
for slots in (False, True):
|
||||
with self.subTest(slots=slots):
|
||||
|
||||
@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.assertNotHasAttr(c, 'i')
|
||||
with self.assertRaises(FrozenInstanceError):
|
||||
c.i = 5
|
||||
self.assertNotHasAttr(c, 'i')
|
||||
with self.assertRaises(FrozenInstanceError):
|
||||
del c.i
|
||||
@dataclass(frozen=True, slots=slots)
|
||||
class C:
|
||||
pass
|
||||
|
||||
c = C()
|
||||
self.assertNotHasAttr(c, 'i')
|
||||
with self.assertRaises(FrozenInstanceError):
|
||||
c.i = 5
|
||||
self.assertNotHasAttr(c, 'i')
|
||||
with self.assertRaises(FrozenInstanceError):
|
||||
del c.i
|
||||
|
||||
def test_inherit(self):
|
||||
@dataclass(frozen=True)
|
||||
|
|
@ -3270,41 +3282,43 @@ class D(I):
|
|||
d.i = 5
|
||||
|
||||
def test_non_frozen_normal_derived(self):
|
||||
# See bpo-32953.
|
||||
# See bpo-32953 and https://github.com/python/cpython/issues/105936
|
||||
for slots in (False, True):
|
||||
with self.subTest(slots=slots):
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class D:
|
||||
x: int
|
||||
y: int = 10
|
||||
@dataclass(frozen=True, slots=slots)
|
||||
class D:
|
||||
x: int
|
||||
y: int = 10
|
||||
|
||||
class S(D):
|
||||
pass
|
||||
class S(D):
|
||||
pass
|
||||
|
||||
s = S(3)
|
||||
self.assertEqual(s.x, 3)
|
||||
self.assertEqual(s.y, 10)
|
||||
s.cached = True
|
||||
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)
|
||||
# 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.assertNotHasAttr(s, 'cached')
|
||||
with self.assertRaises(AttributeError) as cm:
|
||||
del s.cached
|
||||
self.assertNotIsInstance(cm.exception, FrozenInstanceError)
|
||||
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.assertNotHasAttr(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)
|
||||
|
|
@ -3971,6 +3985,14 @@ class SlotsTest:
|
|||
|
||||
return SlotsTest
|
||||
|
||||
# See https://github.com/python/cpython/issues/135228#issuecomment-3755979059
|
||||
def make_frozen():
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class SlotsTest:
|
||||
pass
|
||||
|
||||
return SlotsTest
|
||||
|
||||
def make_with_annotations():
|
||||
@dataclass(slots=True)
|
||||
class SlotsTest:
|
||||
|
|
@ -3996,7 +4018,7 @@ class SlotsTest:
|
|||
|
||||
return SlotsTest
|
||||
|
||||
for make in (make_simple, make_with_annotations, make_with_annotations_and_method, make_with_forwardref):
|
||||
for make in (make_simple, make_frozen, make_with_annotations, make_with_annotations_and_method, make_with_forwardref):
|
||||
with self.subTest(make=make):
|
||||
C = make()
|
||||
support.gc_collect()
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue