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.

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

Fix isUnitLikeType to (again) handle tagged literal types #51545

Merged
merged 2 commits into from Nov 30, 2022
Merged

Conversation

ahejlsberg
Copy link
Member

Fixes #51538.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Nov 16, 2022
@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 16, 2022

Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 3b392d9. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 16, 2022

Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at 3b392d9. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 16, 2022

Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 3b392d9. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 16, 2022

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 16, 2022

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

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/51545/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..51545

Metric main 51545 Delta Best Worst
Angular - node (v16.17.1, x64)
Memory used 340,529k (± 0.03%) 340,572k (± 0.02%) +43k (+ 0.01%) 340,403k 340,664k
Parse Time 1.88s (± 0.90%) 1.89s (± 0.43%) +0.01s (+ 0.53%) 1.87s 1.91s
Bind Time 0.65s (± 0.56%) 0.65s (± 0.69%) +0.00s (+ 0.31%) 0.64s 0.66s
Check Time 5.17s (± 0.47%) 5.17s (± 0.40%) -0.00s (- 0.02%) 5.12s 5.21s
Emit Time 5.12s (± 0.75%) 5.10s (± 0.75%) -0.02s (- 0.29%) 5.02s 5.19s
Total Time 12.81s (± 0.48%) 12.81s (± 0.30%) -0.00s (- 0.02%) 12.74s 12.91s
Compiler-Unions - node (v16.17.1, x64)
Memory used 187,890k (± 0.63%) 187,575k (± 0.60%) -315k (- 0.17%) 186,470k 189,963k
Parse Time 0.79s (± 0.63%) 0.80s (± 1.06%) +0.01s (+ 1.78%) 0.78s 0.81s
Bind Time 0.42s (± 1.23%) 0.42s (± 1.06%) +0.00s (+ 0.24%) 0.41s 0.43s
Check Time 6.00s (± 0.96%) 6.04s (± 0.38%) +0.03s (+ 0.53%) 5.96s 6.08s
Emit Time 1.89s (± 0.92%) 1.91s (± 0.86%) +0.02s (+ 1.22%) 1.88s 1.94s
Total Time 9.10s (± 0.79%) 9.17s (± 0.34%) +0.07s (+ 0.73%) 9.10s 9.24s
Monaco - node (v16.17.1, x64)
Memory used 319,830k (± 0.01%) 319,824k (± 0.01%) -6k (- 0.00%) 319,740k 319,915k
Parse Time 1.43s (± 0.66%) 1.42s (± 0.37%) -0.01s (- 0.42%) 1.41s 1.43s
Bind Time 0.59s (± 0.57%) 0.59s (± 0.94%) -0.00s (- 0.17%) 0.58s 0.60s
Check Time 4.87s (± 0.54%) 4.88s (± 0.39%) +0.01s (+ 0.14%) 4.82s 4.91s
Emit Time 2.73s (± 0.61%) 2.72s (± 0.88%) -0.00s (- 0.15%) 2.65s 2.78s
Total Time 9.62s (± 0.47%) 9.62s (± 0.43%) -0.00s (- 0.03%) 9.51s 9.71s
TFS - node (v16.17.1, x64)
Memory used 282,277k (± 0.02%) 282,297k (± 0.01%) +20k (+ 0.01%) 282,247k 282,318k
Parse Time 1.17s (± 0.76%) 1.16s (± 0.80%) -0.00s (- 0.34%) 1.15s 1.19s
Bind Time 0.66s (± 3.85%) 0.63s (± 4.36%) 🟩-0.03s (- 4.82%) 0.59s 0.69s
Check Time 4.74s (± 0.44%) 4.76s (± 0.30%) +0.02s (+ 0.32%) 4.73s 4.79s
Emit Time 2.75s (± 2.15%) 2.75s (± 1.89%) -0.00s (- 0.18%) 2.68s 2.87s
Total Time 9.32s (± 0.70%) 9.30s (± 0.69%) -0.03s (- 0.29%) 9.17s 9.44s
material-ui - node (v16.17.1, x64)
Memory used 435,269k (± 0.00%) 435,284k (± 0.01%) +16k (+ 0.00%) 435,214k 435,343k
Parse Time 1.64s (± 0.68%) 1.65s (± 0.45%) +0.01s (+ 0.73%) 1.63s 1.66s
Bind Time 0.50s (± 1.33%) 0.50s (± 0.66%) -0.00s (- 0.60%) 0.49s 0.51s
Check Time 11.84s (± 0.70%) 11.86s (± 0.93%) +0.02s (+ 0.18%) 11.72s 12.17s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 13.98s (± 0.65%) 14.01s (± 0.83%) +0.03s (+ 0.24%) 13.86s 14.33s
xstate - node (v16.17.1, x64)
Memory used 515,972k (± 0.01%) 515,978k (± 0.01%) +7k (+ 0.00%) 515,919k 516,067k
Parse Time 2.31s (± 0.39%) 2.32s (± 0.48%) +0.01s (+ 0.35%) 2.29s 2.34s
Bind Time 0.84s (± 1.94%) 0.83s (± 1.06%) -0.01s (- 0.83%) 0.82s 0.86s
Check Time 1.36s (± 0.83%) 1.35s (± 0.78%) -0.01s (- 0.59%) 1.33s 1.38s
Emit Time 0.06s (± 0.00%) 0.06s (± 0.00%) 0.00s ( 0.00%) 0.06s 0.06s
Total Time 4.57s (± 0.48%) 4.57s (± 0.37%) -0.00s (- 0.07%) 4.54s 4.62s
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-131-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 51545 10
Baseline main 10

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/51545/merge:

Everything looks good!

@ahejlsberg
Copy link
Member Author

Tests and performance are unaffected. This should be good to merge.

@DanielRosenwasser
Copy link
Member

I'll try, but this probably can't be automatically cherry-picked.

@typescript-bot cherry-pick this to release-4.9

It will probably fail - if it does, would you be able to send a PR with the test and change for release-4.9?

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 17, 2022

Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into release-4.9 on this PR at 3b392d9. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

Hey @DanielRosenwasser, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-4.9 manually.

throw new Error('never');
}

function fx20(value: Either) {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like I'm having deja vu and I've asked this question, but why is this test in a file called unknownControlFlow.ts?

Copy link
Member Author

Choose a reason for hiding this comment

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

You did indeed: #51076 (comment). The test is in that file for the same reason.

@ahejlsberg ahejlsberg merged commit 8036b14 into main Nov 30, 2022
@ahejlsberg ahejlsberg deleted the fix51538 branch November 30, 2022 21:30
@DanielRosenwasser
Copy link
Member

Pick is in 4.9 with b4d382b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
4 participants