Skip to content

Commit bc09e93

Browse files
committed
Alex's second patch, tweaked to sit atop the first
1 parent 76ffa92 commit bc09e93

File tree

2 files changed

+90
-112
lines changed

2 files changed

+90
-112
lines changed

crates/ty_python_semantic/src/types.rs

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2199,30 +2199,34 @@ impl<'db> Type<'db> {
21992199
// A protocol instance can never be a subtype of a nominal type, with the *sole* exception of `object`.
22002200
(Type::ProtocolInstance(_), _) => ConstraintSet::from(false),
22012201

2202-
(Type::TypedDict(self_typeddict), Type::TypedDict(other_typeddict)) => self_typeddict
2203-
.has_relation_to_impl(
2204-
db,
2205-
other_typeddict,
2206-
inferable,
2207-
relation,
2208-
relation_visitor,
2209-
disjointness_visitor,
2210-
),
2202+
(Type::TypedDict(self_typeddict), Type::TypedDict(other_typeddict)) => relation_visitor
2203+
.visit((self, target, relation), || {
2204+
self_typeddict.has_relation_to_impl(
2205+
db,
2206+
other_typeddict,
2207+
inferable,
2208+
relation,
2209+
relation_visitor,
2210+
disjointness_visitor,
2211+
)
2212+
}),
22112213

22122214
// TODO: When we support `closed` and/or `extra_items`, we could allow assignments to other
22132215
// compatible `Mapping`s. `extra_items` could also allow for some assignments to `dict`, as
22142216
// long as `total=False`. (But then again, does anyone want a non-total `TypedDict` where all
22152217
// key types are a supertype of the extra items type?)
2216-
(Type::TypedDict(_), _) => KnownClass::Mapping
2217-
.to_specialized_instance(db, [KnownClass::Str.to_instance(db), Type::object()])
2218-
.has_relation_to_impl(
2219-
db,
2220-
target,
2221-
inferable,
2222-
relation,
2223-
relation_visitor,
2224-
disjointness_visitor,
2225-
),
2218+
(Type::TypedDict(_), _) => relation_visitor.visit((self, target, relation), || {
2219+
KnownClass::Mapping
2220+
.to_specialized_instance(db, [KnownClass::Str.to_instance(db), Type::object()])
2221+
.has_relation_to_impl(
2222+
db,
2223+
target,
2224+
inferable,
2225+
relation,
2226+
relation_visitor,
2227+
disjointness_visitor,
2228+
)
2229+
}),
22262230

22272231
// A non-`TypedDict` cannot subtype a `TypedDict`
22282232
(_, Type::TypedDict(_)) => ConstraintSet::from(false),

crates/ty_python_semantic/src/types/typed_dict.rs

Lines changed: 67 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -108,60 +108,51 @@ impl<'db> TypedDictType<'db> {
108108
// A required field is not required in self.
109109
return ConstraintSet::from(false);
110110
}
111-
relation_visitor.visit(
112-
(
113-
self_item_field.declared_ty,
111+
if target_item_field.is_read_only() {
112+
// For `ReadOnly[]` fields in the target, the corresponding fields in
113+
// self need to have the same assignability/subtyping/etc relation
114+
// individually that we're looking for overall between the
115+
// `TypedDict`s.
116+
self_item_field.declared_ty.has_relation_to_impl(
117+
db,
114118
target_item_field.declared_ty,
119+
inferable,
115120
relation,
116-
),
117-
|| {
118-
if target_item_field.is_read_only() {
119-
// For `ReadOnly[]` fields in the target, the corresponding fields in
120-
// self need to have the same assignability/subtyping/etc relation
121-
// individually that we're looking for overall between the
122-
// `TypedDict`s.
123-
self_item_field.declared_ty.has_relation_to_impl(
121+
relation_visitor,
122+
disjointness_visitor,
123+
)
124+
} else {
125+
if self_item_field.is_read_only() {
126+
// A read-only field can't be assigned to a mutable target.
127+
return ConstraintSet::from(false);
128+
}
129+
// For mutable fields in the target, the relation needs to apply both
130+
// ways, or else mutating the target could violate the structural
131+
// invariants of self. For fully-static types, this is "equivalence".
132+
// For gradual types, it depends on the relation, but mutual
133+
// assignability is "consistency".
134+
self_item_field
135+
.declared_ty
136+
.has_relation_to_impl(
137+
db,
138+
target_item_field.declared_ty,
139+
inferable,
140+
relation,
141+
relation_visitor,
142+
disjointness_visitor,
143+
)
144+
.intersect(
145+
db,
146+
target_item_field.declared_ty.has_relation_to_impl(
124147
db,
125-
target_item_field.declared_ty,
148+
self_item_field.declared_ty,
126149
inferable,
127150
relation,
128151
relation_visitor,
129152
disjointness_visitor,
130-
)
131-
} else {
132-
if self_item_field.is_read_only() {
133-
// A read-only field can't be assigned to a mutable target.
134-
return ConstraintSet::from(false);
135-
}
136-
// For mutable fields in the target, the relation needs to apply both
137-
// ways, or else mutating the target could violate the structural
138-
// invariants of self. For fully-static types, this is "equivalence".
139-
// For gradual types, it depends on the relation, but mutual
140-
// assignability is "consistency".
141-
self_item_field
142-
.declared_ty
143-
.has_relation_to_impl(
144-
db,
145-
target_item_field.declared_ty,
146-
inferable,
147-
relation,
148-
relation_visitor,
149-
disjointness_visitor,
150-
)
151-
.intersect(
152-
db,
153-
target_item_field.declared_ty.has_relation_to_impl(
154-
db,
155-
self_item_field.declared_ty,
156-
inferable,
157-
relation,
158-
relation_visitor,
159-
disjointness_visitor,
160-
),
161-
)
162-
}
163-
},
164-
)
153+
),
154+
)
155+
}
165156
} else {
166157
// `NotRequired[]` target fields
167158
if target_item_field.is_read_only() {
@@ -172,22 +163,13 @@ impl<'db> TypedDictType<'db> {
172163
// spec are structured like they are), and following the structure of the spec
173164
// makes it easier to check the logic here.
174165
if let Some(self_item_field) = self_items.get(target_item_name) {
175-
relation_visitor.visit(
176-
(
177-
self_item_field.declared_ty,
178-
target_item_field.declared_ty,
179-
relation,
180-
),
181-
|| {
182-
self_item_field.declared_ty.has_relation_to_impl(
183-
db,
184-
target_item_field.declared_ty,
185-
inferable,
186-
relation,
187-
relation_visitor,
188-
disjointness_visitor,
189-
)
190-
},
166+
self_item_field.declared_ty.has_relation_to_impl(
167+
db,
168+
target_item_field.declared_ty,
169+
inferable,
170+
relation,
171+
relation_visitor,
172+
disjointness_visitor,
191173
)
192174
} else {
193175
// Self is missing this not-required, read-only item. However, since all
@@ -216,38 +198,30 @@ impl<'db> TypedDictType<'db> {
216198
// in the target, because `del` is allowed on the target field.
217199
return ConstraintSet::from(false);
218200
}
219-
relation_visitor.visit(
220-
(
221-
self_item_field.declared_ty,
201+
202+
// As above, for mutable fields in the target, the relation needs
203+
// to apply both ways.
204+
self_item_field
205+
.declared_ty
206+
.has_relation_to_impl(
207+
db,
222208
target_item_field.declared_ty,
209+
inferable,
223210
relation,
224-
),
225-
|| {
226-
// As above, for mutable fields in the target, the relation needs
227-
// to apply both ways.
228-
self_item_field
229-
.declared_ty
230-
.has_relation_to_impl(
231-
db,
232-
target_item_field.declared_ty,
233-
inferable,
234-
relation,
235-
relation_visitor,
236-
disjointness_visitor,
237-
)
238-
.intersect(
239-
db,
240-
target_item_field.declared_ty.has_relation_to_impl(
241-
db,
242-
self_item_field.declared_ty,
243-
inferable,
244-
relation,
245-
relation_visitor,
246-
disjointness_visitor,
247-
),
248-
)
249-
},
250-
)
211+
relation_visitor,
212+
disjointness_visitor,
213+
)
214+
.intersect(
215+
db,
216+
target_item_field.declared_ty.has_relation_to_impl(
217+
db,
218+
self_item_field.declared_ty,
219+
inferable,
220+
relation,
221+
relation_visitor,
222+
disjointness_visitor,
223+
),
224+
)
251225
} else {
252226
// Self is missing this not-required, mutable field. This isn't ok if self
253227
// has read-only extra items, which all `TypedDict`s effectively do until

0 commit comments

Comments
 (0)