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 crash on ParamSpec unification #16251

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Oct 11, 2023

Fixes #16245
Fixes #16248

Unfortunately I was a bit reckless with parentheses, but in my defense unify_generic_callable() is kind of broken for long time, as it can return "solutions" like {1: T`1}. We need a more principled approach there (IIRC there is already an issue about this in the scope of --new-type-inference).

(The fix is quite trivial so I am not going to wait for review too long to save time, unless there will be some issues in mypy_primer etc.)

@JelleZijlstra
Copy link
Member

There's a slightly mysterious-looking test failure in a mypyc test. Possibly unrelated to this diff; let's see if it also happens on #12661 where I just retriggered CI.

@ilevkivskyi
Copy link
Member Author

Btw @sobolevn can you please try this PR to check if it fixes #16248 as well?

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

@JelleZijlstra It passed on retry LOL. Actually, I think it is the second time Python 3.8 MacOS randomly flakes during last week (and a different test case IIRC, but can't find logs now).

@rudolfix
Copy link

I also had segfault and this branch does not solve it. here's offending code

@overload
def transformer(
    f: Callable[Concatenate[TDataItem, TResourceFunParams], Any],
    /,
    data_from: TUnboundDltResource = DltResource.Empty,
    name: str = None,
    table_name: TTableHintTemplate[str] = None,
    write_disposition: TTableHintTemplate[TWriteDisposition] = None,
    columns: TTableHintTemplate[TAnySchemaColumns] = None,
    primary_key: TTableHintTemplate[TColumnNames] = None,
    merge_key: TTableHintTemplate[TColumnNames] = None,
    selected: bool = True,
    spec: Type[BaseConfiguration] = None,
    standalone: Literal[True] = True
) -> Callable[TResourceFunParams, DltResource]:
    ...

here is part of stack trace (infinite recursion)

  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/types.py", line 398, in accept
    return visitor.visit_type_alias_type(self)
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/expandtype.py", line 453, in visit_type_alias_type
    return t.copy_modified(args=args)
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/types.py", line 431, in copy_modified
    return TypeAliasType(
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/types.py", line 306, in __init__
    super().__init__(line, column)
  File "/home/rudolfix/.cache/pypoetry/virtualenvs/dlt-BKjzTm6--py3.8/lib/python3.8/site-packages/mypy/types.py", line 231, in __init__
    super().__init__(line, column)
RecursionError: maximum recursion depth exceeded while calling a Python objec

seems some other function that you fixes is also affected

here's the minimum modification of the code above that prevents segfault

@overload
def transformer(
    f: Callable[Concatenate[TDataItem, TResourceFunParams], Any],
    /,
    data_from: TUnboundDltResource = DltResource.Empty,
    name: str = None,
    table_name: TTableHintTemplate[str] = None,
    write_disposition: TTableHintTemplate[TWriteDisposition] = None,
    columns: TTableHintTemplate[TAnySchemaColumns] = None,
    primary_key: TTableHintTemplate[TColumnNames] = None,
    merge_key: TTableHintTemplate[TColumnNames] = None,
    selected: bool = True,
    spec: Type[BaseConfiguration] = None,
    standalone: Literal[True] = True
) -> Callable[..., DltResource]:
    ...

hope this helps

@sobolevn
Copy link
Member

No, it still fails with:

  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 338, in visit_callable_type
    arg_types=self.expand_types(t.arg_types),
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 458, in expand_types
    a.append(t.accept(self))
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1954, in accept
    return visitor.visit_callable_type(self)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/expandtype.py", line 324, in visit_callable_type
    param_spec = t.param_spec()
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 2069, in param_spec
    prefix = Parameters(self.arg_types[:-2], self.arg_kinds[:-2], self.arg_names[:-2])
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 1584, in __init__
    super().__init__(line, column)
  File "/home/runner/work/returns/returns/.venv/lib/python3.8/site-packages/mypy/types.py", line 231, in __init__
    super().__init__(line, column)
RecursionError: maximum recursion depth exceeded while calling a Python object

https://github.com/dry-python/returns/actions/runs/6492022546/job/17630240757?pr=1702

At least, I can see the traceback now :)

@ilevkivskyi
Copy link
Member Author

Oops, yes, same thing for plain callables (I will also do a small refactoring while I am at this).

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

discord.py (https://github.com/Rapptz/discord.py)
- discord/app_commands/commands.py:2381: error: "Callable[..., Coroutine[Any, Any, Any]]" has no attribute "__discord_app_commands_checks__"  [attr-defined]
- discord/app_commands/commands.py:2383: error: "Callable[..., Coroutine[Any, Any, Any]]" has no attribute "__discord_app_commands_checks__"  [attr-defined]
+ discord/app_commands/commands.py:2381: error: Item "function" of "Callable[[Any, Interaction[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]] | Callable[[Interaction[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]]" has no attribute "__discord_app_commands_checks__"  [union-attr]
+ discord/app_commands/commands.py:2383: error: Item "function" of "Callable[[Any, Interaction[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]] | Callable[[Interaction[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]]" has no attribute "__discord_app_commands_checks__"  [union-attr]

@AlexWaygood
Copy link
Member

Happy to confirm that this fixes the typeshed crash! I tested by installing this PR branch into a venv with all of typeshed's other test requirements, making this change to mypy_test.py at typeshed, and then running python tests/mypy_test.py from the repo root of my local typeshed clone:

--- a/tests/utils.py
+++ b/tests/utils.py
@@ -83,8 +83,8 @@ def make_venv(venv_dir: Path) -> VenvInfo:

 @cache
 def get_mypy_req() -> str:
-    with open("requirements-tests.txt", encoding="UTF-8") as requirements_file:
-        return next(strip_comments(line) for line in requirements_file if "mypy" in line)
+    return "git+https://github.com/ilevkivskyi/mypy.git@fix-paramspec-regression"
+

@ilevkivskyi
Copy link
Member Author

@rudolfix @sobolevn Could you please try this PR again?

@rudolfix
Copy link

@ilevkivskyi yeah now it works! thank you for quick fix!

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix!

@ilevkivskyi
Copy link
Member Author

I tried mypy returns locally and it looks like this fixes #16248 as well.

@ilevkivskyi ilevkivskyi merged commit 72605dc into python:master Oct 12, 2023
17 checks passed
@ilevkivskyi ilevkivskyi deleted the fix-paramspec-regression branch October 12, 2023 20:25
hauntsaninja pushed a commit that referenced this pull request Oct 17, 2023
Fixes #16245
Fixes #16248

Unfortunately I was a bit reckless with parentheses, but in my defense
`unify_generic_callable()` is kind of broken for long time, as it can
return "solutions" like ```{1: T`1}```. We need a more principled
approach there (IIRC there is already an issue about this in the scope
of `--new-type-inference`).

(The fix is quite trivial so I am not going to wait for review too long
to save time, unless there will be some issues in `mypy_primer` etc.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants