Fix meta level on member access

[ilevkivskyi]

Fixes #2804

The problem is that analize_member_access calls freshen_function_type_vars if the member is a function (i.e. method). This is necessary to not mix the type variables during type inference of generic methods inside generic functions. However, if the method does not participate in type inference, type variables are left in meta_level = 1 state. This causes the error, since the types could not be serialized in the middle of type inference.

In this PR I propose to restore meta_level = 0 on member access, this is safe, since anyway, check_call always calls freshen_function_type_vars on callee (for generic functions, generic methods, etc).

[JukkaL]

Can you move this to Dict in fixture/dict.pyi? We try to keep the typing.pyi stub pretty minimal.

[ilevkivskyi]

OK, sure, I moved this to dict.pyi in a new commit.

[JukkaL]

If we just switch the meta level, I think that we can get an id conflict between type variables in the 0 level if we happen to be in a generic function which has some additional type variables in scope. Not sure what's the best way to work around this. What about serializing the meta levels of type variables as well?

[ilevkivskyi]

we can get an id conflict between type variables in the 0 level if we happen to be in a generic function which has some additional type variables in scope

I was also thinking about such possibility, but I didn't manage to trigger this error so I thought maybe I was wrong. Now you are also saying this and I am worried again. I need to think more about this tomorrow, maybe there is a way to exclude such possibility.

What about serializing the meta levels of type variables as well?

I think that serialazing type variables with meta_level == 1 is not a good idea. As I understand, this means that they are in process of being inferred, while serialazing is used for completely analysed modules, or am I wrong?

[JukkaL]

Actually, your original approach actually seems to be fine, since we are should always freshen the type variables before using them in type inference, so I think that there can be no conflict.

[ilevkivskyi]

@gvanrossum What does the new label blocked mean? It looks so scary.

[gvanrossum]

Sorry, the "blocked" label means that the PR is waiting for someone or something not on the core mypy team -- typically waiting for the author to push more commits (e.g. response to code review or a rebase) or when it's waiting for another issue to be fixed, or perhaps on a difficult decision. This is a signal to the core team to reduce priority reviewing. We're still experimenting with this workflow -- we don't have a way to signal exactly what it's waiting for, nor do we have a bot yet that automatically unblocks when the anticipated event happens. (Nor have we consistently applied it yet to the mypy repo -- I did do a pass over typeshed.) Sorry to scare you! I've removed the label now that you've responded to the code review.

[ilevkivskyi]

@gvanrossum
Thank you for the detailed explanation! It looks like a more "calm" color could better reflect the meaning of this label. I realize that the workload for the core team is high. Indeed, it could be helpful to have labels for PRs. I have seen other GitHub repos are using topic labels for PRs.

[gvanrossum]

Do you have a suggestion for a better color? The current color was randomly picked for my by GitHub...

[ilevkivskyi]

Do you have a suggestion for a better color?

White text on blue background might be better (the PR is "cold", waits for a new input).

[JukkaL]

I think that this is actually good now and my concerns weren't valid. Thanks for the PR!

[ilevkivskyi]

@JukkaL
There still one minor question, maybe it makes sense move this code inside analize_member_access at the end of the function? (just to keep it clearer for people who will later read this)

[JukkaL]

Hmm I just noticed that there are several calls to analyze_member_access in checkexpr.py. It's unclear whether we should always tweak the type variables or not at the end.

[ilevkivskyi]

@JukkaL

Hmm I just noticed that there are several calls to analyze_member_access in checkexpr.py. It's unclear whether we should always tweak the type variables or not at the end.

OK, I will take a look at those and will make an additional PR if necessary.

[ilevkivskyi]

@JukkaL

Hmm I just noticed that there are several calls to analyze_member_access in checkexpr.py. It's unclear whether we should always tweak the type variables or not at the end.

I investigated this, and it looks like wee need to do this everywhere, one can still trigger the crash in serialize with

from typing import TypeVar
T = TypeVar('T')

class C:
    def m(self, x: T) -> T:
        return x

class D(C):
    def __init__(self) -> None:
        self.d = super().m

I will make a PR to fix this soon.

[ilevkivskyi]

OK, here is the PR I promised #2847