[ty] Simplify materialization of specialized generics#20121
[ty] Simplify materialization of specialized generics#20121carljm merged 3 commits intoastral-sh:mainfrom
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
JelleZijlstra
changed the title
JelleZijlstra
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
This seems like a win to me. The materialization kind is part of the specialization; it seems reasonable that apply_specialization methods would be responsible for handling it.
I think if we wanted to invest in better error-resilience here (if we find ourselves forgetting to apply the materialization in more cases), the approach would be to extract a Rust type that holds only the types part of a Specialization, and have TypeMappling::Specialization hold only that Rust type, so you'd have to extract that and the materialization from a Specialization in order to wrap it in a TypeMapping. This would make it impossible to ignore the materialization part accidentally.
| ) -> Self { | ||
| if let Some(specialization) = specialization { | ||
| self.apply_type_mapping_impl( | ||
| let new_self = self.apply_type_mapping_impl( |
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) .
3 participants
This is a variant of #20076 that moves some complexity out of
apply_type_mapping_implingenerics.rs. The tradeoff is that now every place that appliesTypeMapping::Specializationmust 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 #20076 (comment) .