Make sure type parameters stay fixed throughout the inference process #2356
Conversation
| } | ||
| else { | ||
| // Infer the empty object type when no inferences were made | ||
| inferredType = emptyObjectType; | ||
| inferenceSucceeded = true; |
There was a problem hiding this comment.
This seems odd. No inferences were made, but inference succeeded? Can you provide some some explnatories comments on this? Thanks!
There was a problem hiding this comment.
Sure, but this is just a rule of the language. If there were no inferences, inference still succeeds. I don't love it. But @DanielRosenwasser has a PR where he issues an error for this case under a compiler flag.
There was a problem hiding this comment.
It's not yet a PR; I'd like to make it a PR if we could solve some of the original concerns in #360.
To clarify, when inference for a type argument fails, the process falls back to using {} as a type argument. 👍 for explaining what a failure scenario would be in contrast to no inferences being found.
There was a problem hiding this comment.
Ok, I'll add a comment.
… typeParameterFixing
| @@ -4379,6 +4379,8 @@ module ts { | |||
| } | |||
| } | |||
|
|
|||
| Debug.assert(!!downfallType, "If there is no common supertype, each type should have a downfallType"); | |||
There was a problem hiding this comment.
downfallType? Is this supposed to be like a fallbackType? If so, rename it. downfallType sounds like it describes revolutions, lost wars, etc.
There was a problem hiding this comment.
It is supposed to be downfallType. It is supposed to sound like it describes revolutions and lost wars, but in particular tragic heroes.
The downfallType is the type that caused a particular candidate to not be the common supertype. So if it weren't for this one downfallType, the type in question could have been the common supertype.
Note that downfallType is the first type that is not a subtype of type. It's not necessarily the only one.
There was a problem hiding this comment.
Can we add this explanation in the code
The downfallType is the type that caused a particular candidate to not be the common supertype. So if it weren't for this one downfallType, the type in question could have been the common supertype.
|
Is there a bug logged for the overload portion? |
|
@danquirk I have not seen one. I can try to file one if I can cook up a good repro. |
… typeParameterFixing
… typeParameterFixing
|
I've logged #2378 for the remaining work in this area. |
| } | ||
| return { | ||
| typeParameters: typeParameters, | ||
| inferUnionTypes: inferUnionTypes, | ||
| inferenceCount: 0, | ||
| inferences: inferences, | ||
| inferredTypes: new Array(typeParameters.length), |
There was a problem hiding this comment.
Should this have failedTypeParameterIndex as a property? It is optional though I find it will be easier to read through
There was a problem hiding this comment.
I think I'm going to leave it out. The reason it's optional is because it is always initially undefined, and undefined means that there have been no failures (yet).
… typeParameterFixing
Make sure type parameters stay fixed throughout the inference process
This fixes #2182 and fixes #2127.
The problem is essentially that after we fix a type parameter during type argument inference, we may unfix it later. This happens because the inference context that we use to store the results gets forgotten on every iteration of type argument inference. The main part of this change is to make the inference context persist throughout the processing of signature.
As an example, if you have:
Right now, T will be inferred to any, because we do 3 passes (one for each argument in this case). The results from each pass are as follows:
This also had a very unusual effect with constraints, which led to a crash. Namely, when it comes time to fix a type parameter, and the type we were going to infer is not assignable to the constraint, we infer the constraint instead, and we fix the type parameter. Then later we may infer back from that constraint, thereby adding an inference candidate that should never have been added. As a result, we have no common supertype among candidates, when really we should only have had one candidate.
The moral of the story is that when a type parameter gets fixed, it should stay fixed. This change is not totally complete, in that fixedness does not get persisted between overloads, but that is a phenomenon for another day.