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

Widen boolean literals when contextual type is full boolean type #48368

Closed
wants to merge 5 commits into from

Conversation

ahejlsberg
Copy link
Member

Fixes #48363.

Also fixes issue mentioned here because boolean literals contextually typed by boolean are now widened.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 21, 2022
@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 21, 2022

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 21, 2022

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 21, 2022

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at b54de54. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 21, 2022

Heya @ahejlsberg, I've started to run the diff-based community code test suite on this PR at b54de54. 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..48368

Metric main 48368 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 332,421k (± 0.00%) 332,455k (± 0.00%) +34k (+ 0.01%) 332,422k 332,499k
Parse Time 2.01s (± 0.38%) 2.04s (± 0.36%) +0.02s (+ 1.14%) 2.02s 2.06s
Bind Time 0.86s (± 0.43%) 0.87s (± 0.51%) +0.00s (+ 0.35%) 0.86s 0.88s
Check Time 5.58s (± 0.55%) 5.60s (± 0.45%) +0.02s (+ 0.45%) 5.53s 5.65s
Emit Time 6.28s (± 0.71%) 6.32s (± 0.80%) +0.04s (+ 0.59%) 6.21s 6.45s
Total Time 14.74s (± 0.42%) 14.82s (± 0.47%) +0.09s (+ 0.58%) 14.70s 15.01s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,805k (± 0.59%) 193,835k (± 0.60%) +1,030k (+ 0.53%) 191,837k 195,140k
Parse Time 0.85s (± 0.40%) 0.85s (± 0.56%) 0.00s ( 0.00%) 0.84s 0.86s
Bind Time 0.56s (± 0.71%) 0.56s (± 0.93%) 0.00s ( 0.00%) 0.55s 0.57s
Check Time 7.48s (± 0.74%) 7.43s (± 0.37%) -0.05s (- 0.70%) 7.36s 7.48s
Emit Time 2.51s (± 1.50%) 2.48s (± 0.54%) -0.03s (- 1.35%) 2.45s 2.51s
Total Time 11.41s (± 0.65%) 11.32s (± 0.28%) -0.09s (- 0.78%) 11.24s 11.38s
Monaco - node (v14.15.1, x64)
Memory used 325,401k (± 0.01%) 325,363k (± 0.00%) -38k (- 0.01%) 325,340k 325,393k
Parse Time 1.58s (± 0.62%) 1.58s (± 0.61%) -0.00s (- 0.19%) 1.55s 1.59s
Bind Time 0.78s (± 0.85%) 0.77s (± 0.47%) -0.01s (- 0.77%) 0.77s 0.78s
Check Time 5.49s (± 0.45%) 5.46s (± 0.40%) -0.03s (- 0.47%) 5.41s 5.51s
Emit Time 3.33s (± 0.78%) 3.29s (± 0.55%) -0.04s (- 1.11%) 3.26s 3.34s
Total Time 11.17s (± 0.37%) 11.10s (± 0.33%) -0.07s (- 0.64%) 11.01s 11.18s
TFS - node (v14.15.1, x64)
Memory used 288,859k (± 0.01%) 288,857k (± 0.01%) -2k (- 0.00%) 288,801k 288,904k
Parse Time 1.34s (± 1.22%) 1.32s (± 0.96%) -0.02s (- 1.64%) 1.29s 1.35s
Bind Time 0.73s (± 1.04%) 0.73s (± 0.71%) -0.00s (- 0.54%) 0.72s 0.74s
Check Time 5.11s (± 0.55%) 5.11s (± 0.35%) +0.00s (+ 0.04%) 5.08s 5.16s
Emit Time 3.51s (± 2.21%) 3.52s (± 1.89%) +0.01s (+ 0.26%) 3.38s 3.63s
Total Time 10.69s (± 0.86%) 10.68s (± 0.77%) -0.02s (- 0.15%) 10.49s 10.83s
material-ui - node (v14.15.1, x64)
Memory used 453,485k (± 0.08%) 453,590k (± 0.07%) +104k (+ 0.02%) 452,376k 453,798k
Parse Time 1.86s (± 0.54%) 1.85s (± 0.33%) -0.02s (- 0.86%) 1.84s 1.86s
Bind Time 0.71s (± 0.97%) 0.70s (± 1.18%) -0.00s (- 0.00%) 0.69s 0.72s
Check Time 20.97s (± 0.59%) 20.49s (± 0.84%) -0.48s (- 2.28%) 20.25s 20.93s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 23.55s (± 0.55%) 23.05s (± 0.76%) -0.50s (- 2.11%) 22.78s 23.48s
xstate - node (v14.15.1, x64)
Memory used 535,220k (± 0.00%) 535,233k (± 0.01%) +13k (+ 0.00%) 535,184k 535,298k
Parse Time 2.58s (± 0.61%) 2.57s (± 0.43%) -0.01s (- 0.35%) 2.55s 2.59s
Bind Time 1.15s (± 1.24%) 1.13s (± 0.77%) -0.02s (- 1.48%) 1.12s 1.15s
Check Time 1.51s (± 0.65%) 1.51s (± 0.73%) +0.00s (+ 0.13%) 1.49s 1.54s
Emit Time 0.07s (± 4.13%) 0.07s (± 0.00%) -0.00s (- 2.78%) 0.07s 0.07s
Total Time 5.31s (± 0.27%) 5.29s (± 0.25%) -0.03s (- 0.49%) 5.26s 5.32s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory4 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 48368 10
Baseline main 10

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

@ahejlsberg
Great news! no new errors were found between main..refs/pull/48368/merge

@ahejlsberg
Copy link
Member Author

Apart from lots of false and true types changing to boolean (as expected), there are really only two notable changes in the baselines. One is in excessPropertyCheckWithMultipleDiscriminants.ts, simplified to:

let x: { kind: false, x: number } | { kind: boolean, x: string } = { kind: false, x: 42 };

Previously this would match against the first variant and succeed. It now matches against the second variant (because the kind: false in the object literal is given type boolean as it is contextually typed by boolean). It's possible to get the old behavior by writing kind: false as const. I think this change is acceptable.

The other change is in inferFromGenericFunctionReturnTypes3.ts:

declare function foldLeft<U>(z: U, f: (acc: U, t: boolean) => U): U;
let res: boolean = foldLeft(true, (acc, t) => acc && t);  // Was error, now ok

Previously the true literal wasn't widened and therefore acc and t would have type true, which doesn't work out. We now widen and the example works as expected.

@ahejlsberg
Copy link
Member Author

@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 22, 2022

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at af5bd9e. You can monitor the build here.

@fatcerberus
Copy link

Someone brought this up in Discord:

let foo: boolean = false;

With the change in this PR, does this no longer narrow-on-assignment to false?

@ahejlsberg
Copy link
Member Author

ahejlsberg commented Mar 23, 2022

Several Definitely Typed packages are adversely affected. Closing this PR and putting up #48380 with less impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent boolean literal types
3 participants