Skip to content

Fix slots issue with deferred base #20573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hauntsaninja merged 2 commits into from
Jan 14, 2026
+25 −5
Merged

Fix slots issue with deferred base #20573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f0dd79
Fix slots issue with deferred base
hauntsaninja Jan 13, 2026
b81647e
simplify
hauntsaninja Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions mypy/plugins/dataclasses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,9 @@ def add_slots(
)
return

generated_slots = {attr.name for attr in attributes}
if (info.slots is not None and info.slots != generated_slots) or info.names.get(
"__slots__"
):
existing_slots = info.names.get("__slots__")
slots_defined_by_plugin = existing_slots is not None and existing_slots.plugin_generated
if existing_slots is not None and not slots_defined_by_plugin:
Comment on lines +461 to +462
Copy link
Collaborator

@A5rocks A5rocks Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really get how the original was wrong. Like why is info.slots a thing (or __slots__ in info.names?)... And why do we have to care about it being plugin generated, is this plugin being run multiple times? (I see the old code had info.slots != generated_slots for this too. That seems odd)

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the deferral led to this being called multiple times

Copy link
Collaborator

@A5rocks A5rocks Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, if this is run multiple times then __slots__ will be added then trigger this if statement (in the old code). Is there a reason we can't store "we've processed this dataclass already" and avoid rerunning this code, if that's indeed the case? That might stop anything similar from happening.

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems worth looking into! Codex generated this commit and it seemed like a more correct version of the previously existing code that attempted to make this logic idempotent, so I opened the PR

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is documented in DataclassTransformer fwiw

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also #12762

Copy link
Collaborator

@A5rocks A5rocks Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see the other things have things like:

            and (
                "__match_args__" not in info.names or info.names["__match_args__"].plugin_generated
            )

(which is basically what happens here)

# This means we have a slots conflict.
# Class explicitly specifies a different `__slots__` field.
# And `@dataclass(slots=True)` is used.
Expand All @@ -478,14 +477,21 @@ def add_slots(
# does not have concrete `__slots__` defined. Ignoring.
return

generated_slots = {attr.name for attr in attributes}
info.slots = generated_slots

# Now, insert `.__slots__` attribute to class namespace:
slots_type = TupleType(
[self._api.named_type("builtins.str") for _ in generated_slots],
self._api.named_type("builtins.tuple"),
)
add_attribute_to_class(self._api, self._cls, "__slots__", slots_type)
add_attribute_to_class(
self._api,
self._cls,
"__slots__",
slots_type,
overwrite_existing=slots_defined_by_plugin,
)

def reset_init_only_vars(self, info: TypeInfo, attributes: list[DataclassAttribute]) -> None:
"""Remove init-only vars from the class and reset init var declarations."""
Expand Down
14 changes: 14 additions & 0 deletions test-data/unit/check-dataclasses.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,20 @@ class PublishedMessages:
left: int
[builtins fixtures/dataclasses.pyi]

[case testDataclassSlotsWithDeferredBase]
# flags: --python-version 3.10
from dataclasses import dataclass

@dataclass
class A(B): ...

@dataclass
class B: ...

@dataclass(slots=True)
class C: ...
[builtins fixtures/dataclasses.pyi]

[case testDataclassesAnyInherit]
from dataclasses import dataclass
from typing import Any
Expand Down