Skip to content

More complete check in isConstTypeVariable#53341

Merged
ahejlsberg merged 2 commits intomainfrom
fix53307
Apr 1, 2023
Merged

More complete check in isConstTypeVariable#53341
ahejlsberg merged 2 commits intomainfrom
fix53307

Conversation

@ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Mar 18, 2023 •
edited
Loading

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

ahejlsberg commented Mar 18, 2023

@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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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

typescript-bot commented Mar 18, 2023

@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

typescript-bot commented Mar 18, 2023

@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

typescript-bot commented Mar 18, 2023

@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

typescript-bot commented Mar 18, 2023

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

ahejlsberg commented Mar 21, 2023

Tests and performance all look good.

@sandersn
Copy link
Member

sandersn commented Mar 27, 2023

@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
@arcanis arcanis mentioned this pull request Jun 12, 2023
Closed
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@weswigham weswigham weswigham approved these changes

@DanielRosenwasser DanielRosenwasser Awaiting requested review from DanielRosenwasser

@RyanCavanaugh RyanCavanaugh Awaiting requested review from RyanCavanaugh

Assignees

@ahejlsberg ahejlsberg

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Archived in project

Milestone

No milestone

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

@ahejlsberg @typescript-bot @sandersn @weswigham