-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
@@ -15958,7 +15958,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
} | |||
|
|||
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