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

Fix the logic in active self-type calculation for current scope #7235

Merged
merged 2 commits into from Jul 18, 2019

Conversation

@ilevkivskyi
Copy link
Collaborator

ilevkivskyi commented Jul 18, 2019

Fixes #5846

The fix for crash is kind of straightforward. Previously active_self_type() always returned None at method scope (sic!)

While working on this I discovered couple other problems:

  • Area around checking LSP is generally problematic (see also #5425 that I tried to fix earlier but failed). In particular, the are two ways to get current TypeInfo, one is from lvalue.node.info or defn.info, another is using active_self_type() (this is rather abusing it IMO). I don't try to fix this here (i.e. switch to always using one way) because this is a relatively large refactoring.
  • Currently in type checker we defer nested functions instead of top-level ones. I believe this is not by design and is rather caused by absence of docstrings and unintuitive method names in CheckerScope class. Namely, there "top function" stays for "top of stack function", not "top-level function". I don't try to fix this here either because this is conceptually a big change.
@ilevkivskyi ilevkivskyi requested a review from JukkaL Jul 18, 2019
@JukkaL
JukkaL approved these changes Jul 18, 2019
Copy link
Collaborator

JukkaL left a comment

I agree that it's strange that this hasn't been causing more trouble. I assume that this only triggers in very specific cases that are pretty rare.


class C(B):
def __init__(self) -> None:
self.x: Any

This comment has been minimized.

Copy link
@JukkaL

JukkaL Jul 18, 2019

Collaborator

What if the type is incompatible with the base class?

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Jul 18, 2019

Author Collaborator

Hm, I am not super happy with the error message, because it says:

Incompatible types in assignment (expression has type "int", base class "B" defined the type as "str")

while the initial type is T (it shows the mapped/expanded type). It is however not easy to improve, so I think it is OK.

Ivan Levkivskyi
@ilevkivskyi ilevkivskyi merged commit 0b59a10 into python:master Jul 18, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:fix-active-self branch Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.