Skip to content

Break cyclic import between mypy.types and mypy.expandtype #21264

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

Closed
rennf93 wants to merge 1 commit into from
+15 −19
Closed

Break cyclic import between mypy.types and mypy.expandtype #21264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions mypy/expandtype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,3 +660,16 @@ def remove_trivial(types: Iterable[Type]) -> list[Type]:
if removed_none:
return [NoneType()]
return [UninhabitedType()]


class InstantiateAliasVisitor(ExpandTypeVisitor):
def visit_union_type(self, t: UnionType) -> Type:
# Unlike regular expand_type(), we don't do any simplification for unions,
# not even removing strict duplicates. There are three reasons for this:
# * get_proper_type() is a very hot function, even slightest slow down will
# cause a perf regression
# * We want to preserve this historical behaviour, to avoid possible
# regressions
# * Simplifying unions may (indirectly) call get_proper_type(), causing
# infinite recursion.
return mypy.type_visitor.TypeTranslator.visit_union_type(self, t)
21 changes: 2 additions & 19 deletions mypy/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ def _expand_once(self) -> Type:
):
mapping[tvar.id] = sub

from mypy.expandtype import InstantiateAliasVisitor
Copy link
Copy Markdown
Member

@ilevkivskyi ilevkivskyi Apr 19, 2026

Choose a reason for hiding this comment

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

Function-level imports are slow in mypyc, and this is very hot code.

Copy link
Copy Markdown
Author

@rennf93 rennf93 Apr 19, 2026

Choose a reason for hiding this comment

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

If it's a hard no, I think there's a couple alternatives:

  1. Module-level cached binding in mypy/types.py:
    _InstantiateAliasVisitor: "type[InstantiateAliasVisitor] | None" = None
# ... inside TypeAliasType._partial_expansion:
global _InstantiateAliasVisitor

if _InstantiateAliasVisitor is None:
    from mypy.expandtype import InstantiateAliasVisitor as _V
    _InstantiateAliasVisitor = _V
visitor = _InstantiateAliasVisitor(...)

First call pays the import; rest are a module-global load. Preserves the no-cycle property because the import still happens lazily.

  1. Move the module-level back-import to the end of mypy/types.py (after all class definitions) and rely on InstantiateAliasVisitor being defined early in expandtype.py. This only works if InstantiateAliasVisitor's body and its dependencies are bound above the from mypy.types import ... line in expandtype.py (currently line 8) — otherwise the cycle reappears with a different name. I'd need to check the class's dependency chain carefully; if it's doable it matches the pre-existing structural pattern and hot-path cost goes to zero.

I can benchmark all three (current PR, 1, 2) on a realistic mypy self-check run and post numbers. If 1 is within noise I'll switch to 1); if 2) is doable I'll switch to 2). Any preferences up-front?


return self.alias.target.accept(InstantiateAliasVisitor(mapping))

@property
Expand Down Expand Up @@ -4441,22 +4443,3 @@ def write_type_map(data: WriteBuffer, value: dict[str, Type]) -> None:
for key in sorted(value):
write_str_bare(data, key)
value[key].write(data)


# This cyclic import is unfortunate, but to avoid it we would need to move away all uses
# of get_proper_type() from types.py. Majority of them have been removed, but few remaining
# are quite tricky to get rid of, but ultimately we want to do it at some point.
from mypy.expandtype import ExpandTypeVisitor


class InstantiateAliasVisitor(ExpandTypeVisitor):
def visit_union_type(self, t: UnionType) -> Type:
# Unlike regular expand_type(), we don't do any simplification for unions,
# not even removing strict duplicates. There are three reasons for this:
# * get_proper_type() is a very hot function, even slightest slow down will
# cause a perf regression
# * We want to preserve this historical behaviour, to avoid possible
# regressions
# * Simplifying unions may (indirectly) call get_proper_type(), causing
# infinite recursion.
return TypeTranslator.visit_union_type(self, t)
Loading