From 7c3d881594db5c10ea73a7fa26e2b5b7c6619c4f Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 19 Apr 2024 15:00:35 +0300 Subject: [PATCH 1/4] gh-118033: Fix `__weakref__` not set for generic dataclasses --- Lib/dataclasses.py | 5 + Lib/test/test_dataclasses/__init__.py | 95 +++++++++++++++++++ ...-04-19-14-59-53.gh-issue-118033.amS4Gw.rst | 2 + 3 files changed, 102 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-04-19-14-59-53.gh-issue-118033.amS4Gw.rst diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 3acd03cd865234..2a0707fc02ec62 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1201,6 +1201,10 @@ def _get_slots(cls): match cls.__dict__.get('__slots__'): # A class which does not define __slots__ at all is equivalent # to a class defining __slots__ = ('__dict__', '__weakref__') + case None if getattr(cls, '__weakrefoffset__', -1) == 0: + # Except for special cases, inheriting from them do not set + # any slots at all: + yield from () case None: yield from ('__dict__', '__weakref__') case str(slot): @@ -1228,6 +1232,7 @@ def _add_slots(cls, is_frozen, weakref_slot): inherited_slots = set( itertools.chain.from_iterable(map(_get_slots, cls.__mro__[1:-1])) ) + print(inherited_slots) # The slots for our class. Remove slots from our base classes. Add # '__weakref__' if weakref_slot was given, unless it is already present. cls_dict["__slots__"] = tuple( diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 832e5672c77d0d..6c73d33e0cc469 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -3515,8 +3515,103 @@ class A: class B(A): pass + self.assertEqual(B.__slots__, ()) B() + def test_dataclass_derived_generic(self): + T = typing.TypeVar('T') + + @dataclass(slots=True, weakref_slot=True) + class A(typing.Generic[T]): + pass + self.assertEqual(A.__slots__, ('__weakref__',)) + self.assertTrue(A.__weakref__) + A() + + @dataclass(slots=True, weakref_slot=True) + class B[T2]: + pass + self.assertEqual(B.__slots__, ('__weakref__',)) + self.assertTrue(B.__weakref__) + B() + + def test_dataclass_derived_generic_from_base(self): + T = typing.TypeVar('T') + + class RawBase: ... + + @dataclass(slots=True, weakref_slot=True) + class C1(typing.Generic[T], RawBase): + pass + self.assertEqual(C1.__slots__, ()) + self.assertTrue(C1.__weakref__) + C1() + @dataclass(slots=True, weakref_slot=True) + class C2(RawBase, typing.Generic[T]): + pass + self.assertEqual(C2.__slots__, ()) + self.assertTrue(C2.__weakref__) + C2() + + @dataclass(slots=True, weakref_slot=True) + class D[T2](RawBase): + pass + self.assertEqual(D.__slots__, ()) + self.assertTrue(D.__weakref__) + D() + + def test_dataclass_derived_generic_from_slotted_base(self): + T = typing.TypeVar('T') + + class WithSlots: + __slots__ = ('a', 'b') + + @dataclass(slots=True, weakref_slot=True) + class E1(WithSlots, Generic[T]): + pass + self.assertEqual(E1.__slots__, ('__weakref__',)) + self.assertTrue(E1.__weakref__) + E1() + @dataclass(slots=True, weakref_slot=True) + class E2(Generic[T], WithSlots): + pass + self.assertEqual(E2.__slots__, ('__weakref__',)) + self.assertTrue(E2.__weakref__) + E2() + + @dataclass(slots=True, weakref_slot=True) + class F[T2](WithSlots): + pass + self.assertEqual(F.__slots__, ('__weakref__',)) + self.assertTrue(F.__weakref__) + F() + + def test_dataclass_derived_generic_from_slotted_base(self): + T = typing.TypeVar('T') + + class WithWeakrefSlot: + __slots__ = ('__weakref__',) + + @dataclass(slots=True, weakref_slot=True) + class G1(WithWeakrefSlot, Generic[T]): + pass + self.assertEqual(G1.__slots__, ()) + self.assertTrue(G1.__weakref__) + G1() + @dataclass(slots=True, weakref_slot=True) + class G2(Generic[T], WithWeakrefSlot): + pass + self.assertEqual(G2.__slots__, ()) + self.assertTrue(G2.__weakref__) + G2() + + @dataclass(slots=True, weakref_slot=True) + class H[T2](WithWeakrefSlot): + pass + self.assertEqual(H.__slots__, ()) + self.assertTrue(H.__weakref__) + H() + class TestDescriptors(unittest.TestCase): def test_set_name(self): diff --git a/Misc/NEWS.d/next/Library/2024-04-19-14-59-53.gh-issue-118033.amS4Gw.rst b/Misc/NEWS.d/next/Library/2024-04-19-14-59-53.gh-issue-118033.amS4Gw.rst new file mode 100644 index 00000000000000..7ceb29330abf22 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-04-19-14-59-53.gh-issue-118033.amS4Gw.rst @@ -0,0 +1,2 @@ +Fix :func:`dataclasses.dataclass` not creating a ``__weakref__`` slot when +subclassing :class:`typing.Generic`. From ac440de892c2256f5b53e10238fd8aa126c19257 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 19 Apr 2024 15:17:34 +0300 Subject: [PATCH 2/4] Also check `__dictoffeset__` --- Lib/dataclasses.py | 11 +++++++---- Lib/test/test_dataclasses/__init__.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 2a0707fc02ec62..91e56a4898d6b8 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1201,12 +1201,15 @@ def _get_slots(cls): match cls.__dict__.get('__slots__'): # A class which does not define __slots__ at all is equivalent # to a class defining __slots__ = ('__dict__', '__weakref__') - case None if getattr(cls, '__weakrefoffset__', -1) == 0: + case None: # Except for special cases, inheriting from them do not set # any slots at all: - yield from () - case None: - yield from ('__dict__', '__weakref__') + slots = ['__dict__', '__weakref__'] + if getattr(cls, '__weakrefoffset__', -1) == 0: + slots.remove('__weakref__') + if getattr(cls, '__dictrefoffset__', -1) == 0: + slots.remove('__dict__') + yield from slots case str(slot): yield slot # Slots may be any iterable, but we cannot handle an iterator diff --git a/Lib/test/test_dataclasses/__init__.py b/Lib/test/test_dataclasses/__init__.py index 6c73d33e0cc469..ea49596eaa4d96 100644 --- a/Lib/test/test_dataclasses/__init__.py +++ b/Lib/test/test_dataclasses/__init__.py @@ -3612,6 +3612,17 @@ class H[T2](WithWeakrefSlot): self.assertTrue(H.__weakref__) H() + def test_dataclass_slot_dict(self): + class WithDictSlot: + __slots__ = ('__dict__',) + + @dataclass(slots=True) + class A(WithDictSlot): ... + + self.assertEqual(A.__slots__, ()) + self.assertEqual(A().__dict__, {}) + A() + class TestDescriptors(unittest.TestCase): def test_set_name(self): From 34cc31dd8971b5805a3ce15b67708d8dd051fa5e Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 19 Apr 2024 16:09:07 +0300 Subject: [PATCH 3/4] Remove `print` --- Lib/dataclasses.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 91e56a4898d6b8..a23290b5a57517 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1235,7 +1235,6 @@ def _add_slots(cls, is_frozen, weakref_slot): inherited_slots = set( itertools.chain.from_iterable(map(_get_slots, cls.__mro__[1:-1])) ) - print(inherited_slots) # The slots for our class. Remove slots from our base classes. Add # '__weakref__' if weakref_slot was given, unless it is already present. cls_dict["__slots__"] = tuple( From 7c09ec80e6c1f61f306d650d7f1a70d5efd6ba06 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 4 May 2024 11:34:27 +0300 Subject: [PATCH 4/4] Address review --- Lib/dataclasses.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index a23290b5a57517..aeafbfbbe6e9c4 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -1199,16 +1199,16 @@ def _dataclass_setstate(self, state): def _get_slots(cls): match cls.__dict__.get('__slots__'): - # A class which does not define __slots__ at all is equivalent - # to a class defining __slots__ = ('__dict__', '__weakref__') + # `__dictoffset__` and `__weakrefoffset__` can tell us whether + # the base type has dict/weakref slots, in a way that works correctly + # for both Python classes and C extension types. Extension types + # don't use `__slots__` for slot creation case None: - # Except for special cases, inheriting from them do not set - # any slots at all: - slots = ['__dict__', '__weakref__'] - if getattr(cls, '__weakrefoffset__', -1) == 0: - slots.remove('__weakref__') - if getattr(cls, '__dictrefoffset__', -1) == 0: - slots.remove('__dict__') + slots = [] + if getattr(cls, '__weakrefoffset__', -1) != 0: + slots.append('__weakref__') + if getattr(cls, '__dictrefoffset__', -1) != 0: + slots.append('__dict__') yield from slots case str(slot): yield slot