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

Loosen hardlimit application heuristic slightly #50579

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Keno
Copy link
Member

@Keno Keno commented Jul 17, 2023

Ordinarily our recursion heuristic looks for recursion of edges, widening types to force convergence if a a recursion of edges is detected. However, under some circumstances (currently - as of #31734 when there are multiple applicable methods), we fall back to simple recursion of methods. This can be quite limiting for packages that have a big central dispatch method (e.g. Diffractor). This attempts to find a middle ground by applying the hardlimit fallback only if the split signatures are not concrete. The kind of case that we want to catch here often arise from signatures with unresolved Vararg (where there's a lot of destructuring methods, but since the size of the vararg is not known, we never terminate) or overly abstract types. I'm hoping this provides a better middle ground tradeoff by still prohibiting those kinds of cases while allowing otherwise very concretely inferred code that just happens to dispatch through a central higher-order method.

Ordinarily our recursion heuristic looks for recursion of edges,
widening types to force convergence if a a recursion of edges is
detected. However, under some circumstances (currently - as of
#31734 when there are multiple applicable methods), we fall back
to simple recursion of methods. This can be quite limiting for
packages that have a big central dispatch method (e.g. Diffractor).
This attempts to find a middle ground by applying the hardlimit
fallback only if the split signatures are not concrete. The kind of
case that we want to catch here often arise from signatures with
unresolved Vararg (where there's a lot of destructuring methods,
but since the size of the vararg is not known, we never terminate)
or overly abstract types. I'm hoping this provides a better middle
ground tradeoff by still prohibiting those kinds of cases while
allowing otherwise very concretely inferred code that just happens
to dispatch through a central higher-order method.
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Hmm, I was pretty sure we had written tests for this case before, but concrete seeming (but erroneous / infinite) signatures are exactly the primary sort of thing this code is supposed to be catching and limiting before they hit runaway.

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.

2 participants