Skip to content

Commit cf2b083

Browse files
dcreagersharkdp
andauthored
[ty] Apply type mappings to functions eagerly (#20596)
`TypeMapping` is no longer cow-shaped. Before, `TypeMapping` defined a `to_owned` method, which would make an owned copy of the type mapping. This let us apply type mappings to function literals lazily. The primary part of a function that you have to apply the type mapping to is its signature. The hypothesis was that doing this lazily would prevent us from constructing the signature of a function just to apply a type mapping; if you never ended up needed the updated function signature, that would be extraneous work. But looking at the CI for this PR, it looks like that hypothesis is wrong! And this definitely cleans up the code quite a bit. It also means that over time we can consider replacing all of these `TypeMapping` enum variants with separate `TypeTransformer` impls. --------- Co-authored-by: David Peter <mail@david-peter.de>
1 parent 3f640da commit cf2b083

File tree

5 files changed

+105
-198
lines changed

5 files changed

+105
-198
lines changed

crates/ty_python_semantic/src/types.rs

Lines changed: 5 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ use crate::types::function::{
5353
};
5454
use crate::types::generics::{
5555
GenericContext, PartialSpecialization, Specialization, bind_typevar, walk_generic_context,
56-
walk_partial_specialization, walk_specialization,
5756
};
5857
pub use crate::types::ide_support::{
5958
CallSignatureDetails, Member, MemberWithDefinition, all_members, call_signature_details,
@@ -6109,7 +6108,7 @@ impl<'db> Type<'db> {
61096108
}
61106109

61116110
Type::FunctionLiteral(function) => {
6112-
let function = Type::FunctionLiteral(function.with_type_mapping(db, type_mapping));
6111+
let function = Type::FunctionLiteral(function.apply_type_mapping_impl(db, type_mapping, visitor));
61136112

61146113
match type_mapping {
61156114
TypeMapping::PromoteLiterals => function.literal_promotion_type(db)
@@ -6120,8 +6119,8 @@ impl<'db> Type<'db> {
61206119

61216120
Type::BoundMethod(method) => Type::BoundMethod(BoundMethodType::new(
61226121
db,
6123-
method.function(db).with_type_mapping(db, type_mapping),
6124-
method.self_instance(db).apply_type_mapping(db, type_mapping),
6122+
method.function(db).apply_type_mapping_impl(db, type_mapping, visitor),
6123+
method.self_instance(db).apply_type_mapping_impl(db, type_mapping, visitor),
61256124
)),
61266125

61276126
Type::NominalInstance(instance) =>
@@ -6140,13 +6139,13 @@ impl<'db> Type<'db> {
61406139

61416140
Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderGet(function)) => {
61426141
Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderGet(
6143-
function.with_type_mapping(db, type_mapping),
6142+
function.apply_type_mapping_impl(db, type_mapping, visitor),
61446143
))
61456144
}
61466145

61476146
Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderCall(function)) => {
61486147
Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderCall(
6149-
function.with_type_mapping(db, type_mapping),
6148+
function.apply_type_mapping_impl(db, type_mapping, visitor),
61506149
))
61516150
}
61526151

@@ -6782,84 +6781,7 @@ pub enum TypeMapping<'a, 'db> {
67826781
Materialize(MaterializationKind),
67836782
}
67846783

6785-
fn walk_type_mapping<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
6786-
db: &'db dyn Db,
6787-
mapping: &TypeMapping<'_, 'db>,
6788-
visitor: &V,
6789-
) {
6790-
match mapping {
6791-
TypeMapping::Specialization(specialization) => {
6792-
walk_specialization(db, *specialization, visitor);
6793-
}
6794-
TypeMapping::PartialSpecialization(specialization) => {
6795-
walk_partial_specialization(db, specialization, visitor);
6796-
}
6797-
TypeMapping::BindSelf(self_type) => {
6798-
visitor.visit_type(db, *self_type);
6799-
}
6800-
TypeMapping::ReplaceSelf { new_upper_bound } => {
6801-
visitor.visit_type(db, *new_upper_bound);
6802-
}
6803-
TypeMapping::PromoteLiterals
6804-
| TypeMapping::BindLegacyTypevars(_)
6805-
| TypeMapping::MarkTypeVarsInferable(_)
6806-
| TypeMapping::Materialize(_) => {}
6807-
}
6808-
}
6809-
68106784
impl<'db> TypeMapping<'_, 'db> {
6811-
fn to_owned(&self) -> TypeMapping<'db, 'db> {
6812-
match self {
6813-
TypeMapping::Specialization(specialization) => {
6814-
TypeMapping::Specialization(*specialization)
6815-
}
6816-
TypeMapping::PartialSpecialization(partial) => {
6817-
TypeMapping::PartialSpecialization(partial.to_owned())
6818-
}
6819-
TypeMapping::PromoteLiterals => TypeMapping::PromoteLiterals,
6820-
TypeMapping::BindLegacyTypevars(binding_context) => {
6821-
TypeMapping::BindLegacyTypevars(*binding_context)
6822-
}
6823-
TypeMapping::BindSelf(self_type) => TypeMapping::BindSelf(*self_type),
6824-
TypeMapping::ReplaceSelf { new_upper_bound } => TypeMapping::ReplaceSelf {
6825-
new_upper_bound: *new_upper_bound,
6826-
},
6827-
TypeMapping::MarkTypeVarsInferable(binding_context) => {
6828-
TypeMapping::MarkTypeVarsInferable(*binding_context)
6829-
}
6830-
TypeMapping::Materialize(materialization_kind) => {
6831-
TypeMapping::Materialize(*materialization_kind)
6832-
}
6833-
}
6834-
}
6835-
6836-
fn normalized_impl(&self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self {
6837-
match self {
6838-
TypeMapping::Specialization(specialization) => {
6839-
TypeMapping::Specialization(specialization.normalized_impl(db, visitor))
6840-
}
6841-
TypeMapping::PartialSpecialization(partial) => {
6842-
TypeMapping::PartialSpecialization(partial.normalized_impl(db, visitor))
6843-
}
6844-
TypeMapping::PromoteLiterals => TypeMapping::PromoteLiterals,
6845-
TypeMapping::BindLegacyTypevars(binding_context) => {
6846-
TypeMapping::BindLegacyTypevars(*binding_context)
6847-
}
6848-
TypeMapping::BindSelf(self_type) => {
6849-
TypeMapping::BindSelf(self_type.normalized_impl(db, visitor))
6850-
}
6851-
TypeMapping::ReplaceSelf { new_upper_bound } => TypeMapping::ReplaceSelf {
6852-
new_upper_bound: new_upper_bound.normalized_impl(db, visitor),
6853-
},
6854-
TypeMapping::MarkTypeVarsInferable(binding_context) => {
6855-
TypeMapping::MarkTypeVarsInferable(*binding_context)
6856-
}
6857-
TypeMapping::Materialize(materialization_kind) => {
6858-
TypeMapping::Materialize(*materialization_kind)
6859-
}
6860-
}
6861-
}
6862-
68636785
/// Update the generic context of a [`Signature`] according to the current type mapping
68646786
pub(crate) fn update_signature_generic_context(
68656787
&self,

crates/ty_python_semantic/src/types/function.rs

Lines changed: 85 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,11 @@ use crate::types::narrow::ClassInfoConstraintFunction;
7777
use crate::types::signatures::{CallableSignature, Signature};
7878
use crate::types::visitor::any_over_type;
7979
use crate::types::{
80-
BoundMethodType, BoundTypeVarInstance, CallableType, ClassBase, ClassLiteral, ClassType,
81-
DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor,
82-
IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, SpecialFormType,
83-
TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, all_members,
84-
binding_type, todo_type, walk_type_mapping,
80+
ApplyTypeMappingVisitor, BoundMethodType, BoundTypeVarInstance, CallableType, ClassBase,
81+
ClassLiteral, ClassType, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor,
82+
HasRelationToVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor,
83+
SpecialFormType, TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation,
84+
UnionBuilder, all_members, binding_type, todo_type, walk_signature,
8585
};
8686
use crate::{Db, FxOrderSet, ModuleName, resolve_module};
8787

@@ -623,33 +623,24 @@ impl<'db> FunctionLiteral<'db> {
623623
/// calling query is not in the same file as this function is defined in, then this will create
624624
/// a cross-module dependency directly on the full AST which will lead to cache
625625
/// over-invalidation.
626-
fn signature<'a>(
627-
self,
628-
db: &'db dyn Db,
629-
type_mappings: &'a [TypeMapping<'a, 'db>],
630-
) -> CallableSignature<'db>
631-
where
632-
'db: 'a,
633-
{
626+
fn signature(self, db: &'db dyn Db) -> CallableSignature<'db> {
634627
// We only include an implementation (i.e. a definition not decorated with `@overload`) if
635628
// it's the only definition.
636629
let inherited_generic_context = self.inherited_generic_context(db);
637630
let (overloads, implementation) = self.overloads_and_implementation(db);
638631
if let Some(implementation) = implementation {
639632
if overloads.is_empty() {
640-
return CallableSignature::single(type_mappings.iter().fold(
633+
return CallableSignature::single(
641634
implementation.signature(db, inherited_generic_context),
642-
|sig, mapping| sig.apply_type_mapping(db, mapping),
643-
));
635+
);
644636
}
645637
}
646638

647-
CallableSignature::from_overloads(overloads.iter().map(|overload| {
648-
type_mappings.iter().fold(
649-
overload.signature(db, inherited_generic_context),
650-
|sig, mapping| sig.apply_type_mapping(db, mapping),
651-
)
652-
}))
639+
CallableSignature::from_overloads(
640+
overloads
641+
.iter()
642+
.map(|overload| overload.signature(db, inherited_generic_context)),
643+
)
653644
}
654645

655646
/// Typed externally-visible signature of the last overload or implementation of this function.
@@ -660,20 +651,10 @@ impl<'db> FunctionLiteral<'db> {
660651
/// calling query is not in the same file as this function is defined in, then this will create
661652
/// a cross-module dependency directly on the full AST which will lead to cache
662653
/// over-invalidation.
663-
fn last_definition_signature<'a>(
664-
self,
665-
db: &'db dyn Db,
666-
type_mappings: &'a [TypeMapping<'a, 'db>],
667-
) -> Signature<'db>
668-
where
669-
'db: 'a,
670-
{
654+
fn last_definition_signature(self, db: &'db dyn Db) -> Signature<'db> {
671655
let inherited_generic_context = self.inherited_generic_context(db);
672-
type_mappings.iter().fold(
673-
self.last_definition(db)
674-
.signature(db, inherited_generic_context),
675-
|sig, mapping| sig.apply_type_mapping(db, mapping),
676-
)
656+
self.last_definition(db)
657+
.signature(db, inherited_generic_context)
677658
}
678659

679660
fn normalized_impl(self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self {
@@ -691,11 +672,19 @@ impl<'db> FunctionLiteral<'db> {
691672
pub struct FunctionType<'db> {
692673
pub(crate) literal: FunctionLiteral<'db>,
693674

694-
/// Type mappings that should be applied to the function's parameter and return types. This
695-
/// might include specializations of enclosing generic contexts (e.g. for non-generic methods
696-
/// of a specialized generic class).
697-
#[returns(deref)]
698-
type_mappings: Box<[TypeMapping<'db, 'db>]>,
675+
/// Contains a potentially modified signature for this function literal, in case certain operations
676+
/// (like type mappings) have been applied to it.
677+
///
678+
/// See also: [`FunctionLiteral::updated_signature`].
679+
#[returns(as_ref)]
680+
updated_signature: Option<CallableSignature<'db>>,
681+
682+
/// Contains a potentially modified signature for the last overload or the implementation of this
683+
/// function literal, in case certain operations (like type mappings) have been applied to it.
684+
///
685+
/// See also: [`FunctionLiteral::last_definition_signature`].
686+
#[returns(as_ref)]
687+
updated_last_definition_signature: Option<Signature<'db>>,
699688
}
700689

701690
// The Salsa heap is tracked separately.
@@ -707,8 +696,13 @@ pub(super) fn walk_function_type<'db, V: super::visitor::TypeVisitor<'db> + ?Siz
707696
visitor: &V,
708697
) {
709698
walk_function_literal(db, function.literal(db), visitor);
710-
for mapping in function.type_mappings(db) {
711-
walk_type_mapping(db, mapping, visitor);
699+
if let Some(callable_signature) = function.updated_signature(db) {
700+
for signature in &callable_signature.overloads {
701+
walk_signature(db, signature, visitor);
702+
}
703+
}
704+
if let Some(signature) = function.updated_last_definition_signature(db) {
705+
walk_signature(db, signature, visitor);
712706
}
713707
}
714708

@@ -722,21 +716,41 @@ impl<'db> FunctionType<'db> {
722716
let literal = self
723717
.literal(db)
724718
.with_inherited_generic_context(db, inherited_generic_context);
725-
Self::new(db, literal, self.type_mappings(db))
719+
let updated_signature = self.updated_signature(db).map(|signature| {
720+
signature.with_inherited_generic_context(Some(inherited_generic_context))
721+
});
722+
let updated_last_definition_signature =
723+
self.updated_last_definition_signature(db).map(|signature| {
724+
signature
725+
.clone()
726+
.with_inherited_generic_context(Some(inherited_generic_context))
727+
});
728+
Self::new(
729+
db,
730+
literal,
731+
updated_signature,
732+
updated_last_definition_signature,
733+
)
726734
}
727735

728-
pub(crate) fn with_type_mapping<'a>(
736+
pub(crate) fn apply_type_mapping_impl<'a>(
729737
self,
730738
db: &'db dyn Db,
731739
type_mapping: &TypeMapping<'a, 'db>,
740+
visitor: &ApplyTypeMappingVisitor<'db>,
732741
) -> Self {
733-
let type_mappings: Box<[_]> = self
734-
.type_mappings(db)
735-
.iter()
736-
.cloned()
737-
.chain(std::iter::once(type_mapping.to_owned()))
738-
.collect();
739-
Self::new(db, self.literal(db), type_mappings)
742+
let updated_signature =
743+
self.signature(db)
744+
.apply_type_mapping_impl(db, type_mapping, visitor);
745+
let updated_last_definition_signature = self
746+
.last_definition_signature(db)
747+
.apply_type_mapping_impl(db, type_mapping, visitor);
748+
Self::new(
749+
db,
750+
self.literal(db),
751+
Some(updated_signature),
752+
Some(updated_last_definition_signature),
753+
)
740754
}
741755

742756
pub(crate) fn with_dataclass_transformer_params(
@@ -752,7 +766,7 @@ impl<'db> FunctionType<'db> {
752766
.with_dataclass_transformer_params(db, params);
753767
let literal =
754768
FunctionLiteral::new(db, last_definition, literal.inherited_generic_context(db));
755-
Self::new(db, literal, self.type_mappings(db))
769+
Self::new(db, literal, None, None)
756770
}
757771

758772
/// Returns the [`File`] in which this function is defined.
@@ -907,7 +921,9 @@ impl<'db> FunctionType<'db> {
907921
/// would depend on the function's AST and rerun for every change in that file.
908922
#[salsa::tracked(returns(ref), cycle_fn=signature_cycle_recover, cycle_initial=signature_cycle_initial, heap_size=ruff_memory_usage::heap_size)]
909923
pub(crate) fn signature(self, db: &'db dyn Db) -> CallableSignature<'db> {
910-
self.literal(db).signature(db, self.type_mappings(db))
924+
self.updated_signature(db)
925+
.cloned()
926+
.unwrap_or_else(|| self.literal(db).signature(db))
911927
}
912928

913929
/// Typed externally-visible signature of the last overload or implementation of this function.
@@ -926,8 +942,9 @@ impl<'db> FunctionType<'db> {
926942
heap_size=ruff_memory_usage::heap_size,
927943
)]
928944
pub(crate) fn last_definition_signature(self, db: &'db dyn Db) -> Signature<'db> {
929-
self.literal(db)
930-
.last_definition_signature(db, self.type_mappings(db))
945+
self.updated_last_definition_signature(db)
946+
.cloned()
947+
.unwrap_or_else(|| self.literal(db).last_definition_signature(db))
931948
}
932949

933950
/// Convert the `FunctionType` into a [`CallableType`].
@@ -1017,12 +1034,19 @@ impl<'db> FunctionType<'db> {
10171034
}
10181035

10191036
pub(crate) fn normalized_impl(self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self {
1020-
let mappings: Box<_> = self
1021-
.type_mappings(db)
1022-
.iter()
1023-
.map(|mapping| mapping.normalized_impl(db, visitor))
1024-
.collect();
1025-
Self::new(db, self.literal(db).normalized_impl(db, visitor), mappings)
1037+
let literal = self.literal(db).normalized_impl(db, visitor);
1038+
let updated_signature = self
1039+
.updated_signature(db)
1040+
.map(|signature| signature.normalized_impl(db, visitor));
1041+
let updated_last_definition_signature = self
1042+
.updated_last_definition_signature(db)
1043+
.map(|signature| signature.normalized_impl(db, visitor));
1044+
Self::new(
1045+
db,
1046+
literal,
1047+
updated_signature,
1048+
updated_last_definition_signature,
1049+
)
10261050
}
10271051
}
10281052

0 commit comments

Comments
 (0)