Fix stack overflow with recursive generic protocols (depth limit)#21858
Fix stack overflow with recursive generic protocols (depth limit)#21858
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
2 | 0 | 0 |
| Total | 2 | 0 | 0 |
|
Can't we implement a salsa query with cycle handling instead of using |
|
What would the inputs to the Salsa query be? The issue here is that we have a different pair of types every time, because the specialization changes every time. So I don't think Salsa will detect it as a cycle either, if we just make |
I think a depth limit is fine for as long as it is deterministic. While salsa's cycle handling is convenient, it does come at a cost of higher memory usage (because it's persistent) and cycle handling is more complex than a hard limit in the visitor |
This fixes astral-sh/ty#1736 where recursive generic protocols with growing specializations caused a stack overflow. The issue occurred with protocols like: ```python class C[T](Protocol): a: 'C[set[T]]' ``` When checking `C[set[int]]` against e.g. `C[Unknown]`, member `a` requires checking `C[set[set[int]]]`, which requires `C[set[set[set[int]]]]`, etc. Each level has different type specializations, so the existing cycle detection (using full types as cache keys) didn't catch the infinite recursion. This fix adds a simple recursion depth limit (64) to the CycleDetector. When the depth exceeds the limit, we return the fallback value (assume compatible) to safely terminate the recursion.
I tried to test this out, but there are some problems with making anything in the |
…return * dcreager/die-die-intersections: (29 commits) simpler bounds [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) [ty] Remove legacy `concise_message` fallback behavior (#21847) [ty] Make Python-version subdiagnostics less verbose (#21849) [ty] Supress inlay hints when assigning a trivial initializer call (#21848) ...
…return * dcreager/die-die-intersections: (31 commits) clippy reword comment simpler bounds [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) [ty] Remove legacy `concise_message` fallback behavior (#21847) ...
…return * dcreager/die-die-intersections: (31 commits) clippy reword comment simpler bounds [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) [ty] Remove legacy `concise_message` fallback behavior (#21847) ...
* origin/main: (33 commits) [ty] Simplify union lower bounds and intersection upper bounds in constraint sets (#21871) [ty] Collapse `never` paths in constraint set BDDs (#21880) Fix leading comment formatting for lambdas with multiple parameters (#21879) [ty] Type inference for `@asynccontextmanager` (#21876) Fix comment placement in lambda parameters (#21868) [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) ...
Summary
This fixes astral-sh/ty#1736 where recursive generic protocols with growing specializations caused a stack overflow.
The issue occurred with protocols like:
When checking
C[set[int]]against e.g.C[Unknown], memberarequires checkingC[set[set[int]]], which requiresC[set[set[set[int]]]], etc. Each level has different type specializations, so the existing cycle detection (using full types as cache keys) didn't catch the infinite recursion.This fix adds a simple recursion depth limit (64) to the CycleDetector. When the depth exceeds the limit, we return the fallback value (assume compatible) to safely terminate the recursion.
This is a bit of a blunt hammer, but it should be broadly effective to prevent stack overflow in any nested-relation case, and it's hard to imagine that non-recursive nested relation comparisons of depth > 64 exist much in the wild.
Test Plan
Added mdtest.