Skip to content

Rewrite references to inner functions in treetransform #1791

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

Closed
wants to merge 1 commit into from
Closed
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
38 changes: 36 additions & 2 deletions mypy/treetransform.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
YieldExpr, ExecStmt, Argument, BackquoteExpr
)
from mypy.types import Type, FunctionLike, Instance
from mypy.traverser import TraverserVisitor
from mypy.visitor import NodeVisitor


Expand All @@ -36,7 +37,7 @@ class TransformVisitor(NodeVisitor[Node]):

* Do not duplicate TypeInfo nodes. This would generally not be desirable.
* Only update some name binding cross-references, but only those that
refer to Var nodes, not those targeting ClassDef, TypeInfo or FuncDef
refer to Var or FuncDef nodes, not those targeting ClassDef or TypeInfo
nodes.
* Types are not transformed, but you can override type() to also perform
type transformation.
Expand All @@ -48,6 +49,7 @@ def __init__(self) -> None:
# There may be multiple references to a Var node. Keep track of
# Var translations using a dictionary.
self.var_map = {} # type: Dict[Var, Var]
self.func_map = {} # type: Dict[FuncDef, FuncDef]

def visit_mypy_file(self, node: MypyFile) -> Node:
# NOTE: The 'names' and 'imports' instance variables will be empty!
Expand Down Expand Up @@ -98,6 +100,18 @@ def copy_argument(self, argument: Argument) -> Argument:

def visit_func_def(self, node: FuncDef) -> FuncDef:
# Note that a FuncDef must be transformed to a FuncDef.

# These contortions are needed to handle the case of recursive
# references inside the function being transformed.
# Set up empty nodes for references within this function
# to other functions defined inside it.
# Don't create an entry for this function itself though,
# since we want self-references to point to the original
# function if this is the top-level node we are transforming.
init = FuncMapInitializer(self)
for stmt in node.body.body:
stmt.accept(init)

new = FuncDef(node.name(),
[self.copy_argument(arg) for arg in node.arguments],
self.block(node.body),
Expand All @@ -113,7 +127,13 @@ def visit_func_def(self, node: FuncDef) -> FuncDef:
new.is_class = node.is_class
new.is_property = node.is_property
new.original_def = node.original_def
return new

if node in self.func_map:
result = self.func_map[node]
result.__dict__ = new.__dict__
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is worth explaining, as this is a little unusual. In particular, this only works if the classes of the instances are identical (no subclasses)?

return result
else:
return new

def visit_func_expr(self, node: FuncExpr) -> Node:
new = FuncExpr([self.copy_argument(arg) for arg in node.arguments],
Expand Down Expand Up @@ -330,6 +350,9 @@ def copy_ref(self, new: RefExpr, original: RefExpr) -> None:
target = original.node
if isinstance(target, Var):
target = self.visit_var(target)
elif isinstance(target, FuncDef):
if target in self.func_map:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could use self.func_map.get(target, target) here for a minor simplification.

target = self.func_map[target]
new.node = target
new.is_def = original.is_def

Expand Down Expand Up @@ -521,3 +544,14 @@ def types(self, types: List[Type]) -> List[Type]:

def optional_types(self, types: List[Type]) -> List[Type]:
return [self.optional_type(type) for type in types]


class FuncMapInitializer(TraverserVisitor):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add docstring explaining what this does.

def __init__(self, transformer: TransformVisitor) -> None:
self.transformer = transformer

def visit_func_def(self, node: FuncDef) -> None:
if node not in self.transformer.func_map:
self.transformer.func_map[node] = FuncDef(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs some explanation.

node.name(), node.arguments, node.body, None)
super().visit_func_def(node)
30 changes: 30 additions & 0 deletions test-data/unit/check-typevar-values.test
Original file line number Diff line number Diff line change
Expand Up @@ -479,3 +479,33 @@ a = g
b = g
b = g
b = f # E: Incompatible types in assignment (expression has type Callable[[T], T], variable has type Callable[[U], U])

[case testInnerFunctionWithTypevarValues]
from typing import TypeVar
T = TypeVar('T', int, str)
U = TypeVar('U', int, str)
def outer(x: T) -> T:
def inner(y: T) -> T:
return x
def inner2(y: U) -> U:
return y
inner(x)
inner(3) # E: Argument 1 to "inner" has incompatible type "int"; expected "str"
inner2(x)
inner2(3)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also try calling with an invalid argument type?

outer(3)
return x
[out]
main: note: In function "outer":

[case testInnerFunctionMutualRecursionWithTypevarValues]
from typing import TypeVar
T = TypeVar('T', int, str)
def outer(x: T) -> T:
def inner1(y: T) -> T:
return inner2(y)
def inner2(y: T) -> T:
return inner1('a') # E: Argument 1 to "inner1" has incompatible type "str"; expected "int"
return inner1(x)
[out]
main: note: In function "inner2":