Refrain from attempting to perform parameter fixing on a generic signature multiple times #43835
Conversation
…ature multiple times
typescript-bot
added
Author: Team
For Uncommitted Bug
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
One comment and some background questions.
|
|
||
| function assignContextualParameterTypes(signature: Signature, context: Signature) { | ||
| signature.typeParameters = context.typeParameters; | ||
| if (context.typeParameters) { |
There was a problem hiding this comment.
Is context.typeParameters ever undefined? Was that the bug here?
There was a problem hiding this comment.
I mean, I don't want to overwrite a signature's type parameters with undefined, and there's no reason to do anything if they're both undefined.
| signature.typeParameters = context.typeParameters; | ||
| } | ||
| else { | ||
| return; // This signature has already has a contextual inference performed and cached on it! |
There was a problem hiding this comment.
Double-checking my understanding; is this correct?: it's enough to bail after seeing that typeParameters is set, because a normal run of this function sets both typeParameters and parameters. So you don't have to check parameters as well.
There was a problem hiding this comment.
Aye. Moreover, if a function somehow has type parameters set already, but not parameters, fixing parameters which contain a separate set of type parameters would be bad, too. (It'd be like the inverse of the original issue that caused the report)
3 participants
Fixes #43833
We would perform contextual parameter fixing twice on the same signature (against different instantiations of the target signature) - once during generic propagation, and again later when doing contextual typing. This caused the
typeParametersof the signature being fixed to get mutated, while the cached calculated type of each parameter was unchanged, causing a type parameter mismatch leading to inference failure. Since signatures are supposed to be immutable-ish, I disallowed the second fixing pass if the first fixing pass has occurred. Additionally, there is now a debug assertion in parameter fixing for if we attempt to fix a type to a parameter and it already has a cached type which differs from the type we want to fix. Logically, we never want this to happen (since we can only contextually fix a given signature once), so the assertion should only be triggered if we've broken this subtle invariant somewhere.