Skip to content

Fix mypyc crash with enum type aliases #18725

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.

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 3 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion mypyc/irbuild/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def build_ir(

for module in modules:
# First pass to determine free symbols.
pbv = PreBuildVisitor(errors, module, singledispatch_info.decorators_to_remove)
pbv = PreBuildVisitor(errors, module, singledispatch_info.decorators_to_remove, types)
module.accept(pbv)

# Construct and configure builder objects (cyclic runtime dependency).
Expand Down
20 changes: 20 additions & 0 deletions mypyc/irbuild/missingtypevisitor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from __future__ import annotations

from mypy.nodes import Expression, Node
from mypy.traverser import ExtendedTraverserVisitor
from mypy.types import AnyType, Type, TypeOfAny


class MissingTypesVisitor(ExtendedTraverserVisitor):
"""AST visitor that can be used to add any missing types as a generic AnyType."""

def __init__(self, types: dict[Expression, Type]) -> None:
super().__init__()
self.types: dict[Expression, Type] = types

def visit(self, o: Node) -> bool:
if isinstance(o, Expression) and o not in self.types:
self.types[o] = AnyType(TypeOfAny.special_form)

# If returns True, will continue to nested nodes.
return True
13 changes: 13 additions & 0 deletions mypyc/irbuild/prebuildvisitor.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from mypy.nodes import (
AssignmentStmt,
Block,
Decorator,
Expression,
Expand All @@ -16,7 +17,9 @@
Var,
)
from mypy.traverser import ExtendedTraverserVisitor
from mypy.types import Type
from mypyc.errors import Errors
from mypyc.irbuild.missingtypevisitor import MissingTypesVisitor


class PreBuildVisitor(ExtendedTraverserVisitor):
Expand All @@ -39,6 +42,7 @@ def __init__(
errors: Errors,
current_file: MypyFile,
decorators_to_remove: dict[FuncDef, list[int]],
types: dict[Expression, Type],
) -> None:
super().__init__()
# Dict from a function to symbols defined directly in the
Expand Down Expand Up @@ -82,11 +86,20 @@ def __init__(

self.current_file: MypyFile = current_file

self.missing_types_visitor = MissingTypesVisitor(types)

def visit(self, o: Node) -> bool:
if not isinstance(o, Import):
self._current_import_group = None
return True

def visit_assignment_stmt(self, stmt: AssignmentStmt) -> None:
# These are cases where mypy may not have types for certain expressions,
# but mypyc needs some form type to exist.
if stmt.is_alias_def:
stmt.rvalue.accept(self.missing_types_visitor)
return super().visit_assignment_stmt(stmt)

def visit_block(self, block: Block) -> None:
self._current_import_group = None
super().visit_block(block)
Expand Down
10 changes: 10 additions & 0 deletions mypyc/test-data/irbuild-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -1335,3 +1335,13 @@ def outer():
if True:
class OtherInner: # E: Nested class definitions not supported
pass

[case testEnumClassAlias]
from enum import Enum
from typing import Literal, Union

class SomeEnum(Enum):
AVALUE = "a"

ALIAS = Literal[SomeEnum.AVALUE]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also test nested Literal types that aren't nested within an IndexExpr, such as Literal[<...>] | None. You must target 3.10 to enable the new style union type syntax. I think you can add _python3_10 suffix to the test case name to only run it on 3.10 and later.

Test explicit type aliases (e.g. AliasExplicit: TypeAlias = Literal[<...>]).

Test type alias statements (e.g. type Alias = Literal[<...>]). Since it only works on 3.12 and later, it's best to add it to testPEP695Basics in mypyc/test-data/run-python312.test. It already has some similar test cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Literal[<...>] | None didn't seem to work:

[...]
  File "/home/svalentin/src/mypy-svalentin/mypy/checkexpr.py", line 4890, in alias_type_in_runtime_context
    return self.chk.named_generic_type("types.UnionType", item.items)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/svalentin/src/mypy-svalentin/mypy/checker.py", line 7285, in named_generic_type
    info = self.lookup_typeinfo(name)
  File "/home/svalentin/src/mypy-svalentin/mypy/checker.py", line 7292, in lookup_typeinfo
    sym = self.lookup_qualified(fullname)
  File "/home/svalentin/src/mypy-svalentin/mypy/checker.py", line 7370, in lookup_qualified
    n = self.modules[parts[0]]
        ~~~~~~~~~~~~^^^^^^^^^^
KeyError: 'types'

Even with [typing fixtures/typing-full.pyi].
I haven't seen any other test use the X | Y form of type definition. I think we should leave the supporting of X | Y types as a separate issue.
I'll add it in run-python312.test since it seems to be fine there.

Separately, is there any reason why we can't have Test_python3_12 tests and we must use run-python312.test?
I think we should update the testing README.md with the differences between Test_python3_12, run-python312.test and # flags: --python-version 3.12. I realize that the last one is just for mypy, not for mypyc, but might be worth documenting the differences in both set of tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, it's fine to support X | Y in a follow-up issue. It might be a stub issue -- maybe adding import types to the test case would help to simulate how the real typeshed stubs work.

ALIAS2 = Union[Literal[SomeEnum.AVALUE], None]
10 changes: 9 additions & 1 deletion mypyc/test-data/run-python312.test
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[case testPEP695Basics]
from typing import Any, TypeAliasType, cast
from enum import Enum
from typing import Any, Literal, TypeAliasType, cast

from testutil import assertRaises

Expand Down Expand Up @@ -188,6 +189,13 @@ type R = int | list[R]
def test_recursive_type_alias() -> None:
assert isinstance(R, TypeAliasType)
assert getattr(R, "__value__") == (int | list[R])

class SomeEnum(Enum):
AVALUE = "a"

type EnumLiteralAlias1 = Literal[SomeEnum.AVALUE]
type EnumLiteralAlias2 = Literal[SomeEnum.AVALUE] | None
EnumLiteralAlias3 = Literal[SomeEnum.AVALUE] | None
[typing fixtures/typing-full.pyi]

[case testPEP695GenericTypeAlias]
Expand Down