Skip to content

Clean up inference of type parameters, contravariant types - #49915

Merged
Jake Bailey (jakebailey) merged 2 commits into
microsoft:mainfrom
jakebailey:fix-43962-followup
Jul 19, 2022
Merged

Clean up inference of type parameters, contravariant types#49915
Jake Bailey (jakebailey) merged 2 commits into
microsoft:mainfrom
jakebailey:fix-43962-followup

Conversation

@jakebailey

@jakebaileyJake Bailey (jakebailey) commented Jul 15, 2022•
edited
Loading

Copy link
Copy Markdown
Member

These are some oddities and cleanups that I noticed in writing #49887 (see also #43962 (comment)).

Namely, inferFromContravariantTypes only did a contravariant inference if strictFunctionTypes was set, but that function call was used in many places that strictFunctionTypes should not be applied, like in the the alias type argument path that was the target of #49887.

I can't think of any particular test case that would be affected by this (probably, people always have this flag on, though notably not our own repo, oops), but it feels wrong.

@typescript-botTypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jul 15, 2022
@jakebailey

Jake Bailey (jakebailey) commented Jul 15, 2022

Copy link
Copy Markdown
MemberAuthor

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test this inline
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) perf test this faster

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 15, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the extended test suite on this PR at 01789d3. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 15, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the abridged perf test suite on this PR at 01789d3. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 15, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based user code test suite on this PR at 01789d3. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 15, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the parallelized Definitely Typed test suite on this PR at 01789d3. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 15, 2022

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
Great news! no new errors were found between main..refs/pull/49915/merge

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 15, 2022

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..49915

Metricmain49915DeltaBestWorst
Angular - node (v14.15.1, x64)
Memory used335,936k (± 0.01%)335,885k (± 0.01%)-50k (- 0.01%)335,847k335,960k
Parse Time2.06s (± 0.54%)2.07s (± 0.61%)+0.01s (+ 0.39%)2.05s2.11s
Bind Time0.90s (± 0.53%)0.90s (± 0.41%)-0.00s (- 0.22%)0.89s0.90s
Check Time5.79s (± 0.57%)5.79s (± 0.43%)-0.01s (- 0.14%)5.74s5.85s
Emit Time6.38s (± 0.78%)6.39s (± 0.41%)+0.01s (+ 0.13%)6.32s6.44s
Total Time15.13s (± 0.44%)15.14s (± 0.31%)+0.01s (+ 0.07%)15.07s15.28s
Compiler-Unions - node (v14.15.1, x64)
Memory used192,880k (± 0.01%)192,878k (± 0.01%)-1k (- 0.00%)192,819k192,928k
Parse Time0.85s (± 0.99%)0.84s (± 0.43%)-0.01s (- 0.82%)0.84s0.85s
Bind Time0.57s (± 0.78%)0.57s (± 1.32%)+0.00s (+ 0.35%)0.56s0.59s
Check Time6.71s (± 0.70%)6.70s (± 0.54%)-0.01s (- 0.16%)6.62s6.79s
Emit Time2.50s (± 1.01%)2.48s (± 0.97%)-0.02s (- 0.76%)2.44s2.56s
Total Time10.63s (± 0.63%)10.60s (± 0.38%)-0.03s (- 0.27%)10.51s10.68s
Monaco - node (v14.15.1, x64)
Memory used325,735k (± 0.01%)325,720k (± 0.01%)-15k (- 0.00%)325,669k325,767k
Parse Time1.57s (± 0.82%)1.58s (± 0.62%)+0.00s (+ 0.25%)1.56s1.60s
Bind Time0.78s (± 0.79%)0.78s (± 0.61%)+0.00s (+ 0.51%)0.78s0.80s
Check Time5.69s (± 0.49%)5.69s (± 0.34%)-0.00s (- 0.02%)5.66s5.74s
Emit Time3.35s (± 0.61%)3.37s (± 0.80%)+0.02s (+ 0.69%)3.32s3.45s
Total Time11.39s (± 0.27%)11.42s (± 0.29%)+0.03s (+ 0.25%)11.37s11.54s
TFS - node (v14.15.1, x64)
Memory used288,808k (± 0.01%)288,817k (± 0.01%)+8k (+ 0.00%)288,776k288,845k
Parse Time1.34s (± 1.84%)1.33s (± 1.56%)-0.01s (- 0.82%)1.28s1.37s
Bind Time0.75s (± 3.45%)0.76s (± 3.57%)+0.01s (+ 1.07%)0.73s0.86s
Check Time5.32s (± 0.36%)5.33s (± 0.27%)+0.02s (+ 0.34%)5.31s5.38s
Emit Time3.57s (± 2.23%)3.61s (± 2.28%)+0.04s (+ 1.23%)3.41s3.73s
Total Time10.97s (± 0.63%)11.04s (± 0.81%)+0.06s (+ 0.56%)10.79s11.17s
material-ui - node (v14.15.1, x64)
Memory used446,567k (± 0.04%)446,661k (± 0.01%)+94k (+ 0.02%)446,596k446,774k
Parse Time1.87s (± 0.43%)1.87s (± 0.40%)-0.00s (- 0.11%)1.85s1.89s
Bind Time0.72s (± 1.67%)0.72s (± 1.01%)-0.01s (- 0.83%)0.70s0.73s
Check Time13.19s (± 0.87%)13.13s (± 0.59%)-0.06s (- 0.46%)12.98s13.31s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time15.78s (± 0.71%)15.72s (± 0.47%)-0.07s (- 0.43%)15.56s15.91s
xstate - node (v14.15.1, x64)
Memory used541,319k (± 0.01%)541,297k (± 0.00%)-22k (- 0.00%)541,226k541,344k
Parse Time2.60s (± 0.36%)2.59s (± 0.50%)-0.01s (- 0.35%)2.57s2.63s
Bind Time1.15s (± 1.04%)1.15s (± 1.31%)-0.01s (- 0.69%)1.10s1.18s
Check Time1.56s (± 0.49%)1.54s (± 0.61%)-0.01s (- 0.84%)1.53s1.57s
Emit Time0.07s (± 6.66%)0.07s (± 3.14%)🟩-0.00s (- 4.05%)0.07s0.08s
Total Time5.39s (± 0.34%)5.36s (± 0.39%)-0.03s (- 0.54%)5.32s5.42s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
BenchmarkNameIterations
Current4991510
Baselinemain10

Developer Information:

Download Benchmark

weswigham
Wesley Wigham (weswigham) approved these changes Jul 19, 2022
View reviewed changes

@weswighamWesley Wigham (weswigham) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the extended test suites are fine, these changes look good to me.

@jakebailey

Jake Bailey (jakebailey) commented Jul 19, 2022

Copy link
Copy Markdown
MemberAuthor

It was clean, yeah.

jakebailey
Jake Bailey (jakebailey) commented Jul 19, 2022
View reviewed changes
Comment threadsrc/compiler/checker.ts Outdated
// Instantiate a generic signature in the context of a non-generic signature (section 3.8.5 in TypeScript spec)
function instantiateSignatureInContextOf(signature: Signature, contextualSignature: Signature, inferenceContext?: InferenceContext, compareTypes?: TypeComparer): Signature {
const context = createInferenceContext(signature.typeParameters!, signature, InferenceFlags.None, compareTypes);
const context = createInferenceContext(signature.typeParameters!, signature, inferenceContext?.flags ?? InferenceFlags.None, compareTypes);

@jakebaileyJake Bailey (jakebailey)Jul 19, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I think this is cruft from my other PR. Will remove and retest.

@jakebailey

Jake Bailey (jakebailey) commented Jul 19, 2022

Copy link
Copy Markdown
MemberAuthor

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test this inline

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 19, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based user code test suite on this PR at 4c5d4c1. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 19, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the extended test suite on this PR at 4c5d4c1. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
Great news! no new errors were found between main..refs/pull/49915/merge

@jakebailey
Jake Bailey (jakebailey) merged commit aba867d into microsoft:mainJul 19, 2022
@jakebailey
Jake Bailey (jakebailey) deleted the fix-43962-followup branch July 19, 2022 22:30
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@weswighamWesley Wigham (weswigham)weswigham approved these changes
@sandersnNathan Shively-Sanders (sandersn)Awaiting requested review from sandersn

Assignees

@jakebaileyJake Bailey (jakebailey)

Labels

Author: TeamFor Uncommitted BugPR for untriaged, rejected, closed or missing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jakebailey@typescript-bot@weswigham