Skip to content

Improve unknown narrowing by negated type predicates #60795

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

Andarist
Copy link
Contributor

fixes #60789

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 17, 2024
@gabritto
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 17, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

@gabritto Here are the results of running the user tests with tsc comparing main and refs/pull/60795/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

Hey @gabritto, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

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

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 34 34 ~ ~ ~ p=1.000 n=6
Symbols 62,363 62,363 ~ ~ ~ p=1.000 n=6
Types 50,395 50,395 ~ ~ ~ p=1.000 n=6
Memory used 193,677k (± 0.76%) 193,082k (± 0.07%) ~ 193,008k 193,350k p=1.000 n=6
Parse Time 1.31s (± 0.57%) 1.30s (± 0.97%) ~ 1.29s 1.32s p=0.115 n=6
Bind Time 0.72s 0.72s ~ ~ ~ p=1.000 n=6
Check Time 9.76s (± 0.45%) 9.78s (± 0.26%) ~ 9.76s 9.83s p=0.373 n=6
Emit Time 2.72s (± 1.49%) 2.73s (± 0.51%) ~ 2.70s 2.74s p=0.565 n=6
Total Time 14.51s (± 0.43%) 14.53s (± 0.16%) ~ 14.50s 14.56s p=0.936 n=6
angular-1 - node (v18.15.0, x64)
Errors 37 37 ~ ~ ~ p=1.000 n=6
Symbols 947,936 947,936 ~ ~ ~ p=1.000 n=6
Types 410,955 410,955 ~ ~ ~ p=1.000 n=6
Memory used 1,225,832k (± 0.00%) 1,225,817k (± 0.00%) ~ 1,225,759k 1,225,866k p=0.575 n=6
Parse Time 6.66s (± 0.69%) 6.64s (± 0.51%) ~ 6.60s 6.68s p=0.468 n=6
Bind Time 1.88s (± 0.29%) 1.89s (± 0.22%) ~ 1.88s 1.89s p=0.282 n=6
Check Time 31.90s (± 0.27%) 31.97s (± 0.23%) ~ 31.85s 32.04s p=0.173 n=6
Emit Time 15.18s (± 0.62%) 15.19s (± 0.35%) ~ 15.12s 15.24s p=0.747 n=6
Total Time 55.63s (± 0.17%) 55.69s (± 0.20%) ~ 55.53s 55.84s p=0.336 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,448,617 2,448,617 ~ ~ ~ p=1.000 n=6
Types 896,282 896,282 ~ ~ ~ p=1.000 n=6
Memory used 2,320,401k (± 0.00%) 2,320,426k (± 0.00%) ~ 2,320,369k 2,320,464k p=0.173 n=6
Parse Time 9.40s (± 0.16%) 9.41s (± 0.25%) ~ 9.37s 9.43s p=0.566 n=6
Bind Time 2.24s (± 0.36%) 2.25s (± 0.24%) ~ 2.24s 2.25s p=0.441 n=6
Check Time 73.44s (± 0.63%) 73.43s (± 0.42%) ~ 73.04s 73.81s p=0.810 n=6
Emit Time 0.28s (± 2.26%) 0.29s (± 2.85%) ~ 0.28s 0.30s p=0.177 n=6
Total Time 85.37s (± 0.55%) 85.37s (± 0.35%) ~ 84.99s 85.74s p=0.810 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,390 1,225,392 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,584 266,584 ~ ~ ~ p=1.000 n=6
Memory used 2,475,670k (± 7.59%) 2,354,436k (± 0.03%) ~ 2,353,546k 2,355,328k p=0.575 n=6
Parse Time 5.25s (± 1.45%) 5.23s (± 0.42%) ~ 5.20s 5.26s p=0.518 n=6
Bind Time 1.78s (± 0.98%) 1.78s (± 1.12%) ~ 1.76s 1.81s p=1.000 n=6
Check Time 35.10s (± 0.63%) 35.26s (± 0.38%) ~ 35.08s 35.44s p=0.173 n=6
Emit Time 2.95s (± 2.48%) 2.97s (± 1.51%) ~ 2.90s 3.04s p=0.421 n=6
Total Time 45.08s (± 0.47%) 45.25s (± 0.37%) ~ 45.06s 45.49s p=0.128 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,390 1,225,392 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,584 266,584 ~ ~ ~ p=1.000 n=6
Memory used 3,029,128k (± 9.76%) 2,908,581k (±12.86%) ~ 2,424,626k 3,150,792k p=1.000 n=6
Parse Time 7.00s (± 1.38%) 6.95s (± 1.77%) ~ 6.77s 7.11s p=0.575 n=6
Bind Time 2.14s (± 1.71%) 2.15s (± 1.33%) ~ 2.10s 2.19s p=0.806 n=6
Check Time 42.90s (± 0.49%) 42.83s (± 0.80%) ~ 42.36s 43.28s p=0.748 n=6
Emit Time 3.49s (± 1.80%) 3.45s (± 2.27%) ~ 3.37s 3.56s p=0.298 n=6
Total Time 55.53s (± 0.51%) 55.37s (± 0.84%) ~ 54.66s 55.88s p=0.575 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 262,278 262,280 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 106,628 106,628 ~ ~ ~ p=1.000 n=6
Memory used 439,877k (± 0.01%) 439,917k (± 0.01%) ~ 439,864k 440,027k p=0.298 n=6
Parse Time 3.53s (± 1.21%) 3.53s (± 0.97%) ~ 3.47s 3.57s p=0.517 n=6
Bind Time 1.32s (± 0.39%) 1.32s (± 0.96%) ~ 1.31s 1.34s p=0.931 n=6
Check Time 18.96s (± 0.44%) 18.94s (± 0.37%) ~ 18.82s 19.03s p=0.809 n=6
Emit Time 1.53s (± 0.99%) 1.53s (± 0.76%) ~ 1.52s 1.55s p=0.868 n=6
Total Time 25.33s (± 0.32%) 25.32s (± 0.39%) ~ 25.20s 25.45s p=0.936 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 70 70 ~ ~ ~ p=1.000 n=6
Symbols 226,062 226,062 ~ ~ ~ p=1.000 n=6
Types 94,488 94,488 ~ ~ ~ p=1.000 n=6
Memory used 371,644k (± 0.03%) 371,606k (± 0.01%) ~ 371,562k 371,652k p=0.936 n=6
Parse Time 2.89s (± 0.58%) 2.89s (± 0.60%) ~ 2.87s 2.92s p=0.867 n=6
Bind Time 1.58s (± 0.48%) 1.58s (± 1.86%) ~ 1.54s 1.63s p=0.620 n=6
Check Time 16.49s (± 0.22%) 16.51s (± 0.63%) ~ 16.38s 16.65s p=1.000 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.96s (± 0.20%) 20.99s (± 0.43%) ~ 20.84s 21.09s p=0.518 n=6
vscode - node (v18.15.0, x64)
Errors 3 3 ~ ~ ~ p=1.000 n=6
Symbols 3,220,108 3,220,108 ~ ~ ~ p=1.000 n=6
Types 1,107,848 1,107,853 +5 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 3,286,380k (± 0.01%) 3,286,563k (± 0.01%) ~ 3,286,129k 3,286,945k p=0.298 n=6
Parse Time 14.19s (± 0.36%) 14.19s (± 0.80%) ~ 14.06s 14.37s p=0.873 n=6
Bind Time 4.55s (± 1.41%) 4.54s (± 0.85%) ~ 4.48s 4.60s p=0.683 n=6
Check Time 87.63s (± 1.35%) 86.93s (± 0.28%) ~ 86.71s 87.28s p=0.128 n=6
Emit Time 28.40s (± 1.86%) 28.28s (± 2.65%) ~ 27.40s 29.29s p=0.689 n=6
Total Time 134.78s (± 0.97%) 133.94s (± 0.65%) ~ 132.81s 135.11s p=0.298 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 291,463 291,463 ~ ~ ~ p=1.000 n=6
Types 118,920 118,920 ~ ~ ~ p=1.000 n=6
Memory used 445,205k (± 0.03%) 445,107k (± 0.02%) ~ 445,009k 445,211k p=0.230 n=6
Parse Time 4.09s (± 1.02%) 4.09s (± 1.15%) ~ 4.04s 4.16s p=1.000 n=6
Bind Time 1.78s (± 1.20%) 1.78s (± 0.68%) ~ 1.76s 1.79s p=0.622 n=6
Check Time 18.77s (± 0.38%) 18.72s (± 0.65%) ~ 18.50s 18.83s p=0.748 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.64s (± 0.35%) 24.59s (± 0.58%) ~ 24.33s 24.72s p=0.688 n=6
xstate-main - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 552,233 552,233 ~ ~ ~ p=1.000 n=6
Types 184,971 184,971 ~ ~ ~ p=1.000 n=6
Memory used 492,401k (± 0.00%) 492,387k (± 0.01%) ~ 492,343k 492,473k p=0.378 n=6
Parse Time 3.43s (± 0.93%) 3.42s (± 0.73%) ~ 3.40s 3.46s p=0.570 n=6
Bind Time 1.18s (± 0.99%) 1.17s (± 0.88%) ~ 1.15s 1.18s p=0.117 n=6
Check Time 19.45s (± 0.36%) 19.42s (± 0.42%) ~ 19.32s 19.52s p=0.574 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.06s (± 0.21%) 24.01s (± 0.30%) ~ 23.93s 24.13s p=0.261 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@gabritto Here are the results of running the top 400 repos with tsc comparing main and refs/pull/60795/merge:

Everything looks good!

@Andarist Andarist requested a review from gabritto December 18, 2024 06:26
@gabritto
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 18, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 18, 2024

Hey @gabritto, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/164425/artifacts?artifactName=tgz&fileId=854838C8C1BC0A3E52BA95C06664905DE93FF91DAF52B7D70630FEA7698FFC8002&fileName=/typescript-5.8.0-insiders.20241218.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.8.0-pr-60795-8".;

@gabritto
Copy link
Member

gabritto commented Dec 18, 2024

I'm now slightly curious about what it would take to fix this example: https://www.typescriptlang.org/play/?ts=5.8.0-dev.20241218#code/GYVwdgxgLglg9mABAIwIYBMA8AVRBTADyjzHQGdFwBrMOAdzAD4AKAgLkWwEoOA3OGOkQBvAFCJEMYIlaIAhAF5KpPMBhg86LiPETEEBGSiIAnh2EBfREoIBuXRd14ANmTw69+w8bOIwIZ2dEAB9ldFV1TWtEO0QAejjEAFEAJxS4FI4AAwIsyQpaYzBUNPoogAs8FLwHUUdRUEhYBEQAczg4LFxCYlIKSxC-AKDQ8HC1DXQWdk4eRH5BD0lpWUUwiMntMU8DMCNTcysbewlHCRc3JYld-d9-QMGxjajj+MSAdQyqMmzc-L8Suk6BUqjVTnUgA
I used it as an example of why you should use union types explicitly in #56941, but at some point I might have to update that example to not use unknown, as that is a special type we sometimes know to break into a union.

@gabritto gabritto merged commit 0dda037 into microsoft:main Dec 18, 2024
32 checks passed
@Andarist Andarist deleted the improve-negated-unknown-narrowing-by-type-predicate branch December 19, 2024 11:23
@sandersn sandersn removed this from PR Backlog Apr 22, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Negated {} and null | undefined predicates on unknown should still narrow
3 participants