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

mypy 0.990 crashes with segmentation fault on hydra_zen due to recursive types #14031

Closed
rsokl opened this issue Nov 8, 2022 · 4 comments · Fixed by #14038
Closed

mypy 0.990 crashes with segmentation fault on hydra_zen due to recursive types #14031

rsokl opened this issue Nov 8, 2022 · 4 comments · Fixed by #14038

Comments

@rsokl
Copy link
Contributor

rsokl commented Nov 8, 2022

Crash Report

Upgrading from mypy 0.982 to 0.990 leads to a segmentation fault when I run on a file that merely imports hydra_zen:

import hydra_zen

This appears to be related to hydra-zen's use of recursive types and mypy 0.990 enabling these by default. To confirm, I reverted to 0.982 and configured then enable_recursive_aliases = true and confirmed that this also produces a segmentation fault.

This is the recursive type that is creating the issue. Oddly, when I specifically remove Sequence["SupportedPrimitive"] from the union (comment out line 223), the issue goes away. I spent time trying to further isolate the culprit and create a minimal example, but I was unable to decouple the issue from hydra_zen.

Traceback

Segmentation fault

To Reproduce

rsokl@Maxwell:/tmp$ pip install hydra-zen==0.8.0 mypy==0.990
rsokl@Maxwell:/tmp$ echo "import hydra_zen" > foo.py
rsokl@Maxwell:/tmp/hydra-zen$ mypy foo.py
Segmentation fault

Your Environment

  • Mypy version used: 0.990
  • Mypy command-line flags: (None)
  • Mypy configuration options from mypy.ini (and other config files): (None)
  • Python version used: python-3.9.13-haa1d7c7_2
  • Operating system and version: Ubuntu 20.04.4 LTS (GNU/Linux 5.10.102.1-microsoft-standard-WSL2 x86_64)
@JelleZijlstra
Copy link
Member

cc @ilevkivskyi

@ilevkivskyi
Copy link
Member

Hm, I have a possible fix for this

--- a/mypy/constraints.py
+++ b/mypy/constraints.py
@@ -177,7 +177,7 @@ def infer_constraints(template: Type, actual: Type, direction: int) -> list[Cons
         for (t, a) in reversed(TypeState.inferring)
     ):
         return []
-    if has_recursive_types(template):
+    if has_recursive_types(template) or isinstance(template, Instance):
         # This case requires special care because it may cause infinite recursion.
         if not has_type_vars(template):
             # Return early on an empty branch.

Now need to find a more compact repro for a regression test. Unfortunately, as with the previous crash I fixed, this will likely cause some performance hit. I will see if I can come up with something better, since this may affect performance of non-recursive types as well.

I can probably give a bit more context on all the crashes. There is a classic algorithm for recursive types subtyping, it is however at least quadratic in number of type components of the recursive type, plus it slows down non-recursive types as well. So I tried to play smart, like "why would we need to push trivial pairs like (int, str) on an assumption stack, right?" It turns out there are various unexpected corner cases that are hard to predict where my optimization breaks (e.g. str is a subclass of Sequence[str], this is what causes issues here).

@ilevkivskyi
Copy link
Member

And here is a simple repro:

from typing import Sequence, TypeVar

T = TypeVar("T")
def foo(x: T) -> str:
    return "test"

Nested = str | Sequence[Nested]
x: Nested = foo(1)

@ilevkivskyi
Copy link
Member

It turns out the above proposed fix doesn't cause any visible performance regression on self check #14038

JukkaL pushed a commit that referenced this issue Nov 8, 2022
…4038)

Fixes #14031 

It turns out premature optimization is the root of all evil. (It turns
out this costs us less than 1% time on self-check).
svalentin pushed a commit that referenced this issue Nov 8, 2022
…4038)

Fixes #14031 

It turns out premature optimization is the root of all evil. (It turns
out this costs us less than 1% time on self-check).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants