Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions crates/ty_python_semantic/resources/mdtest/protocols.md
Original file line number Diff line number Diff line change
Expand Up @@ -3184,14 +3184,9 @@ from ty_extensions import reveal_protocol_interface
reveal_protocol_interface(Foo)
```

## Known panics
## Protocols generic over TypeVars bound to forward references

### Protocols generic over TypeVars bound to forward references

This test currently panics because the `ClassLiteral::explicit_bases` query fails to converge. See
issue <https://github.com/astral-sh/ty/issues/1587>.

<!-- expect-panic: execute: too many cycle iterations -->
Protocols can have TypeVars with forward reference bounds that form cycles.

```py
from typing import Any, Protocol, TypeVar
Expand All @@ -3209,6 +3204,19 @@ class A2(Protocol[T2]):

class B1(A1[T3], Protocol[T3]): ...
class B2(A2[T4], Protocol[T4]): ...

# TODO should just be `B2[Any]`
reveal_type(T3.__bound__) # revealed: B2[Any] | @Todo(specialized non-generic class)

# TODO error: [invalid-type-arguments]
def f(x: B1[int]):
pass

reveal_type(T4.__bound__) # revealed: B1[Any]

# error: [invalid-type-arguments]
def g(x: B2[int]):
pass
```

## TODO
Expand Down
79 changes: 78 additions & 1 deletion crates/ty_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9612,6 +9612,7 @@ impl<'db> TypeVarInstance<'db> {
}

#[salsa::tracked(
cycle_fn=lazy_bound_or_constraints_cycle_recover,
cycle_initial=lazy_bound_or_constraints_cycle_initial,
heap_size=ruff_memory_usage::heap_size
)]
Expand All @@ -9636,6 +9637,7 @@ impl<'db> TypeVarInstance<'db> {
}

#[salsa::tracked(
cycle_fn=lazy_bound_or_constraints_cycle_recover,
cycle_initial=lazy_bound_or_constraints_cycle_initial,
heap_size=ruff_memory_usage::heap_size
)]
Expand Down Expand Up @@ -9730,14 +9732,31 @@ fn lazy_bound_or_constraints_cycle_initial<'db>(
None
}

#[allow(clippy::ref_option)]
#[expect(clippy::ref_option)]
fn lazy_bound_or_constraints_cycle_recover<'db>(
db: &'db dyn Db,
cycle: &salsa::Cycle,
previous: &Option<TypeVarBoundOrConstraints<'db>>,
current: Option<TypeVarBoundOrConstraints<'db>>,
_typevar: TypeVarInstance<'db>,
) -> Option<TypeVarBoundOrConstraints<'db>> {
// Normalize the bounds/constraints to ensure cycle convergence.
match (previous, current) {
(Some(prev), Some(current)) => Some(current.cycle_normalized(db, *prev, cycle)),
(None, Some(current)) => Some(current.recursive_type_normalized(db, cycle)),
(_, None) => None,
}
}

#[expect(clippy::ref_option)]
fn lazy_default_cycle_recover<'db>(
db: &'db dyn Db,
cycle: &salsa::Cycle,
previous_default: &Option<Type<'db>>,
default: Option<Type<'db>>,
_typevar: TypeVarInstance<'db>,
) -> Option<Type<'db>> {
// Normalize the default to ensure cycle convergence.
match (previous_default, default) {
(Some(prev), Some(default)) => Some(default.cycle_normalized(db, *prev, cycle)),
(None, Some(default)) => Some(default.recursive_type_normalized(db, cycle)),
Expand Down Expand Up @@ -10106,6 +10125,64 @@ impl<'db> TypeVarBoundOrConstraints<'db> {
}
}

/// Normalize for cycle recovery by combining with the previous value and
/// removing divergent types introduced by the cycle.
///
/// See [`Type::cycle_normalized`] for more details on how this works.
fn cycle_normalized(self, db: &'db dyn Db, previous: Self, cycle: &salsa::Cycle) -> Self {
match (self, previous) {
(
TypeVarBoundOrConstraints::UpperBound(bound),
TypeVarBoundOrConstraints::UpperBound(prev_bound),
) => {
TypeVarBoundOrConstraints::UpperBound(bound.cycle_normalized(db, prev_bound, cycle))
}
(
TypeVarBoundOrConstraints::Constraints(constraints),
TypeVarBoundOrConstraints::Constraints(prev_constraints),
) => {
// Normalize each constraint with its corresponding previous constraint
let current_elements = constraints.elements(db);
let prev_elements = prev_constraints.elements(db);
TypeVarBoundOrConstraints::Constraints(UnionType::new(
db,
current_elements
.iter()
.zip(prev_elements.iter())
.map(|(ty, prev_ty)| ty.cycle_normalized(db, *prev_ty, cycle))
.collect::<Box<_>>(),
))
}
// The choice of whether it's an upper bound or constraints is purely syntactic and
// thus can never change in a cycle: `parsed_module` does not participate in cycles,
// the AST will never change from one iteration to the next.
_ => unreachable!(
"TypeVar switched from bound to constraints (or vice versa) in fixpoint iteration"
),
}
}

/// Normalize recursive types for cycle recovery when there's no previous value.
///
/// See [`Type::recursive_type_normalized`] for more details.
fn recursive_type_normalized(self, db: &'db dyn Db, cycle: &salsa::Cycle) -> Self {
match self {
TypeVarBoundOrConstraints::UpperBound(bound) => {
TypeVarBoundOrConstraints::UpperBound(bound.recursive_type_normalized(db, cycle))
}
TypeVarBoundOrConstraints::Constraints(constraints) => {
TypeVarBoundOrConstraints::Constraints(UnionType::new(
db,
constraints
.elements(db)
.iter()
.map(|ty| ty.recursive_type_normalized(db, cycle))
.collect::<Box<_>>(),
))
}
}
}

fn materialize_impl(
self,
db: &'db dyn Db,
Expand Down
8 changes: 8 additions & 0 deletions crates/ty_python_semantic/src/types/infer/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11481,6 +11481,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
if typevar.default_type(db).is_some() {
typevar_with_defaults += 1;
}
// TODO consider just accepting the given specialization without checking
// against bounds/constraints, but recording the expression for deferred
// checking at end of scope. This would avoid a lot of cycles caused by eagerly
// doing assignment checks here.
match typevar.typevar(db).bound_or_constraints(db) {
Some(TypeVarBoundOrConstraints::UpperBound(bound)) => {
if provided_type
Expand All @@ -11505,6 +11509,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
}
}
Some(TypeVarBoundOrConstraints::Constraints(constraints)) => {
// TODO: this is wrong, the given specialization needs to be assignable
// to _at least one_ of the individual constraints, not to the union of
// all of them. `int | str` is not a valid specialization of a typevar
// constrained to `(int, str)`.
if provided_type
.when_assignable_to(
db,
Expand Down
Loading