From b1ede8885bb55608231c5a188ce21aaa1eaf653b Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Tue, 9 Dec 2025 20:33:07 -0500 Subject: [PATCH] add more comments --- .../ty_python_semantic/src/types/generics.rs | 26 +++++++++++++++++++ .../src/types/signatures.rs | 4 +++ 2 files changed, 30 insertions(+) diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index 39ca666eab..5aaf2aad96 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -556,6 +556,13 @@ impl<'db> GenericContext<'db> { let partial = PartialSpecialization { generic_context: self, types: &types, + // Don't recursively substitute type[i] in itself. Ideally, we could instead + // check if the result is self-referential after we're done applying the + // partial specialization. But when we apply a paramspec, we don't use the + // callable that it maps to directly; we create a new callable that reuses + // parts of it. That means we can't look for the previous type directly. + // Instead we use this to skip specializing the type in itself in the first + // place. skip: Some(i), }; let updated = types[i].apply_type_mapping( @@ -1362,6 +1369,8 @@ impl<'db> Specialization<'db> { pub struct PartialSpecialization<'a, 'db> { generic_context: GenericContext<'db>, types: &'a [Type<'db>], + /// An optional typevar to _not_ substitute when applying the specialization. We use this to + /// avoid recursively substituting a type inside of itself. skip: Option, } @@ -1471,6 +1480,14 @@ impl<'db> SpecializationBuilder<'db> { } } + /// Finds all of the valid specializations of a constraint set, and adds their type mappings to + /// the specialization that this builder is building up. + /// + /// TODO: This is a stopgap! Eventually, the builder will maintain a single constraint set for + /// the main specialization that we are building, and [`build`][Self::build] will build the + /// specialization directly from that constraint set. This method lets us migrate to that brave + /// new world incrementally, by using the new constraint set mechanism piecemeal for certain + /// type comparisons. fn add_type_mappings_from_constraint_set( &mut self, constraints: ConstraintSet<'db>, @@ -1526,6 +1543,15 @@ impl<'db> SpecializationBuilder<'db> { polarity: TypeVarVariance, mut f: &mut dyn FnMut(TypeVarAssignment<'db>) -> Option>, ) -> Result<(), SpecializationError<'db>> { + // TODO: Eventually, the builder will maintain a constraint set, instead of a hash-map of + // type mappings, to represent the specialization that we are building up. At that point, + // this method will just need to compare `actual ≤ formal`, using constraint set + // assignability, and AND the result into the constraint set we are building. + // + // To make progress on that migration, we use constraint set assignability whenever + // possible when adding any new heuristics here. See the `Callable` clause below for an + // example. + if formal == actual { return Ok(()); } diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index dd84a5c987..7baeae28dc 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -443,6 +443,10 @@ impl<'db> CallableSignature<'db> { let self_is_single_paramspec = Self::signatures_is_single_paramspec(self_signatures); let other_is_single_paramspec = Self::signatures_is_single_paramspec(other_signatures); + // If either callable is a ParamSpec, the constraint set should bind the ParamSpec to + // the other callable's signature. We also need to compare the return types — for + // instance, to verify in `Callable[P, int]` that the return type is assignable to + // `int`, or in `Callable[P, T]` to bind `T` to the return type of the other callable. match (self_is_single_paramspec, other_is_single_paramspec) { ( Some((self_bound_typevar, self_return_type)),