Skip to content
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

[mypyc] Don't load forward ref targets while setting up non-ext __annotations__ #14401

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
15 changes: 13 additions & 2 deletions mypyc/irbuild/classdef.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
TypeInfo,
is_class_var,
)
from mypy.types import ENUM_REMOVED_PROPS, Instance, get_proper_type
from mypy.types import ENUM_REMOVED_PROPS, Instance, UnboundType, get_proper_type
from mypyc.ir.class_ir import ClassIR, NonExtClassInfo
from mypyc.ir.func_ir import FuncDecl, FuncSignature
from mypyc.ir.ops import (
Expand Down Expand Up @@ -556,6 +556,7 @@ def add_non_ext_class_attr_ann(
get_type_info: Callable[[AssignmentStmt], TypeInfo | None] | None = None,
) -> None:
"""Add a class attribute to __annotations__ of a non-extension class."""
# FIXME: try to better preserve the special forms and type parameters of generics.
typ: Value | None = None
if get_type_info is not None:
type_info = get_type_info(stmt)
Expand All @@ -565,7 +566,17 @@ def add_non_ext_class_attr_ann(
if typ is None:
# FIXME: if get_type_info is not provided, don't fall back to stmt.type?
ann_type = get_proper_type(stmt.type)
if isinstance(ann_type, Instance):
if (
isinstance(stmt.unanalyzed_type, UnboundType)
and stmt.unanalyzed_type.original_str_expr is not None
):
# Annotation is a forward reference, so don't attempt to load the actual
# type and load the string instead.
#
# TODO: is it possible to determine whether a non-string annotation is
# actually a forward reference due to the __annotations__ future?
Copy link
Collaborator

Choose a reason for hiding this comment

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

If from __future__ import annotations is used, I think that we can always use string annotations, or whenever there is any doubt that type might contain forward references.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yeah you're right. I'll open an issue about hardening our __annotations__ handling overall when I have time.

typ = builder.load_str(stmt.unanalyzed_type.original_str_expr)
elif isinstance(ann_type, Instance):
typ = load_type(builder, ann_type.type, stmt.line)
else:
typ = builder.add(LoadAddress(type_object_op.type, type_object_op.src, stmt.line))
Expand Down
10 changes: 6 additions & 4 deletions mypyc/test-data/run-tuples.test
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ assert f(Sub(3, 2)) == 3
[case testNamedTupleClassSyntax]
from typing import Dict, List, NamedTuple, Optional, Tuple, Union

class ClassIR: pass

class FuncIR: pass

StealsDescription = Union[bool, List[bool]]
Expand All @@ -119,8 +117,12 @@ class Record(NamedTuple):
ordering: Optional[List[int]]
extra_int_constants: List[Tuple[int]]

# Make sure mypyc loads the annotation string for this forward reference.
# Ref: https://github.com/mypyc/mypyc/issues/938
class ClassIR: pass

[file driver.py]
from typing import Optional
from typing import ForwardRef, Optional
from native import ClassIR, FuncIR, Record

assert Record.__annotations__ == {
Expand All @@ -129,7 +131,7 @@ assert Record.__annotations__ == {
'is_borrowed': bool,
'hash': str,
'python_path': tuple,
'type': ClassIR,
'type': ForwardRef('ClassIR'),
'method': FuncIR,
'shadow_method': type,
'classes': dict,
Expand Down