From 349e24db9a28206dc6c9c68c044b048169b77a7f Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 2 Nov 2025 00:31:07 -0400 Subject: [PATCH] [ty] Less eager simplification of intersections that include constrained type variables --- .../mdtest/generics/pep695/variables.md | 30 ++-- crates/ty_python_semantic/src/types.rs | 43 ++++- .../ty_python_semantic/src/types/builder.rs | 150 ++---------------- 3 files changed, 71 insertions(+), 152 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md index a1262e8913..2c105345a6 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md @@ -1008,10 +1008,10 @@ def constrained[T: (Base, Sub, Unrelated)](t: T) -> None: reveal_type(x) # revealed: T@constrained & Base def _(x: Intersection[T, Unrelated]) -> None: - reveal_type(x) # revealed: Unrelated + reveal_type(x) # revealed: T@constrained & Unrelated def _(x: Intersection[T, Sub]) -> None: - reveal_type(x) # revealed: Sub + reveal_type(x) # revealed: T@constrained & Sub def _(x: Intersection[T, None]) -> None: reveal_type(x) # revealed: Never @@ -1028,7 +1028,7 @@ from ty_extensions import Not def remove_constraint[T: (int, str, bool)](t: T) -> None: def _(x: Intersection[T, Not[int]]) -> None: - reveal_type(x) # revealed: str + reveal_type(x) # revealed: T@remove_constraint & ~int def _(x: Intersection[T, Not[str]]) -> None: # With OneOf this would be OneOf[int, bool] @@ -1082,38 +1082,38 @@ class R: ... def f[T: (P, Q)](t: T) -> None: if isinstance(t, P): - reveal_type(t) # revealed: P + reveal_type(t) # revealed: T@f & P p: P = t else: - reveal_type(t) # revealed: Q & ~P + reveal_type(t) # revealed: T@f & ~P q: Q = t if isinstance(t, Q): - reveal_type(t) # revealed: Q + reveal_type(t) # revealed: T@f & Q q: Q = t else: - reveal_type(t) # revealed: P & ~Q + reveal_type(t) # revealed: T@f & ~Q p: P = t def g[T: (P, Q, R)](t: T) -> None: if isinstance(t, P): - reveal_type(t) # revealed: P + reveal_type(t) # revealed: T@g & P p: P = t elif isinstance(t, Q): - reveal_type(t) # revealed: Q & ~P + reveal_type(t) # revealed: T@g & Q & ~P q: Q = t else: - reveal_type(t) # revealed: R & ~P & ~Q + reveal_type(t) # revealed: T@g & ~P & ~Q r: R = t if isinstance(t, P): - reveal_type(t) # revealed: P + reveal_type(t) # revealed: T@g & P p: P = t elif isinstance(t, Q): - reveal_type(t) # revealed: Q & ~P + reveal_type(t) # revealed: T@g & Q & ~P q: Q = t elif isinstance(t, R): - reveal_type(t) # revealed: R & ~P & ~Q + reveal_type(t) # revealed: T@g & R & ~P & ~Q r: R = t else: reveal_type(t) # revealed: Never @@ -1124,10 +1124,10 @@ If the constraints are disjoint, simplification does eliminate the redundant neg ```py def h[T: (P, None)](t: T) -> None: if t is None: - reveal_type(t) # revealed: None + reveal_type(t) # revealed: T@h & None p: None = t else: - reveal_type(t) # revealed: P + reveal_type(t) # revealed: T@h & ~None p: P = t ``` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index be2fb264d8..93540bb1ff 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -1899,8 +1899,10 @@ impl<'db> Type<'db> { }) }), - (Type::Intersection(intersection), _) => { - intersection.positive(db).iter().when_any(db, |&elem_ty| { + (Type::Intersection(intersection), _) => intersection + .positive(db) + .iter() + .when_any(db, |&elem_ty| { elem_ty.has_relation_to_impl( db, target, @@ -1910,7 +1912,26 @@ impl<'db> Type<'db> { disjointness_visitor, ) }) - } + .or(db, || { + if intersection + .positive(db) + .iter() + .any(|element| element.is_type_var()) + { + intersection + .with_positive_typevars_solved_to_bounds_or_constraints(db) + .has_relation_to_impl( + db, + target, + inferable, + relation, + relation_visitor, + disjointness_visitor, + ) + } else { + ConstraintSet::from(false) + } + }), // Other than the special cases checked above, no other types are a subtype of a // typevar, since there's no guarantee what type the typevar will be specialized to. @@ -11804,6 +11825,22 @@ impl<'db> IntersectionType<'db> { (self.positive(db).len() + self.negative(db).len()) == 1 } + pub(crate) fn with_positive_typevars_solved_to_bounds_or_constraints( + self, + db: &'db dyn Db, + ) -> Type<'db> { + self.map_positive(db, |ty| match ty { + Type::TypeVar(tvar) => match tvar.typevar(db).bound_or_constraints(db) { + Some(TypeVarBoundOrConstraints::UpperBound(bound)) => bound, + Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { + Type::Union(constraints) + } + None => Type::object(), + }, + _ => *ty, + }) + } + fn heap_size((positive, negative): &(FxOrderSet>, FxOrderSet>)) -> usize { ruff_memory_usage::order_set_heap_size(positive) + ruff_memory_usage::order_set_heap_size(negative) diff --git a/crates/ty_python_semantic/src/types/builder.rs b/crates/ty_python_semantic/src/types/builder.rs index 11017d1571..b6e87b63bf 100644 --- a/crates/ty_python_semantic/src/types/builder.rs +++ b/crates/ty_python_semantic/src/types/builder.rs @@ -40,8 +40,7 @@ use crate::types::enums::{enum_member_literals, enum_metadata}; use crate::types::type_ordering::union_or_intersection_elements_ordering; use crate::types::{ - BytesLiteralType, IntersectionType, KnownClass, StringLiteralType, Type, - TypeVarBoundOrConstraints, UnionType, + BytesLiteralType, IntersectionType, KnownClass, StringLiteralType, Type, UnionType, }; use crate::{Db, FxOrderSet}; use rustc_hash::FxHashSet; @@ -1047,145 +1046,28 @@ impl<'db> InnerIntersectionBuilder<'db> { } } - /// Tries to simplify any constrained typevars in the intersection: - /// - /// - If the intersection contains a positive entry for exactly one of the constraints, we can - /// remove the typevar (effectively replacing it with that one positive constraint). - /// - /// - If the intersection contains negative entries for all but one of the constraints, we can - /// remove the negative constraints and replace the typevar with the remaining positive - /// constraint. - /// - /// - If the intersection contains negative entries for all of the constraints, the overall - /// intersection is `Never`. - fn simplify_constrained_typevars(&mut self, db: &'db dyn Db) { - let mut to_add = SmallVec::<[Type<'db>; 1]>::new(); - let mut positive_to_remove = SmallVec::<[usize; 1]>::new(); - - for (typevar_index, ty) in self.positive.iter().enumerate() { - let Type::TypeVar(bound_typevar) = ty else { - continue; - }; - let Some(TypeVarBoundOrConstraints::Constraints(constraints)) = - bound_typevar.typevar(db).bound_or_constraints(db) - else { - continue; - }; - - // Determine which constraints appear as positive entries in the intersection. Note - // that we shouldn't have duplicate entries in the positive or negative lists, so we - // don't need to worry about finding any particular constraint more than once. - let constraints = constraints.elements(db); - let mut positive_constraint_count = 0; - for (i, positive) in self.positive.iter().enumerate() { - if i == typevar_index { - continue; - } - - // This linear search should be fine as long as we don't encounter typevars with - // thousands of constraints. - positive_constraint_count += constraints - .iter() - .filter(|c| c.is_subtype_of(db, *positive)) - .count(); - } - - // If precisely one constraint appears as a positive element, we can replace the - // typevar with that positive constraint. - if positive_constraint_count == 1 { - positive_to_remove.push(typevar_index); - continue; - } - - // Determine which constraints appear as negative entries in the intersection. - let mut to_remove = Vec::with_capacity(constraints.len()); - let mut remaining_constraints: Vec<_> = constraints.iter().copied().map(Some).collect(); - for (negative_index, negative) in self.negative.iter().enumerate() { - // This linear search should be fine as long as we don't encounter typevars with - // thousands of constraints. - let matching_constraints = constraints - .iter() - .enumerate() - .filter(|(_, c)| c.is_subtype_of(db, *negative)); - for (constraint_index, _) in matching_constraints { - to_remove.push(negative_index); - remaining_constraints[constraint_index] = None; - } - } - - let mut iter = remaining_constraints.into_iter().flatten(); - let Some(remaining_constraint) = iter.next() else { - // All of the typevar constraints have been removed, so the entire intersection is - // `Never`. - *self = Self::default(); - self.positive.insert(Type::Never); - return; - }; - - let more_than_one_remaining_constraint = iter.next().is_some(); - if more_than_one_remaining_constraint { - // This typevar cannot be simplified. - continue; - } - - // Only one typevar constraint remains. Remove all of the negative constraints, and - // replace the typevar itself with the remaining positive constraint. - to_add.push(remaining_constraint); - positive_to_remove.push(typevar_index); - } - - // We don't need to sort the positive list, since we only append to it in increasing order. - for index in positive_to_remove.into_iter().rev() { - self.positive.swap_remove_index(index); - } - - for remaining_constraint in to_add { - self.add_positive(db, remaining_constraint); - } - } - fn build(mut self, db: &'db dyn Db) -> Type<'db> { - self.simplify_constrained_typevars(db); - - // If any typevars are in `self.positive`, speculatively solve all bounded type variables - // to their upper bound and all constrained type variables to the union of their constraints. - // If that speculative intersection simplifies to `Never`, this intersection must also simplify - // to `Never`. - if self.positive.iter().any(|ty| ty.is_type_var()) { - let mut speculative = IntersectionBuilder::new(db); - for pos in &self.positive { - match pos { - Type::TypeVar(type_var) => { - match type_var.typevar(db).bound_or_constraints(db) { - Some(TypeVarBoundOrConstraints::UpperBound(bound)) => { - speculative = speculative.add_positive(bound); - } - Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { - speculative = speculative.add_positive(Type::Union(constraints)); - } - // TypeVars without a bound or constraint implicitly have `object` as their - // upper bound, and it is always a no-op to add `object` to an intersection. - None => {} - } - } - _ => speculative = speculative.add_positive(*pos), - } - } - for neg in &self.negative { - speculative = speculative.add_negative(*neg); - } - if speculative.build().is_never() { - return Type::Never; - } - } - match (self.positive.len(), self.negative.len()) { (0, 0) => Type::object(), (1, 0) => self.positive[0], _ => { self.positive.shrink_to_fit(); self.negative.shrink_to_fit(); - Type::Intersection(IntersectionType::new(db, self.positive, self.negative)) + + let any_typevars_present = + self.positive.iter().any(|element| element.is_type_var()); + + let intersection = IntersectionType::new(db, self.positive, self.negative); + + if any_typevars_present + && intersection + .with_positive_typevars_solved_to_bounds_or_constraints(db) + .is_never() + { + Type::Never + } else { + Type::Intersection(intersection) + } } } }