Skip to content

Couple obvious optimizations for type aliases #8099

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 2 commits into from
Dec 7, 2019
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
5 changes: 4 additions & 1 deletion mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2741,7 +2741,7 @@ def f(x: B[T]) -> T: ... # without T, Any would be used here
line and column: Line an column on the original alias definition.
"""
__slots__ = ('target', '_fullname', 'alias_tvars', 'no_args', 'normalized',
'line', 'column', 'assuming', 'assuming_proper', 'inferring')
'line', 'column', '_is_recursive')

def __init__(self, target: 'mypy.types.Type', fullname: str, line: int, column: int,
*,
Expand All @@ -2755,6 +2755,9 @@ def __init__(self, target: 'mypy.types.Type', fullname: str, line: int, column:
self.alias_tvars = alias_tvars
self.no_args = no_args
self.normalized = normalized
# This attribute is manipulated by TypeAliasType. If non-None,
# it is the cached value.
self._is_recursive = None # type: Optional[bool]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment (mention that if non-None, this the cached value). Maybe also mention that this is manipulated by TypeAliasType, as otherwise a reader may be confused as not much is done here with this.

super().__init__(line, column)

@property
Expand Down
3 changes: 3 additions & 0 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2514,6 +2514,9 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
# so we need to replace it with non-explicit Anys.
if not has_placeholder(res):
res = make_any_non_explicit(res)
# Note: with the new (lazy) type alias representation we only need to set no_args to True
# if the expected number of arguments is non-zero, so that aliases like A = List work.
# However, eagerly expanding aliases like Text = str is a nice performance optimization.
no_args = isinstance(res, Instance) and not res.args # type: ignore
fix_instance_types(res, self.fail, self.note)
alias_node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column,
Expand Down
14 changes: 8 additions & 6 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,13 @@ class Node:
can be represented in a tree-like manner.
"""

__slots__ = ('alias', 'args', 'line', 'column', 'type_ref', '_is_recursive')
__slots__ = ('alias', 'args', 'line', 'column', 'type_ref')

def __init__(self, alias: Optional[mypy.nodes.TypeAlias], args: List[Type],
line: int = -1, column: int = -1) -> None:
self.alias = alias
self.args = args
self.type_ref = None # type: Optional[str]
self._is_recursive = None # type: Optional[bool]
super().__init__(line, column)

def _expand_once(self) -> Type:
Expand Down Expand Up @@ -212,10 +211,13 @@ def expand_all_if_possible(self) -> Optional['ProperType']:

@property
def is_recursive(self) -> bool:
if self._is_recursive is not None:
return self._is_recursive
is_recursive = self.expand_all_if_possible() is None
self._is_recursive = is_recursive
assert self.alias is not None, 'Unfixed type alias'
is_recursive = self.alias._is_recursive
if is_recursive is None:
is_recursive = self.expand_all_if_possible() is None
# We cache the value on the underlying TypeAlias node as an optimization,
# since the value is the same for all instances of the same alias.
self.alias._is_recursive = is_recursive
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment (we cache the value as an optimization).

return is_recursive

def can_be_true_default(self) -> bool:
Expand Down