Skip to content

Allow nested conditionals to be related via constraints #37208

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 19, 2020
Merged

Allow nested conditionals to be related via constraints #37208

merged 3 commits into from
Mar 19, 2020

Conversation

weswigham
Copy link
Member

Fixes #30706

This allows a conditional to be related to another conditional via constraint. This means, notably, that conditionals which resolve to ? X : never should now be assignable to X, where X is another conditional type.

@weswigham weswigham requested a review from ahejlsberg March 4, 2020 11:44
@weswigham
Copy link
Member Author

@typescript-bot user test this
@typescript-bot perf test this
@typescript-bot run dt
@typescript-bot test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 4, 2020 •
edited
Loading

Heya @weswigham, I've started to run the parallelized community code test suite on this PR at b554a2f. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 4, 2020 •
edited
Loading

Heya @weswigham, I've started to run the perf test suite on this PR at b554a2f. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 4, 2020 •
edited
Loading

Heya @weswigham, I've started to run the extended test suite on this PR at b554a2f. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 4, 2020 •
edited
Loading

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at b554a2f. You can monitor the build here.

@weswigham weswigham mentioned this pull request Mar 4, 2020
Closed
@typescript-bot typescript-bot mentioned this pull request Mar 4, 2020
Closed
@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@typescript-bot
Copy link
Collaborator

@weswigham
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..37208

Metric master 37208 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 334,359k (± 0.04%) 333,797k (± 0.03%) -562k (- 0.17%) 333,486k 334,004k
Parse Time 1.63s (± 0.47%) 1.62s (± 0.45%) -0.00s (- 0.18%) 1.61s 1.64s
Bind Time 0.90s (± 0.83%) 0.89s (± 0.87%) -0.01s (- 0.67%) 0.87s 0.90s
Check Time 4.73s (± 0.47%) 4.74s (± 0.49%) +0.01s (+ 0.23%) 4.69s 4.79s
Emit Time 5.29s (± 0.83%) 5.30s (± 0.85%) +0.01s (+ 0.26%) 5.21s 5.43s
Total Time 12.54s (± 0.36%) 12.56s (± 0.50%) +0.02s (+ 0.14%) 12.46s 12.75s
Monaco - node (v10.16.3, x64)
Memory used 335,273k (± 0.02%) 335,231k (± 0.02%) -42k (- 0.01%) 335,045k 335,365k
Parse Time 1.25s (± 0.62%) 1.25s (± 0.69%) +0.00s (+ 0.24%) 1.23s 1.27s
Bind Time 0.78s (± 0.64%) 0.78s (± 0.75%) +0.00s (+ 0.26%) 0.76s 0.79s
Check Time 4.71s (± 0.59%) 4.74s (± 0.46%) +0.02s (+ 0.47%) 4.68s 4.79s
Emit Time 2.92s (± 0.75%) 2.92s (± 0.66%) +0.00s (+ 0.10%) 2.88s 2.98s
Total Time 9.65s (± 0.51%) 9.69s (± 0.31%) +0.03s (+ 0.36%) 9.61s 9.74s
TFS - node (v10.16.3, x64)
Memory used 299,480k (± 0.02%) 299,399k (± 0.02%) -81k (- 0.03%) 299,307k 299,513k
Parse Time 0.94s (± 0.82%) 0.94s (± 0.39%) +0.00s (+ 0.32%) 0.94s 0.95s
Bind Time 0.75s (± 0.94%) 0.75s (± 0.78%) -0.00s (- 0.27%) 0.74s 0.76s
Check Time 4.26s (± 0.43%) 4.25s (± 0.45%) -0.01s (- 0.26%) 4.20s 4.28s
Emit Time 3.04s (± 0.75%) 3.06s (± 0.69%) +0.01s (+ 0.46%) 3.01s 3.10s
Total Time 8.99s (± 0.47%) 9.00s (± 0.43%) +0.01s (+ 0.10%) 8.93s 9.08s
material-ui - node (v10.16.3, x64)
Memory used 488,952k (± 0.02%) 488,689k (± 0.02%) -263k (- 0.05%) 488,409k 488,782k
Parse Time 1.77s (± 0.50%) 1.77s (± 0.58%) +0.01s (+ 0.28%) 1.75s 1.79s
Bind Time 0.68s (± 1.07%) 0.68s (± 0.98%) +0.00s (+ 0.15%) 0.67s 0.70s
Check Time 13.52s (± 0.51%) 13.61s (± 1.16%) +0.09s (+ 0.66%) 13.26s 13.99s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.97s (± 0.49%) 16.06s (± 0.95%) +0.09s (+ 0.58%) 15.70s 16.41s
Angular - node (v12.1.0, x64)
Memory used 310,048k (± 0.07%) 309,399k (± 0.09%) -649k (- 0.21%) 308,580k 309,713k
Parse Time 1.57s (± 0.70%) 1.58s (± 0.46%) +0.01s (+ 0.32%) 1.57s 1.60s
Bind Time 0.87s (± 0.77%) 0.88s (± 1.07%) +0.01s (+ 0.80%) 0.87s 0.91s
Check Time 4.64s (± 1.04%) 4.64s (± 1.11%) -0.01s (- 0.11%) 4.55s 4.77s
Emit Time 5.46s (± 0.92%) 5.54s (± 1.83%) +0.08s (+ 1.47%) 5.32s 5.82s
Total Time 12.55s (± 0.75%) 12.64s (± 1.12%) +0.09s (+ 0.69%) 12.31s 13.01s
Monaco - node (v12.1.0, x64)
Memory used 315,207k (± 0.02%) 315,213k (± 0.02%) +6k (+ 0.00%) 315,065k 315,361k
Parse Time 1.20s (± 0.79%) 1.20s (± 0.49%) -0.00s (- 0.00%) 1.19s 1.22s
Bind Time 0.74s (± 1.03%) 0.74s (± 0.64%) +0.01s (+ 0.95%) 0.73s 0.75s
Check Time 4.55s (± 0.45%) 4.55s (± 0.50%) +0.01s (+ 0.13%) 4.50s 4.61s
Emit Time 2.95s (± 0.86%) 2.95s (± 0.93%) -0.00s (- 0.03%) 2.89s 3.00s
Total Time 9.44s (± 0.33%) 9.46s (± 0.48%) +0.01s (+ 0.15%) 9.38s 9.57s
TFS - node (v12.1.0, x64)
Memory used 281,763k (± 0.02%) 281,702k (± 0.01%) -61k (- 0.02%) 281,587k 281,813k
Parse Time 0.93s (± 0.82%) 0.93s (± 0.81%) +0.00s (+ 0.22%) 0.91s 0.94s
Bind Time 0.71s (± 0.84%) 0.72s (± 1.31%) +0.01s (+ 1.56%) 0.71s 0.75s
Check Time 4.16s (± 0.74%) 4.17s (± 0.39%) +0.01s (+ 0.34%) 4.13s 4.21s
Emit Time 3.07s (± 0.88%) 3.09s (± 1.03%) +0.02s (+ 0.78%) 3.02s 3.14s
Total Time 8.86s (± 0.52%) 8.91s (± 0.46%) +0.04s (+ 0.50%) 8.82s 8.99s
material-ui - node (v12.1.0, x64)
Memory used 466,424k (± 0.01%) 466,052k (± 0.05%) -372k (- 0.08%) 465,191k 466,310k
Parse Time 1.74s (± 0.51%) 1.75s (± 0.60%) +0.01s (+ 0.57%) 1.73s 1.78s
Bind Time 0.62s (± 0.95%) 0.63s (± 1.27%) +0.01s (+ 1.28%) 0.62s 0.65s
Check Time 12.15s (± 0.97%) 12.17s (± 1.09%) +0.02s (+ 0.16%) 11.90s 12.38s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.52s (± 0.84%) 14.55s (± 0.99%) +0.04s (+ 0.25%) 14.26s 14.78s
Angular - node (v8.9.0, x64)
Memory used 329,433k (± 0.02%) 328,919k (± 0.01%) -514k (- 0.16%) 328,860k 329,067k
Parse Time 2.11s (± 0.52%) 2.10s (± 0.36%) -0.02s (- 0.76%) 2.08s 2.12s
Bind Time 0.92s (± 0.73%) 0.92s (± 0.61%) +0.00s (+ 0.11%) 0.91s 0.93s
Check Time 5.51s (± 0.69%) 5.52s (± 0.73%) +0.01s (+ 0.20%) 5.42s 5.60s
Emit Time 6.25s (± 0.50%) 6.20s (± 1.23%) -0.05s (- 0.75%) 5.93s 6.30s
Total Time 14.78s (± 0.36%) 14.74s (± 0.69%) -0.05s (- 0.32%) 14.42s 14.92s
Monaco - node (v8.9.0, x64)
Memory used 333,547k (± 0.01%) 333,567k (± 0.01%) +20k (+ 0.01%) 333,430k 333,670k
Parse Time 1.54s (± 0.75%) 1.54s (± 0.22%) -0.00s (- 0.19%) 1.53s 1.54s
Bind Time 0.90s (± 1.25%) 0.90s (± 0.81%) -0.00s (- 0.11%) 0.89s 0.92s
Check Time 5.38s (± 0.49%) 5.41s (± 0.51%) +0.04s (+ 0.67%) 5.37s 5.49s
Emit Time 3.51s (± 0.59%) 3.51s (± 0.43%) +0.00s (+ 0.00%) 3.48s 3.54s
Total Time 11.33s (± 0.33%) 11.36s (± 0.31%) +0.03s (+ 0.24%) 11.29s 11.47s
TFS - node (v8.9.0, x64)
Memory used 298,864k (± 0.01%) 298,874k (± 0.01%) +10k (+ 0.00%) 298,780k 298,952k
Parse Time 1.25s (± 0.36%) 1.26s (± 0.60%) +0.01s (+ 0.64%) 1.24s 1.27s
Bind Time 0.75s (± 0.63%) 0.76s (± 0.87%) +0.01s (+ 1.33%) 0.75s 0.78s
Check Time 4.81s (± 0.26%) 4.89s (± 1.07%) +0.08s (+ 1.68%) 4.78s 5.05s
Emit Time 3.34s (± 2.40%) 3.35s (± 2.17%) +0.01s (+ 0.21%) 3.09s 3.45s
Total Time 10.15s (± 0.79%) 10.25s (± 0.46%) +0.10s (+ 1.02%) 10.13s 10.35s
material-ui - node (v8.9.0, x64)
Memory used 494,775k (± 0.01%) 494,504k (± 0.01%) -270k (- 0.05%) 494,343k 494,604k
Parse Time 2.11s (± 0.53%) 2.11s (± 0.57%) +0.00s (+ 0.05%) 2.08s 2.13s
Bind Time 0.81s (± 1.07%) 0.81s (± 1.36%) +0.00s (+ 0.12%) 0.79s 0.85s
Check Time 19.61s (± 0.60%) 19.59s (± 0.54%) -0.03s (- 0.15%) 19.37s 19.85s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 22.53s (± 0.56%) 22.51s (± 0.42%) -0.03s (- 0.13%) 22.32s 22.76s
Angular - node (v8.9.0, x86)
Memory used 188,926k (± 0.03%) 188,695k (± 0.03%) -231k (- 0.12%) 188,623k 188,836k
Parse Time 2.06s (± 0.55%) 2.05s (± 0.67%) -0.01s (- 0.53%) 2.02s 2.08s
Bind Time 1.07s (± 0.63%) 1.07s (± 0.54%) -0.00s (- 0.09%) 1.06s 1.09s
Check Time 5.02s (± 0.62%) 5.02s (± 0.70%) +0.00s (+ 0.08%) 4.95s 5.10s
Emit Time 6.15s (± 0.76%) 6.14s (± 0.53%) -0.01s (- 0.13%) 6.08s 6.23s
Total Time 14.30s (± 0.54%) 14.28s (± 0.38%) -0.02s (- 0.10%) 14.18s 14.44s
Monaco - node (v8.9.0, x86)
Memory used 189,231k (± 0.01%) 189,211k (± 0.03%) -20k (- 0.01%) 189,125k 189,360k
Parse Time 1.58s (± 0.75%) 1.59s (± 0.51%) +0.01s (+ 0.51%) 1.57s 1.61s
Bind Time 0.76s (± 0.64%) 0.77s (± 0.89%) +0.00s (+ 0.26%) 0.75s 0.78s
Check Time 5.31s (± 2.12%) 5.42s (± 2.01%) +0.10s (+ 1.94%) 5.10s 5.55s
Emit Time 3.00s (± 3.43%) 2.93s (± 3.18%) -0.07s (- 2.46%) 2.78s 3.18s
Total Time 10.66s (± 0.47%) 10.70s (± 0.48%) +0.04s (+ 0.36%) 10.63s 10.84s
TFS - node (v8.9.0, x86)
Memory used 170,466k (± 0.02%) 170,422k (± 0.02%) -44k (- 0.03%) 170,328k 170,486k
Parse Time 1.28s (± 0.55%) 1.27s (± 0.70%) -0.01s (- 0.47%) 1.26s 1.30s
Bind Time 0.72s (± 1.17%) 0.72s (± 1.80%) -0.00s (- 0.28%) 0.70s 0.75s
Check Time 4.60s (± 0.56%) 4.62s (± 0.65%) +0.02s (+ 0.48%) 4.55s 4.68s
Emit Time 2.96s (± 0.82%) 2.94s (± 1.19%) -0.02s (- 0.54%) 2.87s 3.04s
Total Time 9.56s (± 0.37%) 9.56s (± 0.57%) -0.00s (- 0.01%) 9.45s 9.70s
material-ui - node (v8.9.0, x86)
Memory used 277,213k (± 0.01%) 277,081k (± 0.01%) -131k (- 0.05%) 277,011k 277,136k
Parse Time 2.17s (± 0.43%) 2.17s (± 0.72%) +0.00s (+ 0.05%) 2.15s 2.22s
Bind Time 0.68s (± 0.70%) 0.68s (± 1.86%) +0.00s (+ 0.30%) 0.66s 0.72s
Check Time 17.62s (± 0.60%) 17.63s (± 0.40%) +0.01s (+ 0.03%) 17.43s 17.74s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 20.47s (± 0.54%) 20.48s (± 0.38%) +0.01s (+ 0.05%) 20.27s 20.62s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 37208 10
Baseline master 10

@weswigham
Copy link
Member Author

Every test suite is either green or identical to master - this change seems good.

@weswigham weswigham mentioned this pull request Mar 10, 2020
Closed
ahejlsberg
ahejlsberg approved these changes Mar 19, 2020
View reviewed changes
src/compiler/checker.ts Outdated
}
}
// while conditionals _can_ be related to one another via normal constraint, as, eg, `A extends B ? O : never` should be assignable to `O`
// when `O` is a conditional (`never` is trivially aissgnable to `O`, as is `O`!).
Copy link
Member

Choose a reason for hiding this comment

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

The comment doesn't read right. Delete the leading "while"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

@weswigham weswigham merged commit c513a4a into microsoft:master Mar 19, 2020
@andrewbranch andrewbranch mentioned this pull request Sep 1, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahejlsberg ahejlsberg ahejlsberg approved these changes

Assignees

@weswigham weswigham

Labels
Author: Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NonNullable<ConditionalType<T>> can't be assigned to ConditionalType<T> with --strictNullChecks
3 participants
@weswigham @typescript-bot @ahejlsberg