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

Bind comma expression LHS call expressions #41312

Merged
merged 1 commit into from
Oct 28, 2020
Merged

Bind comma expression LHS call expressions #41312

merged 1 commit into from
Oct 28, 2020

Conversation

weswigham
Copy link
Member

We originally made a conscious choice to, generally speaking, only bind calls when they're the child of an expression statement. This expands our binding to also include the LHS of comma expressions, since, just like lists of statements, they generally only exist to compose lists of possibly-side-effecting actions (like, say, assertions).

Fixes #41264

@weswigham
Copy link
Member Author

@typescript-bot perf test this
@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 28, 2020 •
edited
Loading

Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 4ec02a5. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 28, 2020 •
edited
Loading

Heya @weswigham, I've started to run the perf test suite on this PR at 4ec02a5. You can monitor the build here.

Update: The results are in!

@typescript-bot typescript-bot added Author: Team For Backlog Bug PRs that fix a backlog bug labels Oct 28, 2020
@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..41312

Metric master 41312 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 350,663k (± 0.03%) 350,605k (± 0.01%) -59k (- 0.02%) 350,514k 350,693k
Parse Time 2.07s (± 0.56%) 2.10s (± 1.17%) +0.03s (+ 1.40%) 2.07s 2.19s
Bind Time 0.84s (± 0.57%) 0.84s (± 1.06%) +0.01s (+ 0.84%) 0.82s 0.87s
Check Time 4.98s (± 0.40%) 5.00s (± 0.60%) +0.02s (+ 0.34%) 4.91s 5.04s
Emit Time 5.32s (± 0.64%) 5.31s (± 0.59%) -0.01s (- 0.15%) 5.25s 5.38s
Total Time 13.21s (± 0.30%) 13.26s (± 0.43%) +0.04s (+ 0.31%) 13.13s 13.38s
Monaco - node (v10.16.3, x64)
Memory used 354,609k (± 0.02%) 354,568k (± 0.02%) -42k (- 0.01%) 354,379k 354,705k
Parse Time 1.60s (± 0.75%) 1.61s (± 0.47%) +0.00s (+ 0.19%) 1.59s 1.62s
Bind Time 0.72s (± 0.65%) 0.72s (± 0.80%) +0.00s (+ 0.42%) 0.71s 0.74s
Check Time 5.10s (± 0.57%) 5.12s (± 0.67%) +0.02s (+ 0.41%) 5.04s 5.19s
Emit Time 2.79s (± 0.77%) 2.78s (± 0.86%) -0.01s (- 0.39%) 2.74s 2.86s
Total Time 10.21s (± 0.49%) 10.23s (± 0.53%) +0.02s (+ 0.15%) 10.10s 10.34s
TFS - node (v10.16.3, x64)
Memory used 307,812k (± 0.02%) 307,829k (± 0.01%) +17k (+ 0.01%) 307,764k 307,878k
Parse Time 1.24s (± 0.60%) 1.24s (± 0.60%) +0.00s (+ 0.24%) 1.23s 1.26s
Bind Time 0.67s (± 0.92%) 0.67s (± 1.47%) -0.00s (- 0.45%) 0.65s 0.69s
Check Time 4.58s (± 0.47%) 4.54s (± 0.56%) -0.03s (- 0.68%) 4.49s 4.58s
Emit Time 2.95s (± 1.18%) 2.94s (± 1.31%) -0.00s (- 0.14%) 2.86s 3.02s
Total Time 9.43s (± 0.59%) 9.39s (± 0.57%) -0.03s (- 0.37%) 9.30s 9.52s
material-ui - node (v10.16.3, x64)
Memory used 489,229k (± 0.01%) 489,285k (± 0.02%) +55k (+ 0.01%) 489,064k 489,391k
Parse Time 2.06s (± 0.40%) 2.07s (± 0.47%) +0.01s (+ 0.63%) 2.05s 2.09s
Bind Time 0.65s (± 0.62%) 0.65s (± 0.68%) +0.00s (+ 0.46%) 0.64s 0.66s
Check Time 13.52s (± 0.65%) 13.50s (± 0.45%) -0.03s (- 0.19%) 13.34s 13.66s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.23s (± 0.52%) 16.22s (± 0.40%) -0.01s (- 0.07%) 16.05s 16.38s
Angular - node (v12.1.0, x64)
Memory used 327,861k (± 0.02%) 327,858k (± 0.02%) -3k (- 0.00%) 327,707k 327,997k
Parse Time 2.08s (± 0.62%) 2.07s (± 0.75%) -0.01s (- 0.53%) 2.04s 2.11s
Bind Time 0.82s (± 0.91%) 0.82s (± 0.75%) +0.00s (+ 0.12%) 0.81s 0.84s
Check Time 4.92s (± 0.94%) 4.91s (± 0.42%) -0.02s (- 0.32%) 4.86s 4.95s
Emit Time 5.48s (± 0.57%) 5.50s (± 0.95%) +0.02s (+ 0.33%) 5.40s 5.66s
Total Time 13.31s (± 0.53%) 13.30s (± 0.56%) -0.00s (- 0.04%) 13.17s 13.52s
Monaco - node (v12.1.0, x64)
Memory used 336,801k (± 0.01%) 336,765k (± 0.01%) -36k (- 0.01%) 336,640k 336,849k
Parse Time 1.58s (± 0.93%) 1.59s (± 0.61%) +0.01s (+ 0.89%) 1.56s 1.61s
Bind Time 0.70s (± 0.70%) 0.70s (± 1.08%) +0.00s (+ 0.14%) 0.69s 0.72s
Check Time 4.90s (± 0.48%) 4.91s (± 0.41%) +0.01s (+ 0.16%) 4.87s 4.96s
Emit Time 2.86s (± 0.75%) 2.87s (± 1.07%) +0.01s (+ 0.42%) 2.82s 2.97s
Total Time 10.04s (± 0.34%) 10.07s (± 0.45%) +0.04s (+ 0.37%) 9.96s 10.20s
TFS - node (v12.1.0, x64)
Memory used 292,096k (± 0.01%) 292,075k (± 0.02%) -21k (- 0.01%) 291,980k 292,179k
Parse Time 1.25s (± 0.80%) 1.25s (± 0.68%) +0.00s (+ 0.16%) 1.23s 1.26s
Bind Time 0.65s (± 1.03%) 0.64s (± 0.76%) -0.00s (- 0.46%) 0.63s 0.65s
Check Time 4.50s (± 0.46%) 4.50s (± 0.63%) -0.01s (- 0.11%) 4.43s 4.56s
Emit Time 2.95s (± 0.65%) 2.94s (± 0.55%) -0.00s (- 0.10%) 2.89s 2.96s
Total Time 9.34s (± 0.33%) 9.33s (± 0.43%) -0.01s (- 0.11%) 9.22s 9.41s
material-ui - node (v12.1.0, x64)
Memory used 467,236k (± 0.05%) 467,137k (± 0.07%) -100k (- 0.02%) 466,177k 467,556k
Parse Time 2.08s (± 0.45%) 2.08s (± 0.61%) -0.00s (- 0.05%) 2.06s 2.10s
Bind Time 0.64s (± 1.16%) 0.64s (± 0.76%) +0.00s (+ 0.16%) 0.63s 0.65s
Check Time 12.15s (± 0.75%) 12.16s (± 0.78%) +0.00s (+ 0.01%) 11.91s 12.37s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.88s (± 0.60%) 14.89s (± 0.61%) +0.00s (+ 0.02%) 14.66s 15.11s
Angular - node (v8.9.0, x64)
Memory used 353,060k (± 0.02%) 353,046k (± 0.02%) -14k (- 0.00%) 352,871k 353,210k
Parse Time 2.65s (± 0.40%) 2.66s (± 0.69%) +0.01s (+ 0.45%) 2.63s 2.71s
Bind Time 0.89s (± 0.84%) 0.89s (± 1.21%) +0.00s (+ 0.45%) 0.87s 0.93s
Check Time 5.58s (± 0.52%) 5.61s (± 0.69%) +0.03s (+ 0.48%) 5.55s 5.71s
Emit Time 6.32s (± 1.22%) 6.37s (± 1.23%) +0.05s (+ 0.85%) 6.23s 6.60s
Total Time 15.44s (± 0.52%) 15.53s (± 0.62%) +0.09s (+ 0.60%) 15.36s 15.82s
Monaco - node (v8.9.0, x64)
Memory used 358,413k (± 0.01%) 358,413k (± 0.02%) +0k (+ 0.00%) 358,290k 358,635k
Parse Time 1.93s (± 0.46%) 1.94s (± 0.31%) +0.01s (+ 0.47%) 1.92s 1.94s
Bind Time 0.91s (± 0.61%) 0.90s (± 0.91%) -0.00s (- 0.44%) 0.88s 0.92s
Check Time 5.64s (± 0.53%) 5.66s (± 0.53%) +0.02s (+ 0.34%) 5.60s 5.71s
Emit Time 3.39s (± 0.66%) 3.38s (± 1.39%) -0.01s (- 0.24%) 3.25s 3.46s
Total Time 11.86s (± 0.37%) 11.87s (± 0.54%) +0.02s (+ 0.14%) 11.71s 12.03s
TFS - node (v8.9.0, x64)
Memory used 310,291k (± 0.02%) 310,255k (± 0.02%) -36k (- 0.01%) 310,141k 310,404k
Parse Time 1.57s (± 0.37%) 1.57s (± 0.56%) +0.00s (+ 0.00%) 1.56s 1.60s
Bind Time 0.68s (± 0.00%) 0.69s (± 0.50%) +0.01s (+ 1.03%) 0.68s 0.69s
Check Time 5.34s (± 0.58%) 5.33s (± 0.64%) -0.01s (- 0.19%) 5.26s 5.41s
Emit Time 2.95s (± 0.72%) 2.97s (± 0.72%) +0.01s (+ 0.41%) 2.92s 3.02s
Total Time 10.55s (± 0.41%) 10.55s (± 0.29%) +0.01s (+ 0.08%) 10.50s 10.61s
material-ui - node (v8.9.0, x64)
Memory used 496,137k (± 0.01%) 496,183k (± 0.01%) +46k (+ 0.01%) 496,073k 496,324k
Parse Time 2.50s (± 0.55%) 2.50s (± 0.42%) +0.00s (+ 0.08%) 2.48s 2.53s
Bind Time 0.82s (± 1.00%) 0.82s (± 1.30%) -0.01s (- 0.97%) 0.80s 0.84s
Check Time 17.98s (± 0.57%) 17.93s (± 0.76%) -0.05s (- 0.28%) 17.54s 18.24s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.30s (± 0.49%) 21.24s (± 0.63%) -0.06s (- 0.28%) 20.88s 21.54s
Angular - node (v8.9.0, x86)
Memory used 202,063k (± 0.03%) 202,112k (± 0.01%) +49k (+ 0.02%) 202,032k 202,167k
Parse Time 2.56s (± 0.87%) 2.58s (± 1.21%) +0.02s (+ 0.94%) 2.53s 2.68s
Bind Time 1.01s (± 1.06%) 1.02s (± 0.71%) +0.01s (+ 0.89%) 1.01s 1.04s
Check Time 5.06s (± 0.59%) 5.10s (± 0.80%) +0.04s (+ 0.73%) 4.97s 5.19s
Emit Time 6.10s (± 0.47%) 6.10s (± 0.63%) +0.00s (+ 0.05%) 6.04s 6.19s
Total Time 14.73s (± 0.25%) 14.80s (± 0.62%) +0.07s (+ 0.48%) 14.60s 14.98s
Monaco - node (v8.9.0, x86)
Memory used 202,909k (± 0.02%) 202,953k (± 0.02%) +44k (+ 0.02%) 202,886k 203,015k
Parse Time 1.97s (± 0.63%) 1.98s (± 0.73%) +0.01s (+ 0.51%) 1.96s 2.03s
Bind Time 0.71s (± 0.78%) 0.72s (± 0.66%) +0.01s (+ 0.70%) 0.71s 0.73s
Check Time 5.75s (± 1.04%) 5.77s (± 0.68%) +0.02s (+ 0.33%) 5.68s 5.90s
Emit Time 2.78s (± 2.41%) 2.77s (± 1.25%) -0.01s (- 0.47%) 2.73s 2.90s
Total Time 11.22s (± 0.49%) 11.24s (± 0.59%) +0.02s (+ 0.20%) 11.16s 11.49s
TFS - node (v8.9.0, x86)
Memory used 177,394k (± 0.02%) 177,386k (± 0.02%) -8k (- 0.00%) 177,245k 177,465k
Parse Time 1.61s (± 0.95%) 1.61s (± 0.91%) +0.00s (+ 0.06%) 1.59s 1.65s
Bind Time 0.65s (± 1.40%) 0.65s (± 1.26%) +0.01s (+ 0.77%) 0.64s 0.68s
Check Time 4.90s (± 0.68%) 4.84s (± 0.71%) -0.06s (- 1.25%) 4.78s 4.91s
Emit Time 2.85s (± 0.95%) 2.85s (± 1.36%) 0.00s ( 0.00%) 2.75s 2.94s
Total Time 10.01s (± 0.53%) 9.96s (± 0.53%) -0.05s (- 0.53%) 9.83s 10.09s
material-ui - node (v8.9.0, x86)
Memory used 279,262k (± 0.01%) 279,271k (± 0.02%) +9k (+ 0.00%) 279,122k 279,395k
Parse Time 2.56s (± 0.50%) 2.55s (± 0.61%) -0.01s (- 0.39%) 2.52s 2.58s
Bind Time 0.72s (± 5.40%) 0.72s (± 3.73%) +0.01s (+ 0.83%) 0.69s 0.82s
Check Time 16.53s (± 0.67%) 16.36s (± 0.67%) -0.17s (- 1.05%) 16.13s 16.63s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.81s (± 0.66%) 19.64s (± 0.54%) -0.18s (- 0.89%) 19.45s 19.90s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 41312 10
Baseline master 10

@typescript-bot typescript-bot mentioned this pull request Oct 28, 2020
Open
@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@weswigham weswigham merged commit 3de6ed0 into microsoft:master Oct 28, 2020
@weswigham weswigham deleted the maybe-bind-comma-lhs branch October 28, 2020 22:12
@fatcerberus fatcerberus mentioned this pull request Jun 7, 2021
Closed
@jakebailey jakebailey mentioned this pull request Dec 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewbranch andrewbranch andrewbranch approved these changes

@sandersn sandersn Awaiting requested review from sandersn

@ahejlsberg ahejlsberg Awaiting requested review from ahejlsberg

Assignees

@weswigham weswigham

Labels
Author: Team For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Assertions do not narrow types when used as operand of the comma operator.
3 participants
@weswigham @typescript-bot @andrewbranch