Skip to content

Commit 2bcbc3e

Browse files
JelleZijlstrasecond-ed
authored andcommitted
[ty] Simplify materialization of specialized generics (astral-sh#20121)
This is a variant of astral-sh#20076 that moves some complexity out of `apply_type_mapping_impl` in `generics.rs`. The tradeoff is that now every place that applies `TypeMapping::Specialization` must take care to call `.materialize()` afterwards. (A previous version of this didn't work because I had missed a spot where I had to call `.materialize()`.) @carljm as asked in astral-sh#20076 (comment) .
1 parent dafd6ef commit 2bcbc3e

File tree

3 files changed

+33
-55
lines changed

3 files changed

+33
-55
lines changed

crates/ty_python_semantic/src/types.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5970,7 +5970,12 @@ impl<'db> Type<'db> {
59705970
db: &'db dyn Db,
59715971
specialization: Specialization<'db>,
59725972
) -> Type<'db> {
5973-
self.apply_type_mapping(db, &TypeMapping::Specialization(specialization))
5973+
let new_specialization =
5974+
self.apply_type_mapping(db, &TypeMapping::Specialization(specialization));
5975+
match specialization.materialization_kind(db) {
5976+
None => new_specialization,
5977+
Some(materialization_kind) => new_specialization.materialize(db, materialization_kind),
5978+
}
59745979
}
59755980

59765981
fn apply_type_mapping<'a>(
@@ -6713,13 +6718,6 @@ impl<'db> TypeMapping<'_, 'db> {
67136718
}
67146719
}
67156720
}
6716-
6717-
fn materialization_kind(&self, db: &'db dyn Db) -> Option<MaterializationKind> {
6718-
match self {
6719-
TypeMapping::Specialization(specialization) => specialization.materialization_kind(db),
6720-
_ => None,
6721-
}
6722-
}
67236721
}
67246722

67256723
/// Singleton types that are heavily special-cased by ty. Despite its name,

crates/ty_python_semantic/src/types/class_base.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ use crate::types::generics::Specialization;
44
use crate::types::tuple::TupleType;
55
use crate::types::{
66
ApplyTypeMappingVisitor, ClassLiteral, ClassType, DynamicType, KnownClass, KnownInstanceType,
7-
MroError, MroIterator, NormalizedVisitor, SpecialFormType, Type, TypeMapping, todo_type,
7+
MaterializationKind, MroError, MroIterator, NormalizedVisitor, SpecialFormType, Type,
8+
TypeMapping, todo_type,
89
};
910

1011
/// Enumeration of the possible kinds of types we allow in class bases.
@@ -286,16 +287,30 @@ impl<'db> ClassBase<'db> {
286287
specialization: Option<Specialization<'db>>,
287288
) -> Self {
288289
if let Some(specialization) = specialization {
289-
self.apply_type_mapping_impl(
290+
let new_self = self.apply_type_mapping_impl(
290291
db,
291292
&TypeMapping::Specialization(specialization),
292293
&ApplyTypeMappingVisitor::default(),
293-
)
294+
);
295+
match specialization.materialization_kind(db) {
296+
None => new_self,
297+
Some(materialization_kind) => new_self.materialize(db, materialization_kind),
298+
}
294299
} else {
295300
self
296301
}
297302
}
298303

304+
fn materialize(self, db: &'db dyn Db, kind: MaterializationKind) -> Self {
305+
match self {
306+
ClassBase::Class(class) => Self::Class(class.materialize(db, kind)),
307+
ClassBase::Dynamic(_)
308+
| ClassBase::Generic
309+
| ClassBase::Protocol
310+
| ClassBase::TypedDict => self,
311+
}
312+
}
313+
299314
pub(super) fn has_cyclic_mro(self, db: &'db dyn Db) -> bool {
300315
match self {
301316
ClassBase::Class(class) => {

crates/ty_python_semantic/src/types/generics.rs

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,11 @@ impl<'db> Specialization<'db> {
581581
/// That lets us produce the generic alias `A[int]`, which is the corresponding entry in the
582582
/// MRO of `B[int]`.
583583
pub(crate) fn apply_specialization(self, db: &'db dyn Db, other: Specialization<'db>) -> Self {
584-
self.apply_type_mapping(db, &TypeMapping::Specialization(other))
584+
let new_specialization = self.apply_type_mapping(db, &TypeMapping::Specialization(other));
585+
match other.materialization_kind(db) {
586+
None => new_specialization,
587+
Some(materialization_kind) => new_specialization.materialize(db, materialization_kind),
588+
}
585589
}
586590

587591
pub(crate) fn apply_type_mapping<'a>(
@@ -598,59 +602,20 @@ impl<'db> Specialization<'db> {
598602
type_mapping: &TypeMapping<'a, 'db>,
599603
visitor: &ApplyTypeMappingVisitor<'db>,
600604
) -> Self {
601-
// TODO it seems like this should be possible to do in a much simpler way in
602-
// `Self::apply_specialization`; just apply the type mapping to create the new
603-
// specialization, then materialize the new specialization appropriately, if the type
604-
// mapping is a materialization. But this doesn't work; see discussion in
605-
// https://github.com/astral-sh/ruff/pull/20076
606-
let applied_materialization_kind = type_mapping.materialization_kind(db);
607-
let mut has_dynamic_invariant_typevar = false;
608605
let types: Box<[_]> = self
609-
.generic_context(db)
610-
.variables(db)
611-
.into_iter()
612-
.zip(self.types(db))
613-
.map(|(bound_typevar, vartype)| {
614-
let ty = vartype.apply_type_mapping_impl(db, type_mapping, visitor);
615-
match (applied_materialization_kind, bound_typevar.variance(db)) {
616-
(None, _) => ty,
617-
(Some(_), TypeVarVariance::Bivariant) =>
618-
// With bivariance, all specializations are subtypes of each other,
619-
// so any materialization is acceptable.
620-
{
621-
ty.materialize(db, MaterializationKind::Top)
622-
}
623-
(Some(materialization_kind), TypeVarVariance::Covariant) => {
624-
ty.materialize(db, materialization_kind)
625-
}
626-
(Some(materialization_kind), TypeVarVariance::Contravariant) => {
627-
ty.materialize(db, materialization_kind.flip())
628-
}
629-
(Some(_), TypeVarVariance::Invariant) => {
630-
let top_materialization = ty.materialize(db, MaterializationKind::Top);
631-
if !ty.is_equivalent_to(db, top_materialization) {
632-
has_dynamic_invariant_typevar = true;
633-
}
634-
ty
635-
}
636-
}
637-
})
606+
.types(db)
607+
.iter()
608+
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, visitor))
638609
.collect();
639610

640611
let tuple_inner = self
641612
.tuple_inner(db)
642613
.and_then(|tuple| tuple.apply_type_mapping_impl(db, type_mapping, visitor));
643-
let new_materialization_kind = if has_dynamic_invariant_typevar {
644-
self.materialization_kind(db)
645-
.or(applied_materialization_kind)
646-
} else {
647-
None
648-
};
649614
Specialization::new(
650615
db,
651616
self.generic_context(db),
652617
types,
653-
new_materialization_kind,
618+
self.materialization_kind(db),
654619
tuple_inner,
655620
)
656621
}

0 commit comments

Comments
 (0)