Fix crash with overload and callable object decorators #11630

hauntsaninja · 2021-11-27T09:12:00Z

Fixes #8356, as identified by @pranavrajpal
Fixes #9112
Fixes #9967

Note that we still don't fully support the singledispatch pattern in #8356,
since we get 'overloaded function has no attribute "register"', but that's
much easier to work around than a crash.

@pranavrajpal

Fixes the crash in python#8356, as identified by @pranavrajpal Note that we still don't support the singledispatch in that issue, since we get 'overloaded function has no attribute "register"', but that's much easier to work around than a crash.

pranavrajpal · 2021-11-27T18:56:30Z

Do you know how hard it would be to fix #9967 as well? The underlying issue seems like the same as the other 2 issues.

I put a simplified example in #9967 (comment).

hauntsaninja · 2021-11-27T22:04:35Z

Thanks for pointing that one out, pushed a fix for it as well!

sobolevn · 2021-11-28T21:52:34Z

mypy/checker.py

+                    impl_type = inner_type
+                elif isinstance(inner_type, Instance):
+                    inner_call = get_proper_type(
+                        find_member('__call__', inner_type, inner_type, is_operator=True)


find_member won't call any plugins. And might not be as accurate as analyze_member_access.

Any specific reason to use find_member here?

I didn't have any specific reason, feel free to change! Just make sure that the getattr handling for the call operator matches runtime handling, e.g. with code like:

class X: def __getattr__(self, attr): if attr == "__call__": return lambda *a, **kw: print(a, kw) raise AttributeError @X() def f(): ...

In general, I'd love for member access to get cleaned up. I last tried a little bit in #8438 but that was swiftly reverted in #8500

Related #3832

I will send a PR in a moment 🙂

Just make sure that the getattr handling for the call operator matches runtime handling

Right now it does not work this way with both find_member and analyze_member_access. I will have to add this feature.

I think find_member does the right thing when passed is_operator=True? This is what I meant: #11637 (the current code does what I'd expect)

sobolevn · 2021-11-29T09:57:35Z

mypy/checker.py

+                    inner_call = get_proper_type(
+                        find_member('__call__', inner_type, inner_type, is_operator=True)
+                    )
+                    if isinstance(inner_call, CallableType):


One more problem:

from typing import Callable, Union, Any, overload, Protocol class DCall(Protocol): def __call__(self, arg: Union[int, str]) -> None: ... class D: def __getattr__(self, attr: str) -> DCall: ... # Will return `__call__` in runtime def dec_d(f: Callable[..., Any]) -> D: return D() @overload def f_d(arg: int) -> None: ... @overload def f_d(arg: str) -> None: ... @dec_d def f_d(arg: Union[int, str]) -> None: ...

This now reports: out/ex.py:18: error: "D" not callable, because the return type of __getattr__ is not CallableType, but Instance.

I am going to create a new issue out of it. Sadly, I don't have the time right now to fix it.

Sorry, it's late for me, so I might be missing something, but that seems to be the correct behaviour?

λ cat ex.py from typing import Callable, Union, Any, overload, Protocol class DCall(Protocol): def __call__(self, arg: Union[int, str]) -> None: ... class D: def __getattr__(self, attr: str) -> DCall: if attr == "__call__": return lambda *a, **kw: print(a, kw) raise AttributeError def dec_d(f: Callable[..., Any]) -> D: return D() @overload def f_d(arg: int) -> None: ... @overload def f_d(arg: str) -> None: ... @dec_d def f_d(arg: Union[int, str]) -> None: ... import pytest D().__call__(1) with pytest.raises(TypeError): print(D()(1)) with pytest.raises(TypeError): print(f_d(1)) print("correct") λ python3 ex.py (1,) {} correct λ mypy ex.py ex.py:20: error: "D" not callable ex.py:27: error: "D" not callable Found 2 errors in 1 file (checked 1 source file)

@rraval

Fixes #8481 Fixes #9941 Fixes #11025 Fixes #11038 This is the unresolved crash that's been reported the most times, e.g., it's as easy to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even `import torch`, with the right PyTorch version). I know of multiple popular Python packages that have made changes just to work around this crash. I would love to see this released, potentially along with #11630. Thanks to @rraval for making a minimal repro! The fix ended up being a one-liner, but it took me a bit to track down :-) Since things worked with implicit reexport, I differentially patched in explicit reexport logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, which fixes things: https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028 Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` pkg.a.B was first created with `module_public=True` (resulting in creation of a PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. So tracking down where pkg.a.B symbol was first created with `module_public=True` led me to this "obvious" bug.

@pranavrajpal

Fixes #8356, as identified by @pranavrajpal Fixes #9112 Fixes #9967 Note that we still don't fully support the singledispatch pattern in #8356, since we get 'overloaded function has no attribute "register"', but that's much easier to work around than a crash. Co-authored-by: hauntsaninja <>

@rraval

Fixes #8481 Fixes #9941 Fixes #11025 Fixes #11038 This is the unresolved crash that's been reported the most times, e.g., it's as easy to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even `import torch`, with the right PyTorch version). I know of multiple popular Python packages that have made changes just to work around this crash. I would love to see this released, potentially along with #11630. Thanks to @rraval for making a minimal repro! The fix ended up being a one-liner, but it took me a bit to track down :-) Since things worked with implicit reexport, I differentially patched in explicit reexport logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, which fixes things: https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028 Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` pkg.a.B was first created with `module_public=True` (resulting in creation of a PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. So tracking down where pkg.a.B symbol was first created with `module_public=True` led me to this "obvious" bug.

@pranavrajpal

Fixes python#8356, as identified by @pranavrajpal Fixes python#9112 Fixes python#9967 Note that we still don't fully support the singledispatch pattern in python#8356, since we get 'overloaded function has no attribute "register"', but that's much easier to work around than a crash. Co-authored-by: hauntsaninja <>

@rraval

…#11632) Fixes python#8481 Fixes python#9941 Fixes python#11025 Fixes python#11038 This is the unresolved crash that's been reported the most times, e.g., it's as easy to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even `import torch`, with the right PyTorch version). I know of multiple popular Python packages that have made changes just to work around this crash. I would love to see this released, potentially along with python#11630. Thanks to @rraval for making a minimal repro! The fix ended up being a one-liner, but it took me a bit to track down :-) Since things worked with implicit reexport, I differentially patched in explicit reexport logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, which fixes things: https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028 Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` pkg.a.B was first created with `module_public=True` (resulting in creation of a PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. So tracking down where pkg.a.B symbol was first created with `module_public=True` led me to this "obvious" bug.

hauntsaninja added 2 commits November 27, 2021 01:11

handle dumb decorators better

e600b0e

hauntsaninja force-pushed the overdec branch from 64db099 to e600b0e Compare November 27, 2021 09:47

JelleZijlstra approved these changes Nov 27, 2021

View reviewed changes

also fix 9967

3f135c3

hauntsaninja merged commit 7b13ff8 into python:master Nov 28, 2021

hauntsaninja deleted the overdec branch November 28, 2021 02:46

hauntsaninja mentioned this pull request Nov 28, 2021

Fix crash involving explicit reexport, import cycle, wildcard #11632

Merged

sobolevn reviewed Nov 28, 2021

View reviewed changes

sobolevn reviewed Nov 29, 2021

View reviewed changes

This was referenced Nov 29, 2021

Addresses review of #11630 #11638

Merged

Callable type is reported as not callable when used as a decorator #11639

Closed

ilevkivskyi mentioned this pull request Dec 3, 2021

Release 0.920 planning #11158

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix crash with overload and callable object decorators #11630

Fix crash with overload and callable object decorators #11630

hauntsaninja commented Nov 27, 2021 •

edited

Loading

pranavrajpal commented Nov 27, 2021

hauntsaninja commented Nov 27, 2021

sobolevn Nov 28, 2021 •

edited

Loading

hauntsaninja Nov 28, 2021

sobolevn Nov 29, 2021

sobolevn Nov 29, 2021 •

edited

Loading

hauntsaninja Nov 29, 2021 •

edited

Loading

sobolevn Nov 29, 2021

sobolevn Nov 29, 2021

hauntsaninja Nov 29, 2021 •

edited

Loading

Fix crash with overload and callable object decorators #11630

Fix crash with overload and callable object decorators #11630

Conversation

hauntsaninja commented Nov 27, 2021 • edited Loading

pranavrajpal commented Nov 27, 2021

hauntsaninja commented Nov 27, 2021

sobolevn Nov 28, 2021 • edited Loading

Choose a reason for hiding this comment

hauntsaninja Nov 28, 2021

Choose a reason for hiding this comment

sobolevn Nov 29, 2021

Choose a reason for hiding this comment

sobolevn Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

hauntsaninja Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

sobolevn Nov 29, 2021

Choose a reason for hiding this comment

sobolevn Nov 29, 2021

Choose a reason for hiding this comment

hauntsaninja Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

hauntsaninja commented Nov 27, 2021 •

edited

Loading

sobolevn Nov 28, 2021 •

edited

Loading

sobolevn Nov 29, 2021 •

edited

Loading

hauntsaninja Nov 29, 2021 •

edited

Loading

hauntsaninja Nov 29, 2021 •

edited

Loading