Skip to content

Derive tuple labels for rest elements from array binding patterns - #59045

Merged
Ron Buckton (rbuckton) merged 4 commits into
mainfrom
tuple-name-from-binding-element
Jul 17, 2024
Merged

Derive tuple labels for rest elements from array binding patterns#59045
Ron Buckton (rbuckton) merged 4 commits into
mainfrom
tuple-name-from-binding-element

Conversation

@rbuckton

@rbucktonRon Buckton (rbuckton) commented Jun 26, 2024

Copy link
Copy Markdown
Contributor

While working on #58243, a community member asked if we could label the tuple we use to properly type arguments to the next() methods of iterators and generators. A brief experiment showed that directly labeling the tuple element resulted in the creation of thousands of new types in some tests since iterator/generator instantiations would produce new named tuple elements since named tuple elements are cached far less frequently.

An alternative to that approach would be to use an additional source for the label when the label comes from a rest parameter whose name is actually an ArrayBindingPattern:

declarefunctiona(...args: []|[number]): void;a(1);// quickinfo shows `(args_0: number)`declarefunctionb(...[value] : []|[number]): void;b(1);// quickinfo now shows `(value: number)`// quickinfo previously showed `(__0_0: number)`

Since this is merely treated as an additional labeling source, there is no need to synthesize additional types for named tuple elements.

This also improves label inference in other cases such as elision and object assignment pattern elements:

declarefunctiona(...[x,,z]: [number,number,number]): void;a(1,2,3);// quickinfo shows `(x: number, arg_1: number, z: number)`// previously showed `(__0_0: number, __0_1: number, __0_2: number)`

Related #58243 (comment)

Fixes#56289
Closes#57619

@DanielRosenwasser

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test it

@typescript-bot

TypeScript Bot (typescript-bot) commented Jun 27, 2024

Copy link
Copy Markdown
Contributor

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

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

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Daniel Rosenwasser (@DanielRosenwasser), the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser) Here are the results of running the user tests with tsc comparing main and refs/pull/59045/merge:

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser)
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-Unions - node (v18.15.0, x64)
Errors3030~~~p=1.000 n=6
Symbols62,15362,153~~~p=1.000 n=6
Types50,24250,242~~~p=1.000 n=6
Memory used192,789k (± 0.75%)192,789k (± 0.73%)~192,133k195,637kp=0.575 n=6
Parse Time1.31s (± 0.57%)1.30s (± 1.05%)~1.28s1.32sp=0.273 n=6
Bind Time0.71s0.71s~~~p=1.000 n=6
Check Time9.44s (± 0.38%)9.44s (± 0.69%)~9.38s9.55sp=0.936 n=6
Emit Time2.75s (± 0.83%)2.74s (± 1.37%)~2.67s2.78sp=1.000 n=6
Total Time14.21s (± 0.30%)14.20s (± 0.24%)~14.17s14.25sp=0.744 n=6
angular-1 - node (v18.15.0, x64)
Errors55~~~p=1.000 n=6
Symbols944,114944,114~~~p=1.000 n=6
Types407,050407,050~~~p=1.000 n=6
Memory used1,218,382k (± 0.00%)1,218,347k (± 0.00%)~1,218,275k1,218,373kp=0.335 n=6
Parse Time6.67s (± 0.74%)6.66s (± 0.92%)~6.59s6.76sp=0.809 n=6
Bind Time1.87s (± 0.55%)1.87s (± 0.56%)~1.85s1.88sp=0.801 n=6
Check Time30.64s (± 0.19%)30.72s (± 0.29%)~30.59s30.81sp=0.148 n=6
Emit Time13.54s (± 0.68%)13.57s (± 0.31%)~13.52s13.62sp=1.000 n=6
Total Time52.72s (± 0.16%)52.81s (± 0.23%)~52.65s52.97sp=0.229 n=6
mui-docs - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols2,132,3862,132,386~~~p=1.000 n=6
Types926,170926,170~~~p=1.000 n=6
Memory used2,114,738k (± 0.01%)2,114,843k (± 0.01%)~2,114,648k2,115,034kp=0.173 n=6
Parse Time9.72s (± 0.25%)9.73s (± 0.31%)~9.70s9.78sp=0.685 n=6
Bind Time3.40s (± 0.69%)3.39s (± 0.94%)~3.34s3.43sp=0.683 n=6
Check Time103.00s (± 0.57%)102.02s (± 1.35%)~99.52s103.17sp=0.173 n=6
Emit Time0.19s (± 2.13%)0.19s (± 2.67%)~0.19s0.20sp=0.595 n=6
Total Time116.31s (± 0.50%)115.33s (± 1.18%)~112.87s116.46sp=0.173 n=6
self-build-src - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,231,6401,231,657+17 (+ 0.00%)~~p=0.001 n=6
Types261,187261,211+24 (+ 0.01%)~~p=0.001 n=6
Memory used2,347,576k (± 0.04%)2,347,596k (± 0.03%)~2,346,508k2,348,766kp=1.000 n=6
Parse Time5.06s (± 0.44%)5.05s (± 0.45%)~5.03s5.09sp=0.810 n=6
Bind Time1.93s (± 0.79%)1.93s (± 0.42%)~1.93s1.95sp=0.209 n=6
Check Time34.05s (± 0.22%)34.16s (± 0.49%)~33.94s34.38sp=0.230 n=6
Emit Time2.76s (± 2.56%)2.73s (± 1.95%)~2.67s2.81sp=0.471 n=6
Total Time43.82s (± 0.30%)43.90s (± 0.41%)~43.72s44.16sp=0.689 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,231,6401,231,657+17 (+ 0.00%)~~p=0.001 n=6
Types261,187261,211+24 (+ 0.01%)~~p=0.001 n=6
Memory used2,424,424k (± 0.06%)2,423,830k (± 0.02%)~2,423,060k2,424,766kp=0.936 n=6
Parse Time6.25s (± 0.61%)6.24s (± 1.16%)~6.15s6.36sp=0.630 n=6
Bind Time2.05s (± 0.67%)2.04s (± 0.74%)~2.01s2.05sp=0.324 n=6
Check Time40.40s (± 0.26%)40.51s (± 0.19%)~40.40s40.61sp=0.128 n=6
Emit Time3.24s (± 2.66%)3.25s (± 2.35%)~3.17s3.36sp=0.873 n=6
Total Time51.95s (± 0.34%)52.04s (± 0.31%)~51.87s52.29sp=0.422 n=6
self-compiler - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols258,818258,835+17 (+ 0.01%)~~p=0.001 n=6
Types104,842104,866+24 (+ 0.02%)~~p=0.001 n=6
Memory used428,204k (± 0.01%)428,303k (± 0.01%)+100k (+ 0.02%)428,252k428,362kp=0.008 n=6
Parse Time3.31s (± 1.17%)3.33s (± 1.18%)~3.30s3.41sp=0.455 n=6
Bind Time1.31s (± 1.64%)1.31s (± 1.31%)~1.29s1.34sp=1.000 n=6
Check Time17.80s (± 0.26%)17.80s (± 0.37%)~17.70s17.86sp=0.872 n=6
Emit Time1.36s (± 1.59%)1.38s (± 0.92%)~1.36s1.39sp=0.217 n=6
Total Time23.79s (± 0.34%)23.82s (± 0.41%)~23.68s23.95sp=0.574 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors3535~~~p=1.000 n=6
Symbols224,565224,565~~~p=1.000 n=6
Types93,73493,734~~~p=1.000 n=6
Memory used369,518k (± 0.03%)369,412k (± 0.02%)~369,312k369,477kp=0.093 n=6
Parse Time2.78s (± 1.87%)2.77s (± 0.96%)~2.72s2.80sp=0.126 n=6
Bind Time1.58s (± 1.03%)1.58s (± 0.67%)~1.56s1.59sp=0.451 n=6
Check Time15.47s (± 0.32%)15.45s (± 0.39%)~15.36s15.51sp=0.688 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time19.84s (± 0.27%)19.79s (± 0.42%)~19.65s19.87sp=0.261 n=6
vscode - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols2,878,5792,878,579~~~p=1.000 n=6
Types975,166975,166~~~p=1.000 n=6
Memory used3,042,078k (± 0.00%)3,042,073k (± 0.00%)~3,042,019k3,042,125kp=0.936 n=6
Parse Time13.54s (± 0.32%)13.59s (± 0.23%)~13.54s13.63sp=0.056 n=6
Bind Time4.22s (± 2.03%)4.18s (± 0.21%)~4.17s4.19sp=0.459 n=6
Check Time73.36s (± 0.39%)73.21s (± 0.27%)~73.00s73.48sp=0.230 n=6
Emit Time23.98s (± 0.38%)23.98s (± 0.24%)~23.91s24.05sp=0.936 n=6
Total Time115.09s (± 0.30%)114.96s (± 0.17%)~114.79s115.31sp=0.378 n=6
webpack - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols267,117267,117~~~p=1.000 n=6
Types108,775108,775~~~p=1.000 n=6
Memory used411,543k (± 0.01%)411,524k (± 0.02%)~411,431k411,593kp=0.810 n=6
Parse Time3.19s (± 0.54%)3.19s (± 0.55%)~3.17s3.22sp=0.870 n=6
Bind Time1.42s (± 0.59%)1.41s (± 0.37%)~1.41s1.42sp=0.533 n=6
Check Time14.20s (± 0.37%)14.22s (± 0.51%)~14.13s14.30sp=0.520 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time18.80s (± 0.21%)18.83s (± 0.36%)~18.76s18.91sp=0.521 n=6
xstate-main - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols525,251525,251~~~p=1.000 n=6
Types178,574178,574~~~p=1.000 n=6
Memory used462,859k (± 0.09%)462,680k (± 0.08%)~462,336k463,129kp=0.471 n=6
Parse Time3.17s (± 0.68%)3.16s (± 0.99%)~3.13s3.21sp=0.418 n=6
Bind Time1.17s (± 1.14%)1.17s~~~p=0.599 n=6
Check Time17.99s (± 0.24%)17.95s (± 0.33%)~17.89s18.03sp=0.296 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time22.34s (± 0.16%)22.28s (± 0.33%)~22.20s22.38sp=0.228 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)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@typescript-bot

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser) Here are the results of running the top 400 repos with tsc comparing main and refs/pull/59045/merge:

Everything looks good!

@Andarist

Copy link
Copy Markdown
Contributor

I already have a very related change to this open here. Both PRs touch the same area of the code and solve very overlapping problems, I think it would make sense to merge both

@jakebailey

Copy link
Copy Markdown
Member

Seems fine to me, but does need a sync from main and baseline update.

@rbuckton
Ron Buckton (rbuckton)force-pushed the tuple-name-from-binding-element branch from a497918 to ae645c3CompareJuly 17, 2024 01:05
@rbuckton
Ron Buckton (rbuckton) deleted the tuple-name-from-binding-element branch July 17, 2024 19:13
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: TeamFor Backlog BugPRs that fix a backlog bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default rest parameter names to destructuring names when of an anonymous tuple

6 participants

@rbuckton@DanielRosenwasser@typescript-bot@Andarist@jakebailey@sandersn