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

Improve checks for infinitely expanding recursive conditional types #46326

Merged
merged 5 commits into from
Oct 13, 2021
Merged

Improve checks for infinitely expanding recursive conditional types #46326

merged 5 commits into from
Oct 13, 2021

Conversation

ahejlsberg
Copy link
Member

Fixes #46183.
Fixes #46316.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Oct 12, 2021
@ahejlsberg
Copy link
Member Author

I haven't added regression tests yet, but have manually verified that both listed issues are fixed.

@ahejlsberg ahejlsberg requested a review from weswigham October 12, 2021 18:24
weswigham
weswigham approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

New regression tests that more closely reflect the reported issues would be nice, but in terms of implementation, this is pretty much what I proposed, so I'm obviously OK with it. 😄

tests/baselines/reference/infiniteConstraints.errors.txt
@@ -64,6 +63,4 @@ tests/cases/compiler/infiniteConstraints.ts(48,27): error TS2321: Excessive stac

type Conv<T, U = T> =
{ 0: [T]; 1: Prepend<T, Conv<ExactExtract<U, T>>>;}[U extends T ? 0 : 1];
~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2321: Excessive stack depth comparing types 'Conv<ExactExtract<U, T>, ExactExtract<U, T>>' and 'unknown[]'.
Copy link
Member

Choose a reason for hiding this comment

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

This test has flip-flopped weather or not there's an error here so many times. I guess that's what it's here for, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but it's a good change. The excessive stack depth error is basically a panic bail out, in this case triggered by a missing check for infinite conditional type recursion on the target side.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I just chuckle because when this test was first added it had this error, then it got removed by another change, then it got added back, and now it's being removed again. This test is surprisingly good at capturing how strict we are about comparing infinite types, even if that's not what it was meant to test (it was meant to test a crash).

@ahejlsberg
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 12, 2021 •
edited
Loading

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 6c633e6. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..46326

Metric main 46326 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 353,993k (± 0.01%) 353,934k (± 0.01%) -59k (- 0.02%) 353,838k 354,057k
Parse Time 1.95s (± 0.42%) 1.96s (± 0.49%) +0.01s (+ 0.51%) 1.93s 1.97s
Bind Time 0.84s (± 0.58%) 0.84s (± 0.62%) -0.01s (- 0.59%) 0.83s 0.85s
Check Time 5.45s (± 0.46%) 5.46s (± 0.57%) +0.01s (+ 0.18%) 5.39s 5.54s
Emit Time 5.86s (± 0.55%) 5.86s (± 0.76%) +0.00s (+ 0.02%) 5.80s 6.00s
Total Time 14.10s (± 0.40%) 14.12s (± 0.31%) +0.02s (+ 0.11%) 14.05s 14.23s
Compiler-Unions - node (v10.16.3, x64)
Memory used 203,855k (± 0.03%) 203,892k (± 0.03%) +37k (+ 0.02%) 203,804k 204,023k
Parse Time 0.79s (± 0.95%) 0.78s (± 0.75%) -0.01s (- 1.27%) 0.77s 0.79s
Bind Time 0.53s (± 0.95%) 0.53s (± 1.10%) +0.00s (+ 0.38%) 0.51s 0.54s
Check Time 7.94s (± 0.81%) 8.00s (± 1.09%) +0.06s (+ 0.72%) 7.86s 8.27s
Emit Time 2.46s (± 1.61%) 2.44s (± 0.92%) -0.02s (- 0.69%) 2.40s 2.52s
Total Time 11.72s (± 0.59%) 11.74s (± 0.83%) +0.03s (+ 0.25%) 11.58s 12.02s
Monaco - node (v10.16.3, x64)
Memory used 342,111k (± 0.02%) 342,132k (± 0.02%) +21k (+ 0.01%) 341,990k 342,285k
Parse Time 1.48s (± 0.46%) 1.48s (± 0.74%) +0.01s (+ 0.47%) 1.45s 1.50s
Bind Time 0.75s (± 0.78%) 0.74s (± 0.50%) -0.00s (- 0.27%) 0.74s 0.75s
Check Time 5.45s (± 0.83%) 5.48s (± 0.63%) +0.02s (+ 0.44%) 5.38s 5.55s
Emit Time 3.19s (± 1.00%) 3.14s (± 0.57%) -0.06s (- 1.75%) 3.10s 3.17s
Total Time 10.87s (± 0.42%) 10.84s (± 0.31%) -0.03s (- 0.28%) 10.76s 10.90s
TFS - node (v10.16.3, x64)
Memory used 304,749k (± 0.02%) 304,764k (± 0.02%) +15k (+ 0.01%) 304,591k 304,924k
Parse Time 1.20s (± 0.99%) 1.19s (± 0.79%) -0.01s (- 0.75%) 1.18s 1.22s
Bind Time 0.71s (± 1.02%) 0.70s (± 0.52%) -0.01s (- 1.12%) 0.70s 0.71s
Check Time 4.97s (± 0.34%) 4.96s (± 0.58%) -0.00s (- 0.06%) 4.90s 5.02s
Emit Time 3.31s (± 0.68%) 3.34s (± 1.12%) +0.03s (+ 1.06%) 3.29s 3.46s
Total Time 10.19s (± 0.33%) 10.20s (± 0.50%) +0.01s (+ 0.10%) 10.09s 10.35s
material-ui - node (v10.16.3, x64)
Memory used 471,077k (± 0.01%) 471,043k (± 0.01%) -33k (- 0.01%) 470,890k 471,200k
Parse Time 1.78s (± 0.42%) 1.77s (± 0.49%) -0.01s (- 0.67%) 1.75s 1.79s
Bind Time 0.66s (± 1.01%) 0.65s (± 0.91%) -0.01s (- 1.36%) 0.64s 0.67s
Check Time 14.48s (± 0.68%) 14.36s (± 0.31%) -0.11s (- 0.79%) 14.29s 14.48s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.92s (± 0.60%) 16.79s (± 0.29%) -0.13s (- 0.79%) 16.70s 16.90s
Angular - node (v12.1.0, x64)
Memory used 331,861k (± 0.02%) 331,895k (± 0.02%) +33k (+ 0.01%) 331,733k 332,067k
Parse Time 1.95s (± 0.62%) 1.96s (± 0.99%) +0.00s (+ 0.20%) 1.93s 2.01s
Bind Time 0.82s (± 0.81%) 0.82s (± 1.08%) -0.01s (- 0.73%) 0.80s 0.84s
Check Time 5.33s (± 0.46%) 5.33s (± 0.46%) +0.01s (+ 0.15%) 5.30s 5.41s
Emit Time 6.14s (± 0.93%) 6.19s (± 1.08%) +0.04s (+ 0.70%) 6.11s 6.43s
Total Time 14.25s (± 0.36%) 14.30s (± 0.54%) +0.05s (+ 0.34%) 14.18s 14.50s
Compiler-Unions - node (v12.1.0, x64)
Memory used 191,304k (± 0.07%) 191,321k (± 0.07%) +17k (+ 0.01%) 190,797k 191,536k
Parse Time 0.78s (± 0.85%) 0.78s (± 0.67%) -0.00s (- 0.13%) 0.77s 0.79s
Bind Time 0.53s (± 0.64%) 0.52s (± 1.13%) -0.01s (- 1.69%) 0.51s 0.54s
Check Time 7.47s (± 0.84%) 7.44s (± 0.62%) -0.02s (- 0.32%) 7.31s 7.53s
Emit Time 2.48s (± 0.89%) 2.47s (± 0.78%) -0.01s (- 0.24%) 2.42s 2.51s
Total Time 11.26s (± 0.64%) 11.22s (± 0.48%) -0.04s (- 0.35%) 11.09s 11.32s
Monaco - node (v12.1.0, x64)
Memory used 325,168k (± 0.02%) 325,166k (± 0.02%) -2k (- 0.00%) 325,044k 325,313k
Parse Time 1.47s (± 0.64%) 1.47s (± 0.81%) +0.00s (+ 0.00%) 1.43s 1.49s
Bind Time 0.73s (± 0.79%) 0.73s (± 0.80%) -0.00s (- 0.41%) 0.72s 0.74s
Check Time 5.34s (± 0.48%) 5.36s (± 0.55%) +0.02s (+ 0.37%) 5.31s 5.46s
Emit Time 3.20s (± 0.61%) 3.21s (± 0.87%) +0.01s (+ 0.41%) 3.16s 3.27s
Total Time 10.74s (± 0.39%) 10.77s (± 0.49%) +0.03s (+ 0.31%) 10.67s 10.92s
TFS - node (v12.1.0, x64)
Memory used 289,453k (± 0.02%) 289,525k (± 0.02%) +73k (+ 0.03%) 289,437k 289,602k
Parse Time 1.22s (± 0.61%) 1.21s (± 1.35%) -0.01s (- 0.74%) 1.19s 1.26s
Bind Time 0.69s (± 0.52%) 0.69s (± 0.71%) -0.00s (- 0.00%) 0.68s 0.70s
Check Time 4.92s (± 0.56%) 4.92s (± 0.34%) -0.00s (- 0.04%) 4.88s 4.95s
Emit Time 3.38s (± 0.80%) 3.38s (± 0.96%) -0.01s (- 0.18%) 3.32s 3.45s
Total Time 10.22s (± 0.36%) 10.21s (± 0.44%) -0.02s (- 0.17%) 10.12s 10.31s
material-ui - node (v12.1.0, x64)
Memory used 449,697k (± 0.07%) 449,684k (± 0.05%) -14k (- 0.00%) 448,745k 449,941k
Parse Time 1.79s (± 0.50%) 1.78s (± 0.62%) -0.00s (- 0.00%) 1.77s 1.82s
Bind Time 0.65s (± 1.06%) 0.64s (± 0.74%) -0.00s (- 0.31%) 0.63s 0.65s
Check Time 13.12s (± 0.56%) 13.06s (± 0.82%) -0.06s (- 0.45%) 12.76s 13.28s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.55s (± 0.47%) 15.49s (± 0.71%) -0.06s (- 0.38%) 15.18s 15.70s
Angular - node (v14.15.1, x64)
Memory used 330,263k (± 0.01%) 330,254k (± 0.01%) -8k (- 0.00%) 330,201k 330,312k
Parse Time 1.96s (± 0.74%) 1.95s (± 0.76%) -0.01s (- 0.61%) 1.93s 2.00s
Bind Time 0.86s (± 0.55%) 0.86s (± 0.35%) -0.00s (- 0.23%) 0.85s 0.86s
Check Time 5.35s (± 0.42%) 5.34s (± 0.44%) -0.01s (- 0.24%) 5.29s 5.39s
Emit Time 6.23s (± 0.57%) 6.19s (± 0.68%) -0.05s (- 0.74%) 6.10s 6.26s
Total Time 14.41s (± 0.31%) 14.34s (± 0.47%) -0.07s (- 0.50%) 14.19s 14.46s
Compiler-Unions - node (v14.15.1, x64)
Memory used 193,202k (± 0.02%) 192,548k (± 0.49%) -654k (- 0.34%) 189,961k 193,242k
Parse Time 0.82s (± 0.73%) 0.81s (± 0.68%) -0.00s (- 0.49%) 0.80s 0.82s
Bind Time 0.56s (± 0.53%) 0.56s (± 0.53%) 0.00s ( 0.00%) 0.55s 0.56s
Check Time 7.64s (± 0.71%) 7.65s (± 0.63%) +0.01s (+ 0.09%) 7.52s 7.75s
Emit Time 2.47s (± 0.59%) 2.45s (± 0.72%) -0.02s (- 0.81%) 2.41s 2.49s
Total Time 11.49s (± 0.56%) 11.47s (± 0.52%) -0.02s (- 0.16%) 11.31s 11.60s
Monaco - node (v14.15.1, x64)
Memory used 324,018k (± 0.01%) 323,983k (± 0.00%) -35k (- 0.01%) 323,951k 324,026k
Parse Time 1.52s (± 0.50%) 1.51s (± 0.84%) -0.02s (- 1.05%) 1.49s 1.55s
Bind Time 0.76s (± 0.44%) 0.76s (± 0.85%) -0.00s (- 0.26%) 0.75s 0.78s
Check Time 5.32s (± 0.63%) 5.32s (± 0.50%) +0.00s (+ 0.02%) 5.27s 5.40s
Emit Time 3.23s (± 0.61%) 3.22s (± 0.89%) -0.00s (- 0.03%) 3.16s 3.28s
Total Time 10.83s (± 0.44%) 10.82s (± 0.38%) -0.02s (- 0.17%) 10.74s 10.91s
TFS - node (v14.15.1, x64)
Memory used 288,366k (± 0.01%) 288,359k (± 0.00%) -7k (- 0.00%) 288,338k 288,376k
Parse Time 1.24s (± 0.90%) 1.24s (± 0.77%) -0.00s (- 0.08%) 1.22s 1.26s
Bind Time 0.74s (± 1.12%) 0.73s (± 0.55%) -0.00s (- 0.68%) 0.72s 0.74s
Check Time 4.92s (± 0.36%) 4.94s (± 0.37%) +0.02s (+ 0.30%) 4.89s 4.97s
Emit Time 3.49s (± 0.87%) 3.47s (± 0.61%) -0.02s (- 0.49%) 3.41s 3.51s
Total Time 10.38s (± 0.41%) 10.37s (± 0.27%) -0.01s (- 0.11%) 10.33s 10.44s
material-ui - node (v14.15.1, x64)
Memory used 447,957k (± 0.06%) 447,970k (± 0.06%) +13k (+ 0.00%) 446,935k 448,104k
Parse Time 1.83s (± 0.42%) 1.82s (± 0.56%) -0.01s (- 0.66%) 1.80s 1.85s
Bind Time 0.68s (± 0.85%) 0.68s (± 0.77%) -0.00s (- 0.59%) 0.67s 0.69s
Check Time 13.23s (± 0.87%) 13.22s (± 0.86%) -0.01s (- 0.04%) 12.95s 13.57s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.74s (± 0.73%) 15.72s (± 0.70%) -0.02s (- 0.13%) 15.48s 16.06s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory6 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
Benchmark Name Iterations
Current 46326 10
Baseline main 10

Developer Information:

Download Benchmark

@ahejlsberg ahejlsberg merged commit 0d2aeb7 into main Oct 13, 2021
@ahejlsberg ahejlsberg deleted the fix46183 branch October 13, 2021 14:08
This was referenced Oct 19, 2021
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weswigham weswigham weswigham approved these changes

Assignees

@ahejlsberg ahejlsberg

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Milestone
No milestone
3 participants
@ahejlsberg @typescript-bot @weswigham