Skip to content

[PEP 696] Fix swapping TypeVars with defaults. #19449

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
merged 12 commits into from
Oct 27, 2025
Merged

[PEP 696] Fix swapping TypeVars with defaults. #19449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
510a89d
initial commit + revert debugging
randolf-scholz Jul 15, 2025
4b34af1
added second test
randolf-scholz Jul 15, 2025
ee4dcc9
Merge branch 'python:master' into fix_typevar_default
randolf-scholz Jul 15, 2025
dc7f73a
Merge branch 'master' into fix_typevar_default
randolf-scholz Jul 17, 2025
665d3ff
Merge branch 'master' into fix_typevar_default
randolf-scholz Jul 19, 2025
70fbf96
Merge branch 'master' into fix_typevar_default
randolf-scholz Jul 27, 2025
14ece7b
Merge branch 'master' into fix_typevar_default
randolf-scholz Jul 28, 2025
fd366cc
Merge branch 'master' into fix_typevar_default
randolf-scholz Aug 4, 2025
76c0123
Merge branch 'master' into fix_typevar_default
randolf-scholz Sep 16, 2025
1a3957a
Merge branch 'master' into fix_typevar_default
randolf-scholz Sep 19, 2025
818175d
Merge branch 'master' into fix_typevar_default
randolf-scholz Oct 6, 2025
8a088dc
Merge branch 'master' into fix_typevar_default
randolf-scholz Oct 14, 2025
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
6 changes: 3 additions & 3 deletions mypy/expandtype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ def visit_type_var(self, t: TypeVarType) -> Type:
if (tvar_id := repl.id) in self.recursive_tvar_guard:
return self.recursive_tvar_guard[tvar_id] or repl
self.recursive_tvar_guard[tvar_id] = None
repl = repl.accept(self)
if isinstance(repl, TypeVarType):
repl.default = repl.default.accept(self)
repl.default = repl.default.accept(self)
Copy link
Member

@ilevkivskyi ilevkivskyi Oct 15, 2025

Choose a reason for hiding this comment

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

Types should be never modified in place. I know you didn't introduce it, but this should be fixed ASAP. (I think there is also a chance some of the problems you try to fix may be caused by this).

Copy link
Contributor Author

@randolf-scholz randolf-scholz Oct 15, 2025

Choose a reason for hiding this comment

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

Types should be never modified in place.

So, with copy_modified then? If stuff should never be modified in place, it surprises me that the variables are not annotated as Final in the first place.

Copy link
Contributor Author

@randolf-scholz randolf-scholz Oct 15, 2025

Choose a reason for hiding this comment

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

Hm, If I change this then now 2 tests fail: testTypeVarDefaultsClassRecursive1 and testTypeVarDefaultsClassRecursiveMultipleFiles

____________________________________________________________________________________________________________________________________ testTypeVarDefaultsClassRecursive1 _____________________________________________________________________________________________________________________________________
[gw0] linux -- Python 3.13.8 /home/rscholz/github/CONTRIBUTING/mypy/.venv/bin/python3
data: /home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test:451:
Failed: Unexpected type checker output (/home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test, line 451)
------------------------------------------------------------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------------------------------------------------------------
Expected:
  main:16: note: Revealed type is "__main__.ClassD1[builtins.str, builtins.str]"
  main:17: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]"
  main:18: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]"
  main:21: note: Revealed type is "__main__.ClassD1[builtins.str, builtins.str]" (diff)
  main:23: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]"
  main:25: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]"
  main:35: note: Revealed type is "__main__.ClassD2[builtins.str, builtins.str, builtins.str]"
  main:36: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, builtins.int]"
  main:37: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]"
  main:38: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]"
  main:41: note: Revealed type is "__main__.ClassD2[builtins.str, builtins.str, builtins.str]" (diff)
  main:43: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, builtins.int]" (diff)
  main:45: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]"
  main:47: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]"
  main:56: note: Revealed type is "__main__.ClassD3[builtins.str, builtins.list[builtins.str]]"
  ...
  main:58: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]"
  main:63: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.list[builtins.int]]"
  main:65: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]"
Actual:
  main:16: note: Revealed type is "__main__.ClassD1[builtins.str, builtins.str]"
  main:17: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]"
  main:18: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]"
  main:21: note: Revealed type is "__main__.ClassD1[builtins.str, T1`1 = builtins.str]" (diff)
  main:23: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]"
  main:25: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]"
  main:35: note: Revealed type is "__main__.ClassD2[builtins.str, builtins.str, builtins.str]"
  main:36: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, builtins.int]"
  main:37: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]"
  main:38: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]"
  main:41: note: Revealed type is "__main__.ClassD2[builtins.str, T1`1 = builtins.str, T2`2 = T1`1 = builtins.str]" (diff)
  main:43: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, T2`2 = builtins.int]" (diff)
  main:45: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]"
  main:47: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]"
  main:56: note: Revealed type is "__main__.ClassD3[builtins.str, builtins.list[builtins.str]]"
  ...
  main:58: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]"
  main:63: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.list[builtins.int]]"
  main:65: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]"

Alignment of first line difference:
  E: ..._.ClassD1[builtins.str, builtins.str]"
  A: ..._.ClassD1[builtins.str, T1`1 = builtins.str]"
                                ^
Update the test output using --update-data (implies -n0; you can additionally use the -k selector to update only specific tests)
______________________________________________________________________________________________________________________________ testTypeVarDefaultsClassRecursiveMultipleFiles _______________________________________________________________________________________________________________________________
[gw0] linux -- Python 3.13.8 /home/rscholz/github/CONTRIBUTING/mypy/.venv/bin/python3
data: /home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test:518:
Failed: Unexpected type checker output (/home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test, line 518)
------------------------------------------------------------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------------------------------------------------------------
Expected:
  main:15: note: Revealed type is "__main__.ClassG1[builtins.int, builtins.int]"
  main:16: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]"
  main:17: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]"
  main:20: note: Revealed type is "__main__.ClassG1[builtins.int, builtins.int]" (diff)
  main:22: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]"
  main:24: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]"
Actual:
  main:15: note: Revealed type is "__main__.ClassG1[builtins.int, builtins.int]"
  main:16: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]"
  main:17: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]"
  main:20: note: Revealed type is "__main__.ClassG1[builtins.int, T2`1 = builtins.int]" (diff)
  main:22: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]"
  main:24: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]"

Alignment of first line difference:
  E: ..._.ClassG1[builtins.int, builtins.int]"
  A: ..._.ClassG1[builtins.int, T2`1 = builtins.int]"
                                ^
Update the test output using --update-data (implies -n0; you can additionally use the -k selector to update only specific tests)
========================================================================================================================================== short test summary info ==========================================================================================================================================
FAILED mypy/test/testcheck.py::TypeCheckSuite::check-typevar-defaults.test::testTypeVarDefaultsClassRecursive1
FAILED mypy/test/testcheck.py::TypeCheckSuite::check-typevar-defaults.test::testTypeVarDefaultsClassRecursiveMultipleFiles

I noticed that TypeVarType.__hash__ is given by hash((self.id, self.upper_bound, tuple(self.values))), so changing default does not change the hash.

Copy link
Member

@ilevkivskyi ilevkivskyi Oct 15, 2025

Choose a reason for hiding this comment

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

Yes. For historical reasons some types are constructed "gradually", so we can't make type attributes final. Maybe we will at some point, but that would be a big refactoring. For now, mypy contributors should remember a simple rule: types always new, symbols always original.

Copy link
Member

@ilevkivskyi ilevkivskyi Oct 15, 2025

Choose a reason for hiding this comment

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

If I change this then now 2 tests fail

Oh well, it means there is another bug. TBH I am not sure if default should be part of the hash conceptually (and what it even means to have a type variable with same id, but a different default). IMO the support for type variable defaults may need to be re-implemented from scratch, as it is problematic on a conceptual level: regular default is not a property of an argument, it is a property of a function; same for type variables, their defaults are not property of type variable type itself, but of the function/class that uses them.

Copy link
Contributor Author

@randolf-scholz randolf-scholz Oct 15, 2025

Choose a reason for hiding this comment

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

So the problem seems to be with getting the correct ID's. In the test case, we have class D[T1=str, T2=T1].

>>> self.variables 
{1: T1`604 = builtins.str, 2: T2`605 = T1`1 = builtins.str}
>>> self.recursive_tvar_guard
{604: T1`604 = builtins.str, 605: None}
>>> repl.default = repl.default.accept(self)
>>> self.variables 
{1: T1`604 = builtins.str, 2: T2`605 = T1`604 = builtins.str}
>>> self.recursive_tvar_guard
{604: T1`604 = builtins.str, 605: None}

Modifying repl.default in-place ensures that in the ExpandTypeVisitor.variables dictionary, the default of the dependent type variable gets updated to the new ID.

Copy link
Member

@ilevkivskyi ilevkivskyi Oct 15, 2025

Choose a reason for hiding this comment

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

TBH I don't understand why do we need to handle the defaults in expand_type in the first place. As I explained, the default is not a property of a type variable, it is a property of a class/function, so the caller (e.g. applytype etc) should always provide a complete type var -> replacement mapping. So that expand_type() should not be even allowed to look at the defaults.

Yeah, I just looked at the original PR that added this logic #16878, it looks broken. The full complete mapping should be computed either in:

  • semanal/typeanal for "static" application like x: C[int]
  • in applytype for "dynamic" application like x = C[int]()

before we even call expand_type. I think the only right way to do it is this way.

Copy link
Contributor Author

@randolf-scholz randolf-scholz Oct 24, 2025

Choose a reason for hiding this comment

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

I am not sure how to fix this at the moment, but given that this PR

  1. Fixes two very annoying bugs with PEP696 checking (essentially prevents typing bijections with defaults)
  2. Doesn't introduce inplace modification -- it was already there before (all we really do is make sure to update the default a-priori, which is crucial e.g. when the second typevar defaults to the first one and we expand the first one.)
  3. This issue probably needs to be addressed elsewhere

Maybe this can still be merged as-is?

Copy link
Member

@ilevkivskyi ilevkivskyi Oct 27, 2025

Choose a reason for hiding this comment

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

The downside is that this is papering over the problem instead of fixing the real root cause. But anyway, if you feel strongly about this, I can merge this, it will not be much more work to axe this all later (and the tests are useful).

expanded = repl.accept(self) # Note: `expanded is repl` may be true.
repl = repl if isinstance(expanded, TypeVarType) else expanded
self.recursive_tvar_guard[tvar_id] = repl
return repl

Expand Down
32 changes: 32 additions & 0 deletions test-data/unit/check-typevar-defaults.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,38 @@ def func_c4(
reveal_type(m) # N: Revealed type is "__main__.ClassC4[builtins.int, builtins.float]"
[builtins fixtures/tuple.pyi]

[case testTypeVarDefaultsSwap]
from typing import TypeVar, Generic

T = TypeVar("T")
X = TypeVar("X", default=object)
Y = TypeVar("Y", default=object)


class Foo(Generic[T, Y]):
def test(self) -> None:
reveal_type( Foo[Y, T]() ) # N: Revealed type is "__main__.Foo[Y`2 = builtins.object, T`1]"


class Bar(Generic[X, Y]):
def test(self) -> None:
reveal_type( Bar[Y, X]() ) # N: Revealed type is "__main__.Bar[Y`2 = builtins.object, X`1 = builtins.object]"


[case testTypeVarDefaultsSwap2]
from typing import TypeVar, Generic

X = TypeVar("X", default=object)
Y = TypeVar("Y", default=object)
U = TypeVar("U", default=object)
V = TypeVar("V", default=object)

class Transform(Generic[X, Y]):
def invert(self) -> "Transform[Y, X]": ...

class Foo(Transform[U, V], Generic[U, V]):
def invert(self) -> "Foo[V, U]": ...

[case testTypeVarDefaultsClassRecursive1]
# flags: --disallow-any-generics
from typing import Generic, TypeVar, List
Expand Down