Type argument inference fix for infinitely recursive anonymous types #3452
Conversation
| inferFromIndexTypes(source, target, IndexKind.String, IndexKind.String); | ||
| inferFromIndexTypes(source, target, IndexKind.Number, IndexKind.Number); | ||
| inferFromIndexTypes(source, target, IndexKind.String, IndexKind.Number); | ||
| depth--; } | ||
| } |
There was a problem hiding this comment.
What is with this brace?
There was a problem hiding this comment.
That is a white space management error
… deeplyNestedTypeArgumentInference
|
@ahejlsberg Do you have any feedback on this? |
|
Overall, I like this unification. We used to stop after five levels of nesting during type inference, but now we'll go to ten levels. I doubt there are real world examples where this makes a difference in the quality of the inferences, but it certainly might impact performance (e.g. we'll spend time making the same inference ten times instead of five times). Thoughts? Perhaps we should pass the limit as a parameter so we can keep it at five for inference? |
|
Passing a limit parameter is an option. And I agree that the quality of the inference will not be impacted in realistic cases. That said, if we are concerned about performance, and we think changing the 10 to a 5 will help, I actually think we should do it across the board, for assignability too. Assignability is way more common than type argument inference, and in fact, I think every round of type argument inference is followed by a round of assignability checking. So we'd get way more mileage if we did that change across the board, and we'd get to keep the limit consistent. I also don't think we would sacrifice much correctness if we change to 5 for assignability. |
|
Yeah, changing to 5 across the board is probably better. Let's do that. |
… deeplyNestedTypeArgumentInference
…ence Type argument inference fix for infinitely recursive anonymous types
Type argument inference should use the same infrastructure as assignability when it comes to detecting infinite recursion in a type structure. This change makes type argument inference rely on the same isDeeplyNestedGeneric function.
Fixes #3451.