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

Don't avoid caching variable types resolved using CheckMode.TypeOnly #59075

Conversation

Andarist
Copy link
Contributor

Fixes the weird behavior observed in #59074

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jun 29, 2024
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@@ -11994,7 +11994,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// to preserve this type. In fact, we need to _prefer_ that type, but it won't
// be assigned until contextual typing is complete, so we need to defer in
// cases where contextual typing may take place.
if (!links.type && !isParameterOfContextSensitiveSignature(symbol) && !checkMode) {
if (!links.type && !isParameterOfContextSensitiveSignature(symbol) && !(checkMode & ~CheckMode.TypeOnly)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this cache suppression was introduced in #56753 and its original PR #50586 was first created before CheckMode.TypeOnly was even a thing (that was introduced in #54380 )

It would be an easy thing to miss either way but partially the bug was born because that PR was long-lived and a new concept was introduced to the checker in the meantime.

@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 29, 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

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

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

@jakebailey
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 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,242 ~ ~ ~ p=1.000 n=6
Memory used 193,934k (± 1.01%) 194,534k (± 0.95%) ~ 192,091k 195,752k p=0.689 n=6
Parse Time 1.31s (± 1.04%) 1.31s (± 0.31%) ~ 1.30s 1.31s p=0.929 n=6
Bind Time 0.71s 0.71s ~ ~ ~ p=1.000 n=6
Check Time 9.44s (± 0.31%) 9.46s (± 0.64%) ~ 9.39s 9.54s p=0.520 n=6
Emit Time 2.75s (± 1.01%) 2.75s (± 0.97%) ~ 2.70s 2.78s p=0.806 n=6
Total Time 14.20s (± 0.26%) 14.23s (± 0.51%) ~ 14.16s 14.31s p=0.627 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,114 944,114 ~ ~ ~ p=1.000 n=6
Types 407,050 407,037 -13 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 1,218,350k (± 0.01%) 1,218,390k (± 0.00%) ~ 1,218,329k 1,218,477k p=0.378 n=6
Parse Time 6.65s (± 0.51%) 6.67s (± 0.42%) ~ 6.63s 6.70s p=0.333 n=6
Bind Time 1.86s (± 0.44%) 1.86s (± 0.81%) ~ 1.84s 1.88s p=0.797 n=6
Check Time 30.65s (± 0.18%) 30.61s (± 0.28%) ~ 30.48s 30.71s p=0.575 n=6
Emit Time 13.64s (± 0.45%) 13.55s (± 0.23%) -0.08s (- 0.61%) 13.50s 13.59s p=0.020 n=6
Total Time 52.80s (± 0.12%) 52.69s (± 0.19%) ~ 52.55s 52.81s p=0.065 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,133,039 2,133,039 ~ ~ ~ p=1.000 n=6
Types 926,444 926,444 ~ ~ ~ p=1.000 n=6
Memory used 2,115,494k (± 0.00%) 2,115,296k (± 0.00%) -198k (- 0.01%) 2,115,188k 2,115,390k p=0.005 n=6
Parse Time 7.90s (± 0.46%) 7.90s (± 0.35%) ~ 7.87s 7.94s p=0.936 n=6
Bind Time 2.75s (± 1.23%) 2.75s (± 0.67%) ~ 2.72s 2.77s p=0.686 n=6
Check Time 82.83s (± 1.19%) 83.41s (± 0.44%) ~ 83.04s 83.94s p=0.298 n=6
Emit Time 0.15s (± 3.53%) 0.16s (± 3.95%) ~ 0.15s 0.17s p=0.201 n=6
Total Time 93.63s (± 1.06%) 94.22s (± 0.40%) ~ 93.84s 94.73s p=0.173 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,260 1,225,260 ~ ~ ~ p=1.000 n=6
Types 261,341 261,340 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 2,341,242k (± 0.05%) 2,340,603k (± 0.04%) ~ 2,339,850k 2,342,479k p=0.261 n=6
Parse Time 5.05s (± 1.04%) 4.99s (± 1.12%) ~ 4.92s 5.07s p=0.109 n=6
Bind Time 1.90s (± 1.05%) 1.91s (± 0.70%) ~ 1.89s 1.92s p=0.417 n=6
Check Time 34.27s (± 0.53%) 34.13s (± 0.53%) ~ 33.98s 34.45s p=0.093 n=6
Emit Time 2.56s (± 3.29%) 2.58s (± 1.83%) ~ 2.49s 2.63s p=0.748 n=6
Total Time 43.81s (± 0.49%) 43.62s (± 0.41%) ~ 43.42s 43.94s p=0.066 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,260 1,225,260 ~ ~ ~ p=1.000 n=6
Types 261,341 261,340 -1 (- 0.00%) ~ ~ p=0.001 n=6
Memory used 2,415,802k (± 0.14%) 2,414,055k (± 0.03%) ~ 2,413,208k 2,415,260k p=0.230 n=6
Parse Time 5.21s (± 1.01%) 5.22s (± 0.62%) ~ 5.18s 5.26s p=0.688 n=6
Bind Time 1.69s (± 0.89%) 1.70s (± 0.96%) ~ 1.68s 1.72s p=0.737 n=6
Check Time 34.81s (± 0.50%) 34.90s (± 0.61%) ~ 34.73s 35.27s p=0.575 n=6
Emit Time 2.64s (± 1.00%) 2.67s (± 2.41%) ~ 2.59s 2.76s p=0.470 n=6
Total Time 44.35s (± 0.46%) 44.48s (± 0.59%) ~ 44.19s 44.91s p=0.378 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,194 258,194 ~ ~ ~ p=1.000 n=6
Types 104,737 104,737 ~ ~ ~ p=1.000 n=6
Memory used 427,481k (± 0.01%) 427,451k (± 0.00%) ~ 427,419k 427,475k p=0.092 n=6
Parse Time 4.08s (± 0.31%) 4.08s (± 0.57%) ~ 4.06s 4.12s p=1.000 n=6
Bind Time 1.61s (± 1.39%) 1.63s (± 0.50%) ~ 1.61s 1.63s p=0.421 n=6
Check Time 22.04s (± 0.37%) 22.02s (± 0.52%) ~ 21.90s 22.21s p=0.470 n=6
Emit Time 1.55s (± 1.66%) 1.56s (± 1.26%) ~ 1.54s 1.59s p=0.571 n=6
Total Time 29.28s (± 0.35%) 29.28s (± 0.42%) ~ 29.16s 29.49s p=0.748 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,734 ~ ~ ~ p=1.000 n=6
Memory used 369,397k (± 0.01%) 369,470k (± 0.02%) ~ 369,381k 369,578k p=0.149 n=6
Parse Time 2.78s (± 1.09%) 2.76s (± 1.25%) ~ 2.71s 2.81s p=0.261 n=6
Bind Time 1.59s (± 0.76%) 1.60s (± 1.70%) ~ 1.57s 1.64s p=0.317 n=6
Check Time 15.50s (± 0.35%) 15.44s (± 0.43%) ~ 15.34s 15.54s p=0.066 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 19.87s (± 0.31%) 19.79s (± 0.34%) -0.08s (- 0.39%) 19.72s 19.92s p=0.037 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,879,835 2,879,835 ~ ~ ~ p=1.000 n=6
Types 975,372 975,372 ~ ~ ~ p=1.000 n=6
Memory used 3,042,585k (± 0.00%) 3,042,555k (± 0.00%) ~ 3,042,452k 3,042,657k p=0.471 n=6
Parse Time 13.66s (± 0.31%) 13.64s (± 0.29%) ~ 13.56s 13.67s p=0.260 n=6
Bind Time 4.20s (± 0.53%) 4.36s (± 2.48%) ~ 4.14s 4.41s p=0.059 n=6
Check Time 75.16s (± 2.21%) 73.89s (± 1.60%) ~ 73.24s 76.30s p=0.066 n=6
Emit Time 22.84s (± 6.26%) 23.67s (± 4.69%) ~ 21.42s 24.37s p=0.297 n=6
Total Time 115.86s (± 0.29%) 115.56s (± 0.20%) ~ 115.32s 115.98s p=0.173 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,117 267,117 ~ ~ ~ p=1.000 n=6
Types 108,775 108,775 ~ ~ ~ p=1.000 n=6
Memory used 411,597k (± 0.01%) 411,638k (± 0.01%) ~ 411,558k 411,693k p=0.230 n=6
Parse Time 3.82s (± 0.36%) 3.82s (± 0.61%) ~ 3.79s 3.85s p=0.935 n=6
Bind Time 1.70s (± 0.49%) 1.70s (± 0.44%) ~ 1.69s 1.71s p=0.652 n=6
Check Time 16.80s (± 0.30%) 16.77s (± 0.49%) ~ 16.64s 16.85s p=0.809 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.32s (± 0.26%) 22.29s (± 0.33%) ~ 22.17s 22.35s p=0.810 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 525,251 525,251 ~ ~ ~ p=1.000 n=6
Types 178,574 178,574 ~ ~ ~ p=1.000 n=6
Memory used 462,951k (± 0.07%) 462,837k (± 0.08%) ~ 462,329k 463,188k p=0.689 n=6
Parse Time 3.18s (± 0.50%) 3.19s (± 0.59%) ~ 3.16s 3.21s p=0.560 n=6
Bind Time 1.17s (± 0.35%) 1.17s (± 0.54%) ~ 1.16s 1.18s p=0.673 n=6
Check Time 17.97s (± 0.73%) 17.92s (± 0.31%) ~ 17.84s 17.99s p=0.521 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.32s (± 0.56%) 22.28s (± 0.23%) ~ 22.19s 22.35s p=0.809 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

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

Everything looks good!

@JoostK
Copy link
Contributor

JoostK commented Jul 8, 2024

Related: #59177 (comment)

@ahejlsberg
Copy link
Member

The proper fix here is #59177. It reverts the changes in #56753 which is the cause of the inconsistent tooltips.

@Andarist Andarist closed this Jul 9, 2024
@Andarist Andarist deleted the fix/dont-skip-assigning-resolved-typeonly branch July 9, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants