diff --git a/changelog.d/718.change.rst b/changelog.d/718.change.rst new file mode 100644 index 000000000..a5dbfde25 --- /dev/null +++ b/changelog.d/718.change.rst @@ -0,0 +1 @@ +Fixed creation of extra slot for ``attr.ib`` when parent class already has a slot with the same name diff --git a/src/attr/_make.py b/src/attr/_make.py index 64af3ccab..1ed202f75 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -703,7 +703,6 @@ def _create_slots_class(self): """ Build and return a new class with a `__slots__` attribute. """ - base_names = self._base_names cd = { k: v for k, v in iteritems(self._cls_dict) @@ -727,12 +726,21 @@ def _create_slots_class(self): cd["__setattr__"] = object.__setattr__ break - # Traverse the MRO to check for an existing __weakref__. + # Traverse the MRO to collect existing slots + # and check for an existing __weakref__. + existing_slots = dict() weakref_inherited = False for base_cls in self._cls.__mro__[1:-1]: if base_cls.__dict__.get("__weakref__", None) is not None: weakref_inherited = True - break + existing_slots.update( + { + name: getattr(base_cls, name) + for name in getattr(base_cls, "__slots__", []) + } + ) + + base_names = set(self._base_names) names = self._attr_names if ( @@ -746,6 +754,17 @@ def _create_slots_class(self): # We only add the names of attributes that aren't inherited. # Setting __slots__ to inherited attributes wastes memory. slot_names = [name for name in names if name not in base_names] + # There are slots for attributes from current class + # that are defined in parent classes. + # As their descriptors may be overriden by a child class, + # we collect them here and update the class dict + reused_slots = { + slot: slot_descriptor + for slot, slot_descriptor in iteritems(existing_slots) + if slot in slot_names + } + slot_names = [name for name in slot_names if name not in reused_slots] + cd.update(reused_slots) if self._cache_hash: slot_names.append(_hash_cache_field) cd["__slots__"] = tuple(slot_names) diff --git a/tests/test_slots.py b/tests/test_slots.py index b57fc639d..d5106ffb8 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -268,6 +268,70 @@ class C2(C1): assert {"x": 1, "y": 2, "z": "test"} == attr.asdict(c2) +def test_inheritance_from_slots_with_attribute_override(): + """ + Inheriting from a slotted class doesn't re-create existing slots + """ + + class HasXSlot(object): + __slots__ = ("x",) + + @attr.s(slots=True, hash=True) + class C2Slots(C1Slots): + # y re-defined here but it shouldn't get a slot + y = attr.ib() + z = attr.ib() + + @attr.s(slots=True, hash=True) + class NonAttrsChild(HasXSlot): + # Parent class has slot for "x" already, so we skip it + x = attr.ib() + y = attr.ib() + z = attr.ib() + + c2 = C2Slots(1, 2, "test") + assert 1 == c2.x + assert 2 == c2.y + assert "test" == c2.z + + assert {"z"} == set(C2Slots.__slots__) + + na = NonAttrsChild(1, 2, "test") + assert 1 == na.x + assert 2 == na.y + assert "test" == na.z + + assert {"__weakref__", "y", "z"} == set(NonAttrsChild.__slots__) + + +def test_inherited_slot_reuses_slot_descriptor(): + """ + We reuse slot descriptor for an attr.ib defined in a slotted attr.s + """ + + class HasXSlot(object): + __slots__ = ("x",) + + class OverridesX(HasXSlot): + @property + def x(self): + return None + + @attr.s(slots=True) + class Child(OverridesX): + x = attr.ib() + + assert Child.x is not OverridesX.x + assert Child.x is HasXSlot.x + + c = Child(1) + assert 1 == c.x + assert set() == set(Child.__slots__) + + ox = OverridesX() + assert ox.x is None + + def test_bare_inheritance_from_slots(): """ Inheriting from a bare attrs slotted class works.