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 polymorphic application for callback protocols #16514

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

ilevkivskyi
Copy link
Member

Fixes #16512

The problems were caused if same callback protocol appeared multiple times in a signature. Previous logic confused this with a recursive callback protocol.

This comment has been minimized.

Comment on lines 6212 to 6225
def __init__(
self,
poly_tvars: Iterable[TypeVarLikeType],
bound_tvars: set[TypeVarLikeType] | None = None,
seen_aliases: set[TypeInfo] | None = None,
) -> None:
self.poly_tvars = set(poly_tvars)
# This is a simplified version of TypeVarScope used during semantic analysis.
self.bound_tvars: set[TypeVarLikeType] = set()
self.seen_aliases: set[TypeInfo] = set()
if bound_tvars is None:
bound_tvars = set()
self.bound_tvars = bound_tvars
if seen_aliases is None:
seen_aliases = set()
self.seen_aliases = seen_aliases
Copy link
Member

Choose a reason for hiding this comment

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

You could possibly do this slightly more simply using frozensets:

--- a/mypy/checkexpr.py
+++ b/mypy/checkexpr.py
@@ -6212,16 +6212,12 @@ class PolyTranslator(TypeTranslator):
     def __init__(
         self,
         poly_tvars: Iterable[TypeVarLikeType],
-        bound_tvars: set[TypeVarLikeType] | None = None,
-        seen_aliases: set[TypeInfo] | None = None,
+        bound_tvars: frozenset[TypeVarLikeType] = frozenset(),
+        seen_aliases: frozenset[TypeInfo] = frozenset(),
     ) -> None:
         self.poly_tvars = set(poly_tvars)
         # This is a simplified version of TypeVarScope used during semantic analysis.
-        if bound_tvars is None:
-            bound_tvars = set()
         self.bound_tvars = bound_tvars
-        if seen_aliases is None:
-            seen_aliases = set()
         self.seen_aliases = seen_aliases

The regression test you added passes with this change

This comment has been minimized.

Copy link
Contributor

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

anyio (https://github.com/agronholm/anyio): typechecking got 24133.51x faster (16.0s -> 0.0s)
(Performance measurements are based on a single noisy sample)

discord.py (https://github.com/Rapptz/discord.py): typechecking got 1.08x slower (176.3s -> 190.0s)
(Performance measurements are based on a single noisy sample)

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!

@JukkaL JukkaL merged commit fc811ae into python:master Nov 22, 2023
18 checks passed
JukkaL pushed a commit that referenced this pull request Nov 22, 2023
Fixes #16512

The problems were caused if same callback protocol appeared multiple
times in a signature. Previous logic confused this with a recursive
callback protocol.
@JukkaL JukkaL mentioned this pull request Nov 22, 2023
2 tasks
@ilevkivskyi ilevkivskyi deleted the fix-cb-poly branch November 22, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using ParamSpec in decorator with new type inference results in Never populated as type arguments
3 participants