Type variable not substituted in a nested function within a generic function

[JukkaL]

Mypy reports a bogus error for the following program (reported by Alex Allain):

from typing import TypeVar

T = TypeVar('T', int)
def outer(a):
    # type: (T) -> None
    def inner(b):
        # type: (T) -> None
        pass

    inner(a)  # Argument 1 to "inner" has incompatible type "int"; expected "T"

I suspect that mypy doesn't substitute T in the nested function when type checking the outer function. Generalizing substitution to nested functions might be enough to fix this.

[gvanrossum]

Wow, this may also explain some problems with decorators that I've seen.

[rwbarton]

The type T does get substituted in the definition of the nested function. The problem is that the call to inner still points to the original, untransformed function of type (T) -> None. (Note that it is not a generic function, because T is bound by the surrounding definition.)

I tried rewriting function references in treetransform in rwbarton@c474fb4, and it does basically work correctly, but I'm not thrilled with that approach. I'm wondering whether instead of doing all this expand_func stuff, we could just maintain a dictionary in the checker of type variables that have been assigned values, and use it to transform types that come out of visit_name_expr and wherever else types are input to the checker, like type checking a cast. (Part of the motivation here is that expand_func is the only user of treetransform.) @JukkaL what do you think?

[JukkaL]

Sounds reasonable. The current approach was designed for a compiler and
it's not important to retain it any more. But I'd only replace it if it
isn't a lot of extra work.
On Tue, Apr 19, 2016 at 20:38 rwbarton [email protected] wrote:

The type T does get substituted in the definition of the nested function.
The problem is that the call to inner still points to the original,
untransformed function of type (T) -> None. (Note that it is not a
generic function, because T is bound by the surrounding definition.)

I tried rewriting function references in treetransform in rwbarton/mypy@c474fb4
rwbarton@c474fb4,
and it does basically work correctly, but I'm not thrilled with that
approach. I'm wondering whether instead of doing all this expand_func
stuff, we could just maintain a dictionary in the checker of type variables
that have been assigned values, and use it to transform types that come out
of visit_name_expr and wherever else types are input to the checker, like
type checking a cast. (Part of the motivation here is that expand_func is
the only user of treetransform.) @JukkaL https://github.com/JukkaL what
do you think?

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#1323 (comment)

[JukkaL]

I guess the tradeoff is that we'd need to find all places in the type
checker where types need to be translated, and it sounds a little fragile
as we could easily miss a place. Though the current transform is fragile as
well.
On Tue, Apr 19, 2016 at 22:53 Jukka Lehtosalo [email protected] wrote:

Sounds reasonable. The current approach was designed for a compiler and
it's not important to retain it any more. But I'd only replace it if it
isn't a lot of extra work.
On Tue, Apr 19, 2016 at 20:38 rwbarton [email protected] wrote:

The type T does get substituted in the definition of the nested
function. The problem is that the call to inner still points to the
original, untransformed function of type (T) -> None. (Note that it is
not a generic function, because T is bound by the surrounding
definition.)

I tried rewriting function references in treetransform in rwbarton@
c474fb4
rwbarton@c474fb4,
and it does basically work correctly, but I'm not thrilled with that
approach. I'm wondering whether instead of doing all this expand_func
stuff, we could just maintain a dictionary in the checker of type variables
that have been assigned values, and use it to transform types that come out
of visit_name_expr and wherever else types are input to the checker,
like type checking a cast. (Part of the motivation here is that
expand_func is the only user of treetransform.) @JukkaL
https://github.com/JukkaL what do you think?

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#1323 (comment)

[rwbarton]

I guess the tradeoff is that we'd need to find all places in the type
checker where types need to be translated, and it sounds a little fragile
as we could easily miss a place. Though the current transform is fragile as
well.

These were exactly my thoughts as well. How does this sound as a plan?

• Maintain a set of "bound" type variables in the checker. These are the type variables of the generic classes and generic functions that surround the current location of the checker. (Type variables with value restrictions don't count, because they will have been substituted by one of their values already, and are no longer generic.)
• In TypeChecker.accept, verify that the result type does not contain any occurrences of unbound variables. This check is probably too expensive to do in normal operation, so we could turn it on for the test suite only.

I implemented most of this in rwbarton@2f94f22; it raises errors on the program in this issue (x/T1323orig.py:10: error: Unexpected free type variable in def (b: T-1)`), and also in the situation that arose in #1317. It also raises errors on a couple of tests, which look like they might indicate other legitimate mypy bugs.

With this kind of linting in place, we should be able to implement the alternate approach without missing anywhere that we need to substitute a type.

[rwbarton]

Hmm, it turns out that copying the syntax tree of the function being type checked is important in cases like this one from the test suite.

T = TypeVar('T', int, str)
def f(x: T) -> T:
    if isinstance(x, int):
        y = 1
    else:
        y = ''
    return y

If we don't copy the syntax tree of f, then when we type check f the first time with T = int, we infer that y has type int and save this information in the Var for y in set_inferred_type. Then when we check f the second time with T = str, we still have this information so mypy thinks y is supposed to be an int still.

[JukkaL]

Ah yes, I didn't remember about that. Do you think that we should go back to fixing treetransform? The alternative could be to remember all variable nodes with modified inferred types (in affected functions) and clear them afterwards, but that would be pretty hacky. Also, I wonder if there is some other state that gets modified during type checking.

[JukkaL]

Moved this to 0.4.x.

[rwbarton]

Do you think that we should go back to fixing treetransform?

Yeah, I think so. I submitted a PR.