Remove non-existing _DT from ordered dataclass

[guitvcer]

(Explain how this PR changes mypy.)

Fixes #18811

Remove _DT attribute that does not exist in runtime from ordered dataclass.

[sterliakov]

Won't this still keep creating _DT on @dataclass(eq=True)? The problematic bit is actually creating a fake TypeVarExpr and putting it in dataclass' symbol table.

Furthermore, this will actually break @dataclass(eq=False, order=True) definitions, because below self._api.fail() does not abort early, we'll still create methods referencing non-existent _DT, maybe leading to some crashes down the road.

[guitvcer]

Okay, I removed _DT from @dataclass(eq=True).

But I don't actually understand about @dataclass(eq=False, order=True) definitions, newbie in mypy:)

[sterliakov]

Okay, this typevar was stored in class' symbol table since #6515. The question is how come that all our tests pass with this (rather huge) block removed, and there are zero primer hits. Note that attrs plugin and namedtuple semanal does the same, adding a typevar node to the modified class.

I feel like this will eventually fail in incremental mode, trying to consume existing __lt__ of some dataclass without corresponding TypeVar node. Using TypeVarType that doesn't have corresponding tvar node somewhere is just begging for trouble, but somehow I can't quickly create a reproducer that crashes readily. Can we really just get rid of it like this?..

[sobolevn]

I have no idea why info.names[SELF_TVAR_NAME] = should be used here. It is either a very clever hack, or just a bug.

All evidences show that this is likely the former.

[sobolevn]

Tests like

mypy/test-data/unit/check-dataclasses.test

Line 618 in a5e4c77

[case testDataclassOrdering]

give me extra confidence.

[sobolevn]

Tagging @ilevkivskyi as the code author

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[ilevkivskyi]

It was not a hack, and I don't think this PR is safe. This code creates a consistent structure that is identical to what would happen for a Python code generated for the dataclass:

class Example:
    _DT = TypeVar("_DT")
    def __lt__(self: _DT, other: _DT) -> bool: ...

Creating a TypeVarType out of thin air without a corresponding TypeVarExpr may cause crashes in subtle scenarios, I guess it doesn't crash immediately because we special-case plugin-generated nodes in most places, but some code paths still may rely on a consistent structure.

[sobolevn]

@ilevkivskyi thank you for the explanation! Then, we need to special case this in stubtest.

[sterliakov]

That means we need to explicitly treat this attribute (and two other such typevars) as nonexistent in checkmember (both instance and class access) and, probably, stubtest. Stupid question: can we fake per-method PEP695 type parameters instead? Those shouldn't need a class-level typevar node, so should work without extra workarounds? Or, if not, create a typevar with a name that isn't a valid identifier (like `_DT-1`)? That would at least avoid extra checks checkmember since such a member can't be accessed as an attribute from code (and it's obscure enough to not check if called via `getattr`)

…

On Fri, Apr 11, 2025, 11:34 Ivan Levkivskyi ***@***.***> wrote: ***@***.**** requested changes on this pull request. It was not a hack, and I don't think this PR is safe. This code creates a consistent structure that is identical to what would happen for a Python code generated for the dataclass: class Example: _DT = TypeVar("_DT") def __lt__(self: _DT, other: _DT) -> bool: ... Creating a TypeVarType out of thin air without a corresponding TypeVarExpr may cause crashes in subtle scenarios, I guess it doesn't crash immediately because we special-case plugin-generated nodes in most places, but some code paths still may rely on a consistent structure. — Reply to this email directly, view it on GitHub <#18846 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMBQIREY6ONYAPK2ZYUHLI32Y6EDZAVCNFSM6AAAAABZ4D7PSSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDONJZG42DEMBQGQ> . You are receiving this because you commented.Message ID: ***@***.***>

[ilevkivskyi]

can we fake per-method PEP695 type parameters instead

Those actually create TypeVarExprs as well under the hood (and add them to symbol tables), but it may be better because they are only added to the method locals IIUC. That said I don't think it is actually worth the effort, these fake type vars have been there for namedtuples, attrs, and dataclasses for years, and no-one complained.

[sterliakov]

Well, if someone is interested in making such a change, I think it's justified: now mypy believes that some dataclasses have a _DT attribute which would be a rather funny problem to debug. (I'm personally fine with just ignoring those typevars in stubtest too)