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.

Sign up for GitHub

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

Jump to bottom

Don't exempt late bound symbols from EPC in presence of string index signature #58190

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Don't exempt late bound symbols from EPC in presence of string index signature #58190

wants to merge 2 commits into from

Conversation

Andarist
Copy link
Contributor

fixes #44794

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Apr 14, 2024
@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2024 •
edited

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 comparing main and refs/pull/58190/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
Angular - node (v18.15.0, x64)
Memory used 296,977k (± 0.01%) 296,984k (± 0.01%) ~ 296,928k 297,007k p=0.261 n=6
Parse Time 2.70s (± 0.51%) 2.71s (± 0.56%) ~ 2.68s 2.72s p=0.187 n=6
Bind Time 0.82s (± 0.77%) 0.82s (± 0.63%) ~ 0.81s 0.82s p=0.386 n=6
Check Time 8.31s (± 0.20%) 8.33s (± 0.30%) ~ 8.29s 8.36s p=0.331 n=6
Emit Time 7.07s (± 0.42%) 7.06s (± 0.32%) ~ 7.04s 7.10s p=0.625 n=6
Total Time 18.90s (± 0.13%) 18.91s (± 0.26%) ~ 18.84s 18.97s p=0.520 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 192,974k (± 0.94%) 192,401k (± 0.74%) ~ 191,794k 195,323k p=0.936 n=6
Parse Time 1.35s (± 0.30%) 1.37s (± 1.10%) ~ 1.35s 1.38s p=0.071 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.59s (± 0.45%) 9.58s (± 0.23%) ~ 9.54s 9.60s p=0.936 n=6
Emit Time 2.61s (± 0.95%) 2.62s (± 0.31%) ~ 2.61s 2.63s p=0.181 n=6
Total Time 14.27s (± 0.40%) 14.29s (± 0.15%) ~ 14.26s 14.31s p=0.418 n=6
Monaco - node (v18.15.0, x64)
Memory used 348,866k (± 0.00%) 348,851k (± 0.00%) ~ 348,837k 348,875k p=0.109 n=6
Parse Time 2.54s (± 0.39%) 2.55s (± 0.41%) ~ 2.53s 2.56s p=0.343 n=6
Bind Time 0.89s (± 0.71%) 0.89s (± 0.46%) ~ 0.88s 0.89s p=0.673 n=6
Check Time 7.07s (± 0.41%) 7.04s (± 0.51%) ~ 7.00s 7.08s p=0.147 n=6
Emit Time 4.07s (± 0.26%) 4.06s (± 0.33%) ~ 4.04s 4.08s p=0.358 n=6
Total Time 14.57s (± 0.13%) 14.53s (± 0.17%) -0.04s (- 0.24%) 14.50s 14.56s p=0.030 n=6
TFS - node (v18.15.0, x64)
Memory used 302,960k (± 0.01%) 302,951k (± 0.00%) ~ 302,940k 302,961k p=0.335 n=6
Parse Time 2.94s (± 0.90%) 2.95s (± 0.90%) ~ 2.92s 2.99s p=0.570 n=6
Bind Time 1.43s (± 0.93%) 1.44s (± 0.95%) ~ 1.43s 1.47s p=0.176 n=6
Check Time 9.37s (± 0.25%) 9.37s (± 0.34%) ~ 9.33s 9.41s p=1.000 n=6
Emit Time 5.29s (± 0.57%) 5.28s (± 0.65%) ~ 5.24s 5.34s p=0.936 n=6
Total Time 19.03s (± 0.29%) 19.05s (± 0.25%) ~ 18.99s 19.12s p=0.572 n=6
material-ui - node (v18.15.0, x64)
Memory used 511,588k (± 0.00%) 511,594k (± 0.00%) ~ 511,565k 511,609k p=0.378 n=6
Parse Time 2.72s (± 0.68%) 2.71s (± 0.64%) ~ 2.69s 2.73s p=0.451 n=6
Bind Time 0.99s (± 0.76%) 0.99s (± 1.74%) ~ 0.96s 1.01s p=0.867 n=6
Check Time 17.25s (± 0.44%) 17.17s (± 0.33%) ~ 17.09s 17.25s p=0.126 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.95s (± 0.39%) 20.86s (± 0.33%) ~ 20.76s 20.96s p=0.127 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,748,639k (± 0.00%) 1,748,684k (± 0.00%) ~ 1,748,637k 1,748,737k p=0.065 n=6
Parse Time 9.96s (± 0.58%) 9.97s (± 1.29%) ~ 9.84s 10.20s p=0.688 n=6
Bind Time 3.35s (± 0.99%) 3.34s (± 1.05%) ~ 3.31s 3.41s p=0.868 n=6
Check Time 81.38s (± 0.35%) 81.57s (± 0.43%) ~ 81.10s 82.12s p=0.378 n=6
Emit Time 0.20s (± 3.79%) 0.20s (± 5.89%) ~ 0.19s 0.22s p=0.796 n=6
Total Time 94.89s (± 0.30%) 95.08s (± 0.35%) ~ 94.71s 95.54s p=0.470 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,306,741k (± 0.01%) 2,307,007k (± 0.03%) ~ 2,306,333k 2,307,799k p=0.689 n=6
Parse Time 6.02s (± 1.12%) 6.02s (± 0.82%) ~ 5.96s 6.11s p=0.872 n=6
Bind Time 2.25s (± 0.87%) 2.24s (± 1.26%) ~ 2.20s 2.27s p=0.872 n=6
Check Time 39.84s (± 0.33%) 39.80s (± 0.36%) ~ 39.55s 39.95s p=0.689 n=6
Emit Time 3.22s (± 1.81%) 3.17s (± 3.69%) ~ 3.02s 3.32s p=0.630 n=6
Total Time 51.35s (± 0.15%) 51.27s (± 0.26%) ~ 51.04s 51.44s p=0.298 n=6
self-build-src-public-api - node (v18.15.0, x64)
Memory used 2,381,607k (± 0.03%) 2,381,639k (± 0.04%) ~ 2,380,104k 2,383,043k p=0.689 n=6
Parse Time 7.68s (± 0.80%) 7.65s (± 0.35%) ~ 7.61s 7.68s p=0.468 n=6
Bind Time 2.51s (± 0.87%) 2.52s (± 0.82%) ~ 2.49s 2.54s p=0.747 n=6
Check Time 49.77s (± 0.33%) 49.69s (± 0.91%) ~ 49.31s 50.29s p=0.423 n=6
Emit Time 3.95s (± 3.75%) 3.92s (± 2.03%) ~ 3.76s 3.97s p=1.000 n=6
Total Time 63.93s (± 0.32%) 63.80s (± 0.66%) ~ 63.43s 64.35s p=0.378 n=6
self-compiler - node (v18.15.0, x64)
Memory used 419,105k (± 0.01%) 419,077k (± 0.01%) ~ 419,041k 419,098k p=0.423 n=6
Parse Time 4.28s (± 3.84%) 4.20s (± 0.36%) ~ 4.18s 4.22s p=0.357 n=6
Bind Time 1.58s (± 1.44%) 1.59s (± 1.93%) ~ 1.56s 1.64s p=0.462 n=6
Check Time 22.28s (± 0.18%) 22.31s (± 0.49%) ~ 22.20s 22.49s p=0.629 n=6
Emit Time 1.71s (± 0.96%) 1.72s (± 1.51%) ~ 1.69s 1.75s p=1.000 n=6
Total Time 29.85s (± 0.57%) 29.82s (± 0.40%) ~ 29.67s 30.03s p=0.873 n=6
vscode - node (v18.15.0, x64)
Memory used 2,913,590k (± 0.00%) 2,913,480k (± 0.00%) -110k (- 0.00%) 2,913,292k 2,913,629k p=0.045 n=6
Parse Time 16.55s (± 0.52%) 16.54s (± 0.48%) ~ 16.45s 16.64s p=0.936 n=6
Bind Time 4.95s (± 0.64%) 5.02s (± 2.53%) ~ 4.90s 5.18s p=0.627 n=6
Check Time 88.26s (± 0.48%) 88.20s (± 0.23%) ~ 87.94s 88.41s p=1.000 n=6
Emit Time 23.88s (± 0.75%) 24.57s (± 7.73%) ~ 23.67s 28.44s p=0.688 n=6
Total Time 133.64s (± 0.30%) 134.34s (± 1.33%) ~ 133.27s 137.96s p=0.575 n=6
webpack - node (v18.15.0, x64)
Memory used 409,368k (± 0.01%) 409,383k (± 0.01%) ~ 409,323k 409,464k p=0.936 n=6
Parse Time 3.93s (± 1.02%) 3.92s (± 1.16%) ~ 3.84s 3.97s p=0.574 n=6
Bind Time 1.65s (± 0.25%) 1.65s (± 0.91%) ~ 1.63s 1.67s p=0.498 n=6
Check Time 16.94s (± 0.37%) 16.99s (± 0.24%) ~ 16.93s 17.04s p=0.199 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.53s (± 0.39%) 22.56s (± 0.27%) ~ 22.48s 22.62s p=0.572 n=6
xstate - node (v18.15.0, x64)
Memory used 672,049k (± 0.03%) 672,003k (± 0.02%) ~ 671,884k 672,184k p=0.689 n=6
Parse Time 4.11s (± 0.60%) 4.14s (± 0.32%) ~ 4.13s 4.16s p=0.056 n=6
Bind Time 1.88s (± 0.78%) 1.88s (± 0.93%) ~ 1.85s 1.89s p=1.000 n=6
Check Time 3.45s (± 0.54%) 3.45s (± 0.57%) ~ 3.42s 3.47s p=1.000 n=6
Emit Time 0.03s (±14.39%) 0.03s (±12.88%) ~ 0.03s 0.04s p=0.218 n=6
Total Time 9.47s (± 0.30%) 9.49s (± 0.39%) ~ 9.45s 9.54s p=0.332 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - 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)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate - 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 comparing main and refs/pull/58190/merge:

Everything looks good!

@jakebailey
Copy link
Member

Shocking

@typescript-bot test top800

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2024 •
edited

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

Command Status Results
test top800 ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 800 repos comparing main and refs/pull/58190/merge:

Everything looks good!

@Andarist
Copy link
Contributor Author

Symbols are not that popular in the wild and often might not even be used directly so I'm not that shocked ;p

@fatcerberus
Copy link

Symbols are not that popular in the wild

Given that you either write something ugly like foo[Symbol.someBuiltInSymbol] = whatever or else foo[localSymVar] = bar which is syntactically indistinguishable from a string key access (and therefore potentially confusing), people shying away from symbols isn't that surprising. IMO symbols were a cool concept but they really needed specialized syntax.

@sandersn sandersn added this to Not started in PR Backlog May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ahejlsberg ahejlsberg

Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
  
Not started
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Symbol properties should not be exempt from excess property checks in presence of a string index signature
5 participants
@Andarist @jakebailey @typescript-bot @fatcerberus @ahejlsberg