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

More complete check in isConstTypeVariable #53341

Merged
merged 2 commits into from
Apr 1, 2023
Merged

More complete check in isConstTypeVariable #53341

merged 2 commits into from
Apr 1, 2023

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Mar 18, 2023

This PR improves the logic introduced in #51865 for determining whether a contextual type implies a const context. With this PR, a contextual type implies a const context when the type is

  • a type parameter that includes a const modifier,
  • a union type with at least one constituent type that implies a const context,
  • an indexed access type T[K] where T implies a const context,
  • a conditional type A extends B ? X : Y where X or Y imply a const context, or
  • a tuple type containing at least one variadic element ...T where T implies a const context.

Fixes #53307.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Mar 18, 2023
@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 Mar 18, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 18, 2023

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 18, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 18, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 18, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/53341/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..53341

Metric main 53341 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 360,896k (± 0.01%) 360,891k (± 0.01%) ~ 360,856k 360,922k p=0.936 n=6
Parse Time 3.52s (± 0.69%) 3.53s (± 0.90%) ~ 3.48s 3.57s p=0.871 n=6
Bind Time 1.18s (± 0.69%) 1.18s (± 0.44%) ~ 1.18s 1.19s p=0.929 n=6
Check Time 9.46s (± 0.52%) 9.47s (± 0.53%) ~ 9.42s 9.56s p=0.746 n=6
Emit Time 7.93s (± 0.68%) 7.89s (± 0.41%) ~ 7.84s 7.92s p=0.195 n=6
Total Time 22.10s (± 0.38%) 22.07s (± 0.44%) ~ 21.97s 22.23s p=0.630 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 192,531k (± 0.04%) 192,548k (± 0.03%) ~ 192,460k 192,633k p=0.810 n=6
Parse Time 1.57s (± 1.64%) 1.58s (± 0.93%) ~ 1.56s 1.60s p=0.870 n=6
Bind Time 0.82s (± 0.77%) 0.82s (± 0.50%) ~ 0.81s 0.82s p=0.673 n=6
Check Time 10.08s (± 0.62%) 10.05s (± 0.68%) ~ 10.01s 10.19s p=0.574 n=6
Emit Time 2.99s (± 0.63%) 2.98s (± 0.68%) ~ 2.95s 3.01s p=0.411 n=6
Total Time 15.47s (± 0.30%) 15.43s (± 0.48%) ~ 15.38s 15.58s p=0.127 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,434k (± 0.01%) 345,439k (± 0.01%) ~ 345,412k 345,485k p=1.000 n=6
Parse Time 2.72s (± 0.36%) 2.72s (± 0.36%) ~ 2.71s 2.74s p=1.000 n=6
Bind Time 1.09s (± 0.50%) 1.08s (± 0.97%) ~ 1.07s 1.10s p=1.000 n=6
Check Time 7.66s (± 0.32%) 7.66s (± 0.48%) ~ 7.62s 7.72s p=0.872 n=6
Emit Time 4.42s (± 0.57%) 4.43s (± 0.67%) ~ 4.40s 4.48s p=0.627 n=6
Total Time 15.89s (± 0.19%) 15.90s (± 0.44%) ~ 15.82s 16.03s p=0.624 n=6
TFS - node (v16.17.1, x64)
Memory used 299,792k (± 0.01%) 299,789k (± 0.01%) ~ 299,738k 299,852k p=0.748 n=6
Parse Time 2.17s (± 0.48%) 2.17s (± 0.48%) ~ 2.16s 2.19s p=0.160 n=6
Bind Time 1.24s (± 0.88%) 1.24s (± 0.97%) ~ 1.23s 1.26s p=0.739 n=6
Check Time 7.17s (± 0.57%) 7.18s (± 0.30%) ~ 7.15s 7.21s p=0.872 n=6
Emit Time 4.31s (± 0.70%) 4.36s (± 0.45%) +0.05s (+ 1.16%) 4.32s 4.37s p=0.019 n=6
Total Time 14.88s (± 0.33%) 14.95s (± 0.23%) +0.07s (+ 0.50%) 14.89s 14.99s p=0.029 n=6
material-ui - node (v16.17.1, x64)
Memory used 477,460k (± 0.02%) 477,484k (± 0.01%) ~ 477,419k 477,560k p=0.688 n=6
Parse Time 3.21s (± 0.36%) 3.23s (± 0.41%) ~ 3.21s 3.24s p=0.077 n=6
Bind Time 0.95s (± 0.54%) 0.96s (± 0.54%) ~ 0.95s 0.96s p=0.311 n=6
Check Time 17.96s (± 0.49%) 17.97s (± 0.51%) ~ 17.87s 18.11s p=0.873 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.12s (± 0.45%) 22.16s (± 0.43%) ~ 22.04s 22.30s p=0.521 n=6
xstate - node (v16.17.1, x64)
Memory used 550,802k (± 0.02%) 550,748k (± 0.02%) ~ 550,639k 550,966k p=0.378 n=6
Parse Time 3.94s (± 0.35%) 3.94s (± 0.58%) ~ 3.90s 3.97s p=0.517 n=6
Bind Time 1.80s (± 0.30%) 1.79s (± 0.77%) ~ 1.76s 1.80s p=0.138 n=6
Check Time 3.03s (± 0.39%) 3.03s (± 0.88%) ~ 3.00s 3.07s p=1.000 n=6
Emit Time 0.09s (± 0.00%) 0.09s (± 0.00%) ~ 0.09s 0.09s p=1.000 n=6
Total Time 8.86s (± 0.27%) 8.85s (± 0.25%) ~ 8.82s 8.88s p=0.746 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-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 53341 6
Baseline main 6

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/53341/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

Hey @ahejlsberg, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@ahejlsberg
Copy link
Member Author

Tests and performance all look good.

@sandersn
Copy link
Member

@ahejlsberg Is this ready to merge now?

@ahejlsberg ahejlsberg merged commit 3f675b6 into main Apr 1, 2023
@ahejlsberg ahejlsberg deleted the fix53307 branch April 1, 2023 21:45
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
Archived in project
Development

Successfully merging this pull request may close these issues.

5.0: const-like inference is not preserved unless a mapped type is used
4 participants