[ty] propagate visitors and constraints through has_relation_in_invariant_position (#20259)
Some checks failed
CI / Determine changes (push) Has been cancelled
CI / cargo fmt (push) Has been cancelled
CI / cargo build (release) (push) Has been cancelled
CI / python package (push) Has been cancelled
CI / pre-commit (push) Has been cancelled
CI / mkdocs (push) Has been cancelled
[ty Playground] Release / publish (push) Has been cancelled
CI / cargo clippy (push) Has been cancelled
CI / cargo test (linux) (push) Has been cancelled
CI / cargo test (linux, release) (push) Has been cancelled
CI / cargo test (windows) (push) Has been cancelled
CI / cargo test (wasm) (push) Has been cancelled
CI / cargo build (msrv) (push) Has been cancelled
CI / cargo fuzz build (push) Has been cancelled
CI / fuzz parser (push) Has been cancelled
CI / test scripts (push) Has been cancelled
CI / ecosystem (push) Has been cancelled
CI / Fuzz for new ty panics (push) Has been cancelled
CI / cargo shear (push) Has been cancelled
CI / formatter instabilities and black similarity (push) Has been cancelled
CI / test ruff-lsp (push) Has been cancelled
CI / check playground (push) Has been cancelled
CI / benchmarks-instrumented (push) Has been cancelled
CI / benchmarks-walltime (push) Has been cancelled

## Summary

The sub-checks for assignability and subtyping of materializations
performed in `has_relation_in_invariant_position` and
`is_subtype_in_invariant_position` need to propagate the
`HasRelationToVisitor`, or we can stack overflow.

A side effect of this change is that we also propagate the
`ConstraintSet` through, rather than using `C::from_bool`, which I think
may also become important for correctness in cases involving type
variables (though it isn't testable yet, since we aren't yet actually
creating constraints other than always-true and always-false.)

## Test Plan

Added mdtest (derived from code found in pydantic) which
stack-overflowed before this PR.

With this change incorporated, pydantic now checks successfully on my
draft PR for PEP 613 TypeAlias support.
This commit is contained in:
Carl Meyer 2025-09-05 17:17:17 -07:00 committed by GitHub
parent a27c64811e
commit 2467c4352e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 74 additions and 40 deletions

View file

@ -181,7 +181,8 @@ fn definition_expression_type<'db>(
pub(crate) type ApplyTypeMappingVisitor<'db> = TypeTransformer<'db, TypeMapping<'db, 'db>>;
/// A [`PairVisitor`] that is used in `has_relation_to` methods.
pub(crate) type HasRelationToVisitor<'db, C> = PairVisitor<'db, TypeRelation, C>;
pub(crate) type HasRelationToVisitor<'db, C> =
CycleDetector<TypeRelation, (Type<'db>, Type<'db>, TypeRelation), C>;
/// A [`PairVisitor`] that is used in `is_disjoint_from` methods.
pub(crate) type IsDisjointVisitor<'db, C> = PairVisitor<'db, IsDisjoint, C>;
@ -1359,13 +1360,13 @@ impl<'db> Type<'db> {
C::from_bool(db, relation.is_assignability())
}
(Type::TypeAlias(self_alias), _) => visitor.visit((self, target), || {
(Type::TypeAlias(self_alias), _) => visitor.visit((self, target, relation), || {
self_alias
.value_type(db)
.has_relation_to_impl(db, target, relation, visitor)
}),
(_, Type::TypeAlias(target_alias)) => visitor.visit((self, target), || {
(_, Type::TypeAlias(target_alias)) => visitor.visit((self, target, relation), || {
self.has_relation_to_impl(db, target_alias.value_type(db), relation, visitor)
}),
@ -1750,7 +1751,7 @@ impl<'db> Type<'db> {
// `bool` is a subtype of `int`, because `bool` subclasses `int`,
// which means that all instances of `bool` are also instances of `int`
(Type::NominalInstance(self_instance), Type::NominalInstance(target_instance)) => {
visitor.visit((self, target), || {
visitor.visit((self, target, relation), || {
self_instance.has_relation_to_impl(db, target_instance, relation, visitor)
})
}
@ -8734,7 +8735,7 @@ impl<'db> ConstructorCallError<'db> {
}
}
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub(crate) enum TypeRelation {
Subtyping,
Assignability,

View file

@ -451,45 +451,50 @@ fn is_subtype_in_invariant_position<'db, C: Constraints<'db>>(
derived_materialization: MaterializationKind,
base_type: &Type<'db>,
base_materialization: MaterializationKind,
visitor: &HasRelationToVisitor<'db, C>,
) -> C {
let derived_top = derived_type.top_materialization(db);
let derived_bottom = derived_type.bottom_materialization(db);
let base_top = base_type.top_materialization(db);
let base_bottom = base_type.bottom_materialization(db);
let is_subtype_of = |derived: Type<'db>, base: Type<'db>| {
derived.has_relation_to_impl(db, base, TypeRelation::Subtyping, visitor)
};
match (derived_materialization, base_materialization) {
// `Derived` is a subtype of `Base` if the range of materializations covered by `Derived`
// is a subset of the range covered by `Base`.
(MaterializationKind::Top, MaterializationKind::Top) => C::from_bool(
db,
base_bottom.is_subtype_of(db, derived_bottom)
&& derived_top.is_subtype_of(db, base_top),
),
(MaterializationKind::Top, MaterializationKind::Top) => {
is_subtype_of(base_bottom, derived_bottom)
.and(db, || is_subtype_of(derived_top, base_top))
}
// One bottom is a subtype of another if it covers a strictly larger set of materializations.
(MaterializationKind::Bottom, MaterializationKind::Bottom) => C::from_bool(
db,
derived_bottom.is_subtype_of(db, base_bottom)
&& base_top.is_subtype_of(db, derived_top),
),
(MaterializationKind::Bottom, MaterializationKind::Bottom) => {
is_subtype_of(derived_bottom, base_bottom)
.and(db, || is_subtype_of(base_top, derived_top))
}
// The bottom materialization of `Derived` is a subtype of the top materialization
// of `Base` if there is some type that is both within the
// range of types covered by derived and within the range covered by base, because if such a type
// exists, it's a subtype of `Top[base]` and a supertype of `Bottom[derived]`.
(MaterializationKind::Bottom, MaterializationKind::Top) => C::from_bool(
db,
(base_bottom.is_subtype_of(db, derived_bottom)
&& derived_bottom.is_subtype_of(db, base_top))
|| (base_bottom.is_subtype_of(db, derived_top)
&& derived_top.is_subtype_of(db, base_top)
|| (base_top.is_subtype_of(db, derived_top)
&& derived_bottom.is_subtype_of(db, base_top))),
),
(MaterializationKind::Bottom, MaterializationKind::Top) => {
(is_subtype_of(base_bottom, derived_bottom)
.and(db, || is_subtype_of(derived_bottom, base_top)))
.or(db, || {
is_subtype_of(base_bottom, derived_top)
.and(db, || is_subtype_of(derived_top, base_top))
})
.or(db, || {
is_subtype_of(base_top, derived_top)
.and(db, || is_subtype_of(derived_bottom, base_top))
})
}
// A top materialization is a subtype of a bottom materialization only if both original
// un-materialized types are the same fully static type.
(MaterializationKind::Top, MaterializationKind::Bottom) => C::from_bool(
db,
derived_top.is_subtype_of(db, base_bottom)
&& base_top.is_subtype_of(db, derived_bottom),
),
(MaterializationKind::Top, MaterializationKind::Bottom) => {
is_subtype_of(derived_top, base_bottom)
.and(db, || is_subtype_of(base_top, derived_bottom))
}
}
}
@ -503,23 +508,32 @@ fn has_relation_in_invariant_position<'db, C: Constraints<'db>>(
base_type: &Type<'db>,
base_materialization: Option<MaterializationKind>,
relation: TypeRelation,
visitor: &HasRelationToVisitor<'db, C>,
) -> C {
match (derived_materialization, base_materialization, relation) {
// Top and bottom materializations are fully static types, so subtyping
// is the same as assignability.
(Some(derived_mat), Some(base_mat), _) => {
is_subtype_in_invariant_position(db, derived_type, derived_mat, base_type, base_mat)
}
(Some(derived_mat), Some(base_mat), _) => is_subtype_in_invariant_position(
db,
derived_type,
derived_mat,
base_type,
base_mat,
visitor,
),
// Subtyping between invariant type parameters without a top/bottom materialization involved
// is equivalence
(None, None, TypeRelation::Subtyping) => {
C::from_bool(db, derived_type.is_equivalent_to(db, *base_type))
}
(None, None, TypeRelation::Assignability) => C::from_bool(
db,
derived_type.is_assignable_to(db, *base_type)
&& base_type.is_assignable_to(db, *derived_type),
),
(None, None, TypeRelation::Subtyping) => derived_type.when_equivalent_to(db, *base_type),
(None, None, TypeRelation::Assignability) => derived_type
.has_relation_to_impl(db, *base_type, TypeRelation::Assignability, visitor)
.and(db, || {
base_type.has_relation_to_impl(
db,
*derived_type,
TypeRelation::Assignability,
visitor,
)
}),
// For gradual types, A <: B (subtyping) is defined as Top[A] <: Bottom[B]
(None, Some(base_mat), TypeRelation::Subtyping) => is_subtype_in_invariant_position(
db,
@ -527,6 +541,7 @@ fn has_relation_in_invariant_position<'db, C: Constraints<'db>>(
MaterializationKind::Top,
base_type,
base_mat,
visitor,
),
(Some(derived_mat), None, TypeRelation::Subtyping) => is_subtype_in_invariant_position(
db,
@ -534,6 +549,7 @@ fn has_relation_in_invariant_position<'db, C: Constraints<'db>>(
derived_mat,
base_type,
MaterializationKind::Bottom,
visitor,
),
// And A <~ B (assignability) is Bottom[A] <: Top[B]
(None, Some(base_mat), TypeRelation::Assignability) => is_subtype_in_invariant_position(
@ -542,6 +558,7 @@ fn has_relation_in_invariant_position<'db, C: Constraints<'db>>(
MaterializationKind::Bottom,
base_type,
base_mat,
visitor,
),
(Some(derived_mat), None, TypeRelation::Assignability) => is_subtype_in_invariant_position(
db,
@ -549,6 +566,7 @@ fn has_relation_in_invariant_position<'db, C: Constraints<'db>>(
derived_mat,
base_type,
MaterializationKind::Top,
visitor,
),
}
}
@ -805,6 +823,7 @@ impl<'db> Specialization<'db> {
other_type,
other_materialization_kind,
relation,
visitor,
),
TypeVarVariance::Covariant => {
self_type.has_relation_to_impl(db, *other_type, relation, visitor)