Skip to content
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

Allow intersections (and substitutions) to be checked against discriminable unions #36663

Merged
merged 1 commit into from Feb 10, 2020
Merged

Allow intersections (and substitutions) to be checked against discriminable unions #36663

merged 1 commit into from Feb 10, 2020

Conversation

weswigham
Copy link
Member

Intersections for the reasons reported, substitutions because they're just opaque intersections and should generally work the same way in relationships.

Fixes #33243

@weswigham
Copy link
Member Author

@typescript-bot perf test this I don't even know why, it's not like there are unions in the perf suite
@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 6, 2020 •
edited

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 6aaf984. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 6, 2020 •
edited

Heya @weswigham, I've started to run the perf test suite on this PR at 6aaf984. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 6, 2020 •
edited

Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 6aaf984. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 6, 2020 •
edited

Heya @weswigham, I've started to run the extended test suite on this PR at 6aaf984. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..36663

Metric master 36663 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 356,858k (± 0.01%) 358,586k (± 0.02%) +1,728k (+ 0.48%) 358,417k 358,788k
Parse Time 1.61s (± 0.50%) 1.62s (± 0.29%) +0.01s (+ 0.50%) 1.61s 1.63s
Bind Time 0.89s (± 0.95%) 0.88s (± 0.65%) -0.01s (- 1.01%) 0.87s 0.89s
Check Time 4.67s (± 0.44%) 4.67s (± 0.39%) +0.00s (+ 0.02%) 4.63s 4.70s
Emit Time 5.22s (± 0.25%) 5.23s (± 0.78%) +0.00s (+ 0.06%) 5.16s 5.33s
Total Time 12.40s (± 0.23%) 12.40s (± 0.43%) +0.00s (+ 0.01%) 12.27s 12.51s
Monaco - node (v10.16.3, x64)
Memory used 364,682k (± 0.02%) 364,656k (± 0.02%) -26k (- 0.01%) 364,556k 364,785k
Parse Time 1.25s (± 0.66%) 1.25s (± 0.42%) -0.00s (- 0.40%) 1.24s 1.26s
Bind Time 0.78s (± 0.48%) 0.77s (± 0.75%) -0.00s (- 0.26%) 0.76s 0.79s
Check Time 4.68s (± 0.70%) 4.65s (± 0.38%) -0.03s (- 0.64%) 4.60s 4.69s
Emit Time 2.90s (± 0.77%) 2.89s (± 0.46%) -0.02s (- 0.55%) 2.85s 2.91s
Total Time 9.61s (± 0.45%) 9.56s (± 0.20%) -0.05s (- 0.55%) 9.50s 9.60s
TFS - node (v10.16.3, x64)
Memory used 324,299k (± 0.07%) 324,149k (± 0.02%) -149k (- 0.05%) 323,947k 324,287k
Parse Time 0.94s (± 0.52%) 0.95s (± 0.68%) +0.00s (+ 0.42%) 0.93s 0.96s
Bind Time 0.75s (± 1.54%) 0.74s (± 1.45%) -0.01s (- 1.07%) 0.72s 0.76s
Check Time 4.23s (± 0.38%) 4.23s (± 0.53%) -0.00s (- 0.09%) 4.18s 4.27s
Emit Time 3.01s (± 0.76%) 3.01s (± 0.63%) -0.01s (- 0.20%) 2.95s 3.05s
Total Time 8.93s (± 0.43%) 8.92s (± 0.37%) -0.02s (- 0.18%) 8.85s 9.00s
Angular - node (v12.1.0, x64)
Memory used 332,632k (± 0.07%) 334,306k (± 0.01%) +1,674k (+ 0.50%) 334,178k 334,399k
Parse Time 1.57s (± 0.54%) 1.57s (± 0.25%) +0.00s (+ 0.13%) 1.56s 1.58s
Bind Time 0.86s (± 1.10%) 0.88s (± 0.68%) +0.01s (+ 1.16%) 0.86s 0.89s
Check Time 4.62s (± 0.80%) 4.57s (± 0.63%) -0.04s (- 0.89%) 4.52s 4.64s
Emit Time 5.41s (± 0.57%) 5.38s (± 0.72%) -0.03s (- 0.55%) 5.29s 5.45s
Total Time 12.46s (± 0.47%) 12.39s (± 0.40%) -0.06s (- 0.51%) 12.31s 12.50s
Monaco - node (v12.1.0, x64)
Memory used 344,551k (± 0.02%) 344,567k (± 0.02%) +16k (+ 0.00%) 344,435k 344,769k
Parse Time 1.22s (± 0.67%) 1.21s (± 0.67%) -0.01s (- 0.82%) 1.20s 1.23s
Bind Time 0.75s (± 0.80%) 0.75s (± 0.53%) +0.00s (+ 0.54%) 0.74s 0.76s
Check Time 4.54s (± 0.35%) 4.54s (± 0.47%) +0.01s (+ 0.18%) 4.51s 4.60s
Emit Time 2.94s (± 0.61%) 2.95s (± 0.61%) +0.01s (+ 0.48%) 2.91s 2.99s
Total Time 9.44s (± 0.31%) 9.46s (± 0.40%) +0.02s (+ 0.18%) 9.38s 9.53s
TFS - node (v12.1.0, x64)
Memory used 306,384k (± 0.02%) 306,440k (± 0.02%) +57k (+ 0.02%) 306,328k 306,576k
Parse Time 0.94s (± 0.66%) 0.94s (± 0.48%) -0.00s (- 0.11%) 0.93s 0.95s
Bind Time 0.70s (± 0.53%) 0.70s (± 0.57%) -0.00s (- 0.57%) 0.69s 0.71s
Check Time 4.17s (± 0.44%) 4.18s (± 0.74%) +0.01s (+ 0.34%) 4.14s 4.24s
Emit Time 3.07s (± 0.63%) 3.07s (± 0.58%) -0.00s (- 0.03%) 3.02s 3.12s
Total Time 8.88s (± 0.38%) 8.89s (± 0.42%) +0.01s (+ 0.12%) 8.81s 8.96s
Angular - node (v8.9.0, x64)
Memory used 351,823k (± 0.01%) 353,504k (± 0.01%) +1,680k (+ 0.48%) 353,427k 353,643k
Parse Time 2.10s (± 0.54%) 2.11s (± 0.39%) +0.01s (+ 0.43%) 2.09s 2.12s
Bind Time 0.92s (± 0.64%) 0.93s (± 0.56%) +0.00s (+ 0.54%) 0.92s 0.94s
Check Time 5.46s (± 0.66%) 5.42s (± 0.51%) -0.04s (- 0.66%) 5.37s 5.48s
Emit Time 6.24s (± 0.36%) 6.19s (± 0.73%) -0.04s (- 0.71%) 6.12s 6.29s
Total Time 14.71s (± 0.25%) 14.65s (± 0.47%) -0.06s (- 0.44%) 14.53s 14.80s
Monaco - node (v8.9.0, x64)
Memory used 362,939k (± 0.01%) 362,924k (± 0.01%) -16k (- 0.00%) 362,880k 363,008k
Parse Time 1.56s (± 0.43%) 1.56s (± 0.33%) +0.00s (+ 0.06%) 1.55s 1.57s
Bind Time 0.95s (± 0.49%) 0.95s (± 0.80%) -0.00s (- 0.42%) 0.93s 0.97s
Check Time 5.39s (± 1.25%) 5.38s (± 1.45%) -0.02s (- 0.28%) 5.24s 5.56s
Emit Time 3.30s (± 3.17%) 3.32s (± 3.40%) +0.02s (+ 0.48%) 3.01s 3.47s
Total Time 11.21s (± 0.55%) 11.20s (± 0.43%) -0.00s (- 0.04%) 11.09s 11.30s
TFS - node (v8.9.0, x64)
Memory used 323,541k (± 0.02%) 323,488k (± 0.02%) -53k (- 0.02%) 323,301k 323,564k
Parse Time 1.26s (± 0.44%) 1.26s (± 0.51%) 0.00s ( 0.00%) 1.25s 1.28s
Bind Time 0.75s (± 0.65%) 0.75s (± 0.30%) -0.00s (- 0.40%) 0.75s 0.76s
Check Time 4.82s (± 0.56%) 4.80s (± 0.75%) -0.02s (- 0.37%) 4.73s 4.89s
Emit Time 3.20s (± 0.73%) 3.19s (± 0.64%) -0.01s (- 0.19%) 3.14s 3.25s
Total Time 10.04s (± 0.41%) 10.01s (± 0.57%) -0.03s (- 0.31%) 9.88s 10.15s
Angular - node (v8.9.0, x86)
Memory used 200,058k (± 0.02%) 200,929k (± 0.02%) +871k (+ 0.44%) 200,829k 201,043k
Parse Time 2.04s (± 1.23%) 2.04s (± 0.46%) +0.00s (+ 0.05%) 2.02s 2.06s
Bind Time 1.05s (± 0.85%) 1.05s (± 0.59%) +0.00s (+ 0.48%) 1.04s 1.06s
Check Time 4.97s (± 0.70%) 4.95s (± 0.75%) -0.02s (- 0.30%) 4.86s 5.05s
Emit Time 6.06s (± 1.14%) 6.06s (± 1.51%) -0.00s (- 0.07%) 5.93s 6.30s
Total Time 14.11s (± 0.61%) 14.10s (± 0.68%) -0.01s (- 0.09%) 13.94s 14.33s
Monaco - node (v8.9.0, x86)
Memory used 203,746k (± 0.02%) 203,766k (± 0.02%) +21k (+ 0.01%) 203,687k 203,864k
Parse Time 1.60s (± 0.55%) 1.59s (± 0.76%) -0.01s (- 0.56%) 1.58s 1.63s
Bind Time 0.77s (± 1.35%) 0.77s (± 0.47%) -0.01s (- 0.78%) 0.76s 0.77s
Check Time 5.19s (± 1.98%) 5.16s (± 1.42%) -0.02s (- 0.42%) 5.07s 5.43s
Emit Time 3.13s (± 2.72%) 3.15s (± 2.78%) +0.02s (+ 0.64%) 2.84s 3.27s
Total Time 10.69s (± 0.42%) 10.67s (± 0.39%) -0.02s (- 0.16%) 10.60s 10.80s
TFS - node (v8.9.0, x86)
Memory used 182,635k (± 0.03%) 182,630k (± 0.02%) -5k (- 0.00%) 182,525k 182,674k
Parse Time 1.30s (± 0.77%) 1.31s (± 0.72%) +0.00s (+ 0.23%) 1.29s 1.33s
Bind Time 0.71s (± 0.87%) 0.71s (± 0.31%) -0.00s (- 0.00%) 0.71s 0.72s
Check Time 4.58s (± 0.77%) 4.57s (± 0.62%) -0.02s (- 0.35%) 4.51s 4.64s
Emit Time 2.96s (± 1.44%) 2.94s (± 0.71%) -0.02s (- 0.81%) 2.88s 2.98s
Total Time 9.57s (± 0.68%) 9.53s (± 0.32%) -0.04s (- 0.40%) 9.43s 9.57s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory4 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)
Benchmark Name Iterations
Current 36663 10
Baseline master 10

@typescript-bot typescript-bot mentioned this pull request Feb 7, 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.

@weswigham
Copy link
Member Author

weswigham commented Feb 7, 2020 •
edited

Perf impact is zero, user baselines look fine, dt error looks identical to master, rwc baselines are identical to master's currently outstanding diff (which I also just accepted). actions-based PR checks seems to have failed because of a networking issue; I'm rerunning them just in case, but everything looks good here~

@rbuckton you wanna give a quick look over this change?

sandersn
sandersn approved these changes Feb 7, 2020
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Seems good! I wonder how there were ZERO changes in any test suite, though.

@weswigham weswigham merged commit aece8c0 into microsoft:master Feb 10, 2020
@weswigham
Copy link
Member Author

@typescript-bot cherrypick this into release-3.8

@typescript-bot
Copy link
Collaborator

Hey @weswigham, I've opened #36726 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Feb 10, 2020
@weswigham
Cherry-pick PR microsoft#36663 into release-3.8
d61ca6c 
Component commits:
6aaf984 Allow intersections (and substitutions) to be checks against discriminable unions
@typescript-bot typescript-bot mentioned this pull request Feb 10, 2020
Merged
weswigham added a commit that referenced this pull request Feb 11, 2020
@typescript-bot @weswigham
Cherry-pick PR #36663 into release-3.8 (#36726)
ae8dc2f 
Component commits:
6aaf984 Allow intersections (and substitutions) to be checks against discriminable unions

Co-authored-by: Wesley Wigham <wwigham@gmail.com>
@rbuckton rbuckton mentioned this pull request Jul 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandersn sandersn sandersn approved these changes

@rbuckton rbuckton Awaiting requested review from rbuckton

@RyanCavanaugh RyanCavanaugh Awaiting requested review from RyanCavanaugh

Assignees

@weswigham weswigham

Labels
Author: Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Union of partial types: common super type of all union members is not assignable to union type
3 participants
@weswigham @typescript-bot @sandersn