[ty] Avoid stack overflow when calculating inferable typevars#21971
[ty] Avoid stack overflow when calculating inferable typevars#21971
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
| walk_bound_type_var_type(db, bound_typevar, self); | ||
| let typevar = bound_typevar.typevar(db); | ||
| if let Some(bound_or_constraints) = typevar.bound_or_constraints(db) { | ||
| walk_type_var_bounds(db, bound_or_constraints, self); | ||
| } |
There was a problem hiding this comment.
Before, we were recursing arbitrarily deep into each of the generic context's typevars — and in particular, arbitrarily deep into their bounds and constraints. There was a recursion in the example from astral-sh/ty#1874.
Really, we don't need to go more than one level deep looking for additional inferable typevars. We just need to look into the bound/constraints of the generic context's typevars. If those bounds/constraints themselves contain typevars, we mark them as inferable, but we don't need to recurse further. So we set should_visit_lazy_type_var_attributes to false to prevent the deeper recursions, and unroll the first level of recursion manually here.
|
|
CodSpeed Performance ReportMerging #21971 will improve performances by 7.94%Comparing Summary
Benchmarks breakdown
Footnotes
|
dcreager
requested review from
AlexWaygood,
carljm and
sharkdp
as code owners
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Nice! Can we add the MRE in astral-sh/ty#1874 (comment) as a regression test?
|
And thank you so much for the added docs, really helpful! |
* origin/main: Update MSRV to 1.90 (#21987) [ty] Improve check enforcing that an overloaded function must have an implementation (#21978) Update actions/checkout digest to 8e8c483 (#21982) [ty] Use `ParamSpec` without the attr for inferable check (#21934) [ty] Emit diagnostic when a type variable with a default is followed by one without a default (#21787) [ty] Fix callout syntax in configuration mkdocs (#1875) (#21961) Update debug_assert which pointed at missing method (#21969) [ty] Add support for `__qualname__` and other implicit class attributes (#21966)
Done |
* origin/main: Fluent formatting of method chains (#21369) [ty] Avoid stack overflow when calculating inferable typevars (#21971) [ty] Add "qualify ..." code fix for undefined references (#21968) [ty] Use jemalloc on linux (#21975) Update MSRV to 1.90 (#21987) [ty] Improve check enforcing that an overloaded function must have an implementation (#21978) Update actions/checkout digest to 8e8c483 (#21982) [ty] Use `ParamSpec` without the attr for inferable check (#21934) [ty] Emit diagnostic when a type variable with a default is followed by one without a default (#21787)
3 participants
When we calculate which typevars are inferable in a generic context, the result might include more than the typevars bound by the generic context. The canonical example is a generic method of a generic class:
Here, the inferable typevar set of
methodcontainsSelfandT, as you'd expect. (Those are the typevars bound by the method.) But it also containsA@C, since the implicitSelftypevar is defined asSelf: C[A]. That means when we callmethod, we need to markA@Cas inferable, so that we can determine the correct mapping forA@Cat the call site.Fixes astral-sh/ty#1874