-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Fixed regression in signature instantiation #59121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed regression in signature instantiation #59121
Conversation
src/compiler/checker.ts
Outdated
|
|
||
| function createSignatureTypeMapper(signature: Signature, typeArguments: readonly Type[] | undefined): TypeMapper { | ||
| return createTypeMapper(signature.typeParameters!, typeArguments); | ||
| return createTypeMapper(map(signature.typeParameters!, tp => tp.mapper ? instantiateType(tp, tp.mapper) : tp), typeArguments); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that the signature might already be a product of instantiateSignature that was called without eraseTypeParameters and which thus cloned type parameters. Those cloned parameters are instantiated there in the arguments list using the available mapper but the same mapper wasn't later used when erasing type parameters from that result. This led to the mapper-instantiated cloned type parameter being left non-instantiated after erasing type parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% certain, but it may be better to do sameMap here just because that would avoid extra arrays when nothing actually needs to be mapped (but maybe it doesn't happen that often).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, I pushed out a commit to use sameMap here
|
@typescript-bot test it |
|
Hey @andrewbranch, the results of running the DT tests are ready. Everything looks the same! |
|
@andrewbranch Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@andrewbranch Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@andrewbranch Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
@typescript-bot cherry-pick to release-5.5 |
|
Hey, @andrewbranch! I've created #59204 for you. |
…e-5.5 (#59204) Co-authored-by: Mateusz Burzyński <[email protected]>
fixes #59108
cc @weswigham