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

Use polymorphic inference in unification #17348

Merged
merged 10 commits into from
Jun 10, 2024

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Jun 8, 2024

Moving towards #15907
Fixes #17206

This PR enables polymorphic inference during unification. This will allow us to handle even more tricky situations involving generic higher-order functions (see a random example I added in tests). Implementation is mostly straightforward, few notes:

  • This uncovered another issue with unions in solver, unfortunately current constraint inference algorithm can sometimes infer weird constraints like T <: Union[T, int], that later confuse the solver.
  • This uncovered another possible type variable clash scenario that was not handled properly. In overloaded generic function, each overload should have a different namespace for type variables (currently they all just get function name). I use module.some_func#0 etc. for overloads namespaces instead.
  • Another thing with overloads is that the switch caused unsafe overlap check to change: after some back and forth I am keeping it mostly the same to avoid possible regressions (unfortunately this requires some extra refreshing of type variables).
  • This makes another ParamSpec crash to happen more often so I fix it in this same PR.
  • Finally this uncovered a bug in handling of overloaded __init__() that I am fixing here as well.

Let's see mypy_primer will show us.

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

Hm, it looks like this makes #17206 (RuntimeError: Parameters cannot be constrained to during unification) to occur much more often. I will take a look (and probably at the original report as well).

@ilevkivskyi
Copy link
Member Author

I also spot-checked new errors and it looks like these are false positives. It looks like they are all caused by the same pattern. I think I know how to fix it, but it is non-trivial, so will do it tomorrow.

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

OK, overload overlap checks are a nightmare. They use some pretty hacky things (I understand it is to avoid duplicating a lot of code in subtypes.py). Initially I hoped to make it more principled, but after few attempts I give up, I am going to minimize new false positives, and this is it.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Jun 9, 2024

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

steam.py (https://github.com/Gobot1234/steam.py)
- steam/utils.py:539: error: Overloaded function implementation does not accept all possible arguments of signature 2  [misc]
+ steam/ext/commands/commands.py:695: error: Overloaded function signatures 2 and 3 overlap with incompatible return types  [overload-overlap]
+ steam/ext/commands/commands.py:741: error: Overloaded function signatures 2 and 3 overlap with incompatible return types  [overload-overlap]
+ steam/ext/commands/commands.py:795: error: Overloaded function signatures 2 and 3 overlap with incompatible return types  [overload-overlap]
- steam/ext/commands/commands.py:851: error: Overloaded function implementation does not accept all possible arguments of signature 3  [misc]
+ steam/ext/commands/commands.py:844: error: Overloaded function signatures 2 and 3 overlap with incompatible return types  [overload-overlap]

Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/commands/menu.py:182: error: Incompatible return value type (got "MenuCommand[_MessageCallbackSigT@__init__, Literal[CommandType.MESSAGE]]", expected "MenuCommand[_MessageCallbackSigT@decorator, Literal[CommandType.MESSAGE]]")  [return-value]
- tanjun/commands/menu.py:309: error: Incompatible return value type (got "MenuCommand[_UserCallbackSigT@__init__, Literal[CommandType.USER]]", expected "MenuCommand[_UserCallbackSigT@decorator, Literal[CommandType.USER]]")  [return-value]
+ tanjun/commands/menu.py:421: error: Type argument "_UserCallbackSigT" of "MessageCommand" must be a subtype of "Callable[[MessageContext, VarArg(Any), KwArg(Any)], Coroutine[Any, Any, None]]"  [type-var]
+ tanjun/commands/menu.py:421: error: Type argument "_UserCallbackSigT" of "SlashCommand" must be a subtype of "Callable[[SlashContext, VarArg(Any), KwArg(Any)], Coroutine[Any, Any, None]]"  [type-var]

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ pandas-stubs/core/reshape/concat.pyi:27: error: Overloaded function signatures 1 and 6 overlap with incompatible return types  [overload-overlap]

discord.py (https://github.com/Rapptz/discord.py)
+ discord/ext/commands/hybrid.py:911: error: Incompatible return value type (got "Callable[[Callable[[CogT@decorator, ContextT@decorator, **P], Coroutine[Any, Any, T@decorator]] | Callable[[ContextT@decorator, **P], Coroutine[Any, Any, T@decorator]]], HybridCommand[CogT@decorator, P, T@decorator]]", expected "Callable[[Callable[[CogT, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]], HybridCommand[CogT, P, T]]")  [return-value]
+ discord/ext/commands/hybrid.py:944: error: Incompatible return value type (got "Callable[[Callable[[CogT@decorator, ContextT@decorator, **P], Coroutine[Any, Any, T@decorator]] | Callable[[ContextT@decorator, **P], Coroutine[Any, Any, T@decorator]]], HybridGroup[CogT@decorator, P, T@decorator]]", expected "Callable[[Callable[[CogT, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]], HybridGroup[CogT, P, T]]")  [return-value]

@ilevkivskyi
Copy link
Member Author

@JukkaL @hauntsaninja I am now happy with mypy_primer output, this is ready for review.

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.

Looks good. This makes type inference more powerful and consistent, and it's nice that you were able to fix a bunch of other issues along the way.

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.

Crash on a tricky overloaded Protocol
2 participants