Skip to content

Commit c4b4c53

Browse files
committed
temporary fix for typevar identity with mark_inferable
1 parent 085e81f commit c4b4c53

File tree

4 files changed

+87
-15
lines changed

4 files changed

+87
-15
lines changed

crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,4 +411,27 @@ class D(Generic[V]):
411411
reveal_type(D().x) # revealed: V@D
412412
```
413413

414+
## Regression
415+
416+
### Use of typevar with default inside a function body that binds it
417+
418+
```toml
419+
[environment]
420+
python-version = "3.13"
421+
```
422+
423+
```py
424+
from typing import Generic, TypeVar
425+
426+
_DataT = TypeVar("_DataT", bound=int, default=int)
427+
428+
class Event(Generic[_DataT]):
429+
def __init__(self, data: _DataT) -> None:
430+
self.data = data
431+
432+
def async_fire_internal(event_data: _DataT):
433+
event: Event[_DataT] | None = None
434+
event = Event(event_data)
435+
```
436+
414437
[generics]: https://typing.python.org/en/latest/spec/generics.html

crates/ty_python_semantic/src/types.rs

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,7 +1600,9 @@ impl<'db> Type<'db> {
16001600
(
16011601
Type::NonInferableTypeVar(lhs_bound_typevar),
16021602
Type::NonInferableTypeVar(rhs_bound_typevar),
1603-
) if lhs_bound_typevar == rhs_bound_typevar => ConstraintSet::from(true),
1603+
) if lhs_bound_typevar.is_identical_to(db, rhs_bound_typevar) => {
1604+
ConstraintSet::from(true)
1605+
}
16041606

16051607
// A fully static typevar is a subtype of its upper bound, and to something similar to
16061608
// the union of its constraints. An unbound, unconstrained, fully static typevar has an
@@ -7686,6 +7688,12 @@ pub struct TypeVarInstance<'db> {
76867688
_default: Option<TypeVarDefaultEvaluation<'db>>,
76877689

76887690
pub kind: TypeVarKind,
7691+
7692+
/// If this typevar was transformed from another typevar via `mark_typevars_inferable`, this
7693+
/// records the identity of the "original" typevar, so we can recognize them as the same
7694+
/// typevar in `bind_typevar`. TODO: this (and the `is_identical_to` methods) should be
7695+
/// removable once we remove `mark_typevars_inferable`.
7696+
pub(crate) original: Option<TypeVarInstance<'db>>,
76897697
}
76907698

76917699
// The Salsa heap is tracked separately.
@@ -7796,6 +7804,7 @@ impl<'db> TypeVarInstance<'db> {
77967804
.map(|ty| ty.normalized_impl(db, visitor).into()),
77977805
}),
77987806
self.kind(db),
7807+
self.original(db),
77997808
)
78007809
}
78017810

@@ -7841,6 +7850,7 @@ impl<'db> TypeVarInstance<'db> {
78417850
.map(|ty| ty.materialize(db, materialization_kind, visitor).into()),
78427851
}),
78437852
self.kind(db),
7853+
self.original(db),
78447854
)
78457855
}
78467856

@@ -7854,10 +7864,7 @@ impl<'db> TypeVarInstance<'db> {
78547864
// inferable, so we set the parameter to `None` here.
78557865
let type_mapping = &TypeMapping::MarkTypeVarsInferable(None);
78567866

7857-
Self::new(
7858-
db,
7859-
self.name(db),
7860-
self.definition(db),
7867+
let new_bound_or_constraints =
78617868
self._bound_or_constraints(db)
78627869
.map(|bound_or_constraints| match bound_or_constraints {
78637870
TypeVarBoundOrConstraintsEvaluation::Eager(bound_or_constraints) => {
@@ -7867,20 +7874,44 @@ impl<'db> TypeVarInstance<'db> {
78677874
}
78687875
TypeVarBoundOrConstraintsEvaluation::LazyUpperBound
78697876
| TypeVarBoundOrConstraintsEvaluation::LazyConstraints => bound_or_constraints,
7870-
}),
7877+
});
7878+
7879+
let new_default = self._default(db).and_then(|default| match default {
7880+
TypeVarDefaultEvaluation::Eager(ty) => {
7881+
Some(ty.apply_type_mapping_impl(db, type_mapping, visitor).into())
7882+
}
7883+
TypeVarDefaultEvaluation::Lazy => self
7884+
.lazy_default(db)
7885+
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, visitor).into()),
7886+
});
7887+
7888+
// Ensure that we only modify the `original` field if we are going to modify one or both of
7889+
// `_bound_or_constraints` and `_default`; don't trigger creation of a new
7890+
// `TypeVarInstance` unnecessarily.
7891+
let new_original = if new_bound_or_constraints == self._bound_or_constraints(db)
7892+
&& new_default == self._default(db)
7893+
{
7894+
self.original(db)
7895+
} else {
7896+
Some(self)
7897+
};
7898+
7899+
Self::new(
7900+
db,
7901+
self.name(db),
7902+
self.definition(db),
7903+
new_bound_or_constraints,
78717904
self.explicit_variance(db),
7872-
self._default(db).and_then(|default| match default {
7873-
TypeVarDefaultEvaluation::Eager(ty) => {
7874-
Some(ty.apply_type_mapping_impl(db, type_mapping, visitor).into())
7875-
}
7876-
TypeVarDefaultEvaluation::Lazy => self
7877-
.lazy_default(db)
7878-
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, visitor).into()),
7879-
}),
7905+
new_default,
78807906
self.kind(db),
7907+
new_original,
78817908
)
78827909
}
78837910

7911+
fn is_identical_to(self, db: &'db dyn Db, other: Self) -> bool {
7912+
self == other || (self.original(db) == Some(other) || other.original(db) == Some(self))
7913+
}
7914+
78847915
fn to_instance(self, db: &'db dyn Db) -> Option<Self> {
78857916
let bound_or_constraints = match self.bound_or_constraints(db)? {
78867917
TypeVarBoundOrConstraints::UpperBound(upper_bound) => {
@@ -7898,6 +7929,7 @@ impl<'db> TypeVarInstance<'db> {
78987929
self.explicit_variance(db),
78997930
None,
79007931
self.kind(db),
7932+
self.original(db),
79017933
))
79027934
}
79037935

@@ -8062,6 +8094,7 @@ impl<'db> BoundTypeVarInstance<'db> {
80628094
Some(variance),
80638095
None, // _default
80648096
TypeVarKind::Pep695,
8097+
None,
80658098
),
80668099
BindingContext::Synthetic,
80678100
)
@@ -8083,11 +8116,24 @@ impl<'db> BoundTypeVarInstance<'db> {
80838116
Some(TypeVarVariance::Invariant),
80848117
None,
80858118
TypeVarKind::TypingSelf,
8119+
None,
80868120
),
80878121
binding_context,
80888122
)
80898123
}
80908124

8125+
pub(crate) fn is_identical_to(self, db: &'db dyn Db, other: Self) -> bool {
8126+
if self == other {
8127+
return true;
8128+
}
8129+
8130+
if self.binding_context(db) != other.binding_context(db) {
8131+
return false;
8132+
}
8133+
8134+
self.typevar(db).is_identical_to(db, other.typevar(db))
8135+
}
8136+
80918137
pub(crate) fn variance_with_polarity(
80928138
self,
80938139
db: &'db dyn Db,

crates/ty_python_semantic/src/types/generics.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ pub(crate) fn typing_self<'db>(
126126
Some(TypeVarVariance::Invariant),
127127
None,
128128
TypeVarKind::TypingSelf,
129+
None,
129130
);
130131

131132
bind_typevar(
@@ -396,7 +397,7 @@ impl<'db> GenericContext<'db> {
396397
typevar: TypeVarInstance<'db>,
397398
) -> Option<BoundTypeVarInstance<'db>> {
398399
self.variables(db)
399-
.find(|self_bound_typevar| self_bound_typevar.typevar(db) == typevar)
400+
.find(|self_bound_typevar| self_bound_typevar.typevar(db).is_identical_to(db, typevar))
400401
}
401402

402403
/// Creates a specialization of this generic context. Panics if the length of `types` does not

crates/ty_python_semantic/src/types/infer/builder.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2964,6 +2964,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
29642964
None,
29652965
default.as_deref().map(|_| TypeVarDefaultEvaluation::Lazy),
29662966
TypeVarKind::Pep695,
2967+
None,
29672968
)));
29682969
self.add_declaration_with_binding(
29692970
node.into(),
@@ -4255,6 +4256,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
42554256
Some(variance),
42564257
default,
42574258
TypeVarKind::Legacy,
4259+
None,
42584260
)))
42594261
}
42604262

0 commit comments

Comments
 (0)