Skip to content

Fix creation of composite union type predicates - #54169

Merged
Anders Hejlsberg (ahejlsberg) merged 2 commits into
mainfrom
fix54143
May 19, 2023
Merged

Fix creation of composite union type predicates#54169
Anders Hejlsberg (ahejlsberg) merged 2 commits into
mainfrom
fix54143

Conversation

@ahejlsberg

Copy link
Copy Markdown
Member

Fixes#54143.

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

@typescript-bot

TypeScript Bot (typescript-bot) commented May 7, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the parallelized Definitely Typed test suite on this PR at 98c0460. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented May 7, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the abridged perf test suite on this PR at 98c0460. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented May 7, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based user code test suite on this PR at 98c0460. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented May 7, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at 98c0460. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented May 7, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based top-repos suite on this PR at 98c0460. You can monitor the build here.

Update: The results are in!

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg) Here are the results of running the user test suite comparing main and refs/pull/54169/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg) Here are the results of running the top-repos suite comparing main and refs/pull/54169/merge:

Something interesting changed - please have a look.

Details

microsoft/vscode

4 of 53 projects failed to build with the old tsc and were ignored

src/tsconfig.tsec.json

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Anders Hejlsberg (@ahejlsberg), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

The new error in the vscode project is precisely an example of relying on the issue we're fixing: A union of two function types where one is a type predicate and the other is just a boolean returning function. It definitely isn't sound to rely on the boolean function proving or disproving anything, so the new error is correct.

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..54169

Metricmain54169DeltaBestWorstp-value
Angular - node (v16.17.1, x64)
Memory used365,352k (± 0.01%)365,349k (± 0.00%)~365,329k365,379kp=0.810 n=6
Parse Time3.55s (± 0.48%)3.55s (± 0.49%)~3.54s3.58sp=0.737 n=6
Bind Time1.17s (± 0.54%)1.17s (± 0.54%)~1.16s1.18sp=1.000 n=6
Check Time9.56s (± 0.37%)9.58s (± 0.40%)~9.54s9.62sp=0.418 n=6
Emit Time7.91s (± 0.34%)7.94s (± 0.38%)~7.91s7.98sp=0.077 n=6
Total Time22.19s (± 0.26%)22.25s (± 0.28%)~22.19s22.35sp=0.227 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used192,810k (± 0.02%)192,710k (± 0.08%)~192,416k192,853kp=0.093 n=6
Parse Time1.61s (± 0.96%)1.59s (± 1.75%)~1.54s1.62sp=0.220 n=6
Bind Time0.82s (± 0.63%)0.83s (± 0.91%)~0.82s0.84sp=0.247 n=6
Check Time10.29s (± 0.43%)10.28s (± 0.39%)~10.24s10.33sp=0.748 n=6
Emit Time3.02s (± 0.65%)3.02s (± 0.92%)~3.00s3.07sp=0.803 n=6
Total Time15.74s (± 0.21%)15.72s (± 0.46%)~15.61s15.82sp=0.809 n=6
Monaco - node (v16.17.1, x64)
Memory used345,862k (± 0.00%)345,863k (± 0.01%)~345,839k345,893kp=0.810 n=6
Parse Time2.72s (± 0.44%)2.73s (± 0.54%)~2.71s2.75sp=0.623 n=6
Bind Time1.08s (± 0.77%)1.08s (± 0.70%)~1.07s1.09sp=0.432 n=6
Check Time7.88s (± 0.72%)7.87s (± 0.42%)~7.82s7.91sp=0.808 n=6
Emit Time4.47s (± 0.59%)4.45s (± 0.49%)~4.41s4.47sp=0.090 n=6
Total Time16.15s (± 0.38%)16.13s (± 0.25%)~16.09s16.19sp=0.421 n=6
TFS - node (v16.17.1, x64)
Memory used300,105k (± 0.01%)300,100k (± 0.01%)~300,078k300,126kp=0.689 n=6
Parse Time2.16s (± 0.64%)2.16s (± 0.72%)~2.14s2.18sp=0.676 n=6
Bind Time1.23s (± 0.66%)1.24s (± 0.79%)~1.23s1.25sp=0.498 n=6
Check Time7.27s (± 0.45%)7.29s (± 0.48%)~7.23s7.33sp=0.421 n=6
Emit Time4.34s (± 0.89%)4.38s (± 0.91%)~4.33s4.42sp=0.334 n=6
Total Time15.00s (± 0.51%)15.06s (± 0.34%)~14.97s15.11sp=0.196 n=6
material-ui - node (v16.17.1, x64)
Memory used481,623k (± 0.01%)481,595k (± 0.01%)~481,534k481,636kp=0.336 n=6
Parse Time3.24s (± 0.28%)3.23s (± 0.47%)~3.22s3.26sp=0.458 n=6
Bind Time0.94s (± 0.80%)0.94s (± 0.55%)~0.93s0.94sp=0.784 n=6
Check Time17.86s (± 0.47%)17.85s (± 0.53%)~17.71s17.96sp=1.000 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time22.04s (± 0.40%)22.02s (± 0.47%)~21.86s22.12sp=0.872 n=6
xstate - node (v16.17.1, x64)
Memory used560,369k (± 0.02%)560,377k (± 0.02%)~560,279k560,519kp=0.810 n=6
Parse Time4.00s (± 0.31%)4.00s (± 0.16%)~3.99s4.01sp=0.591 n=6
Bind Time1.76s (± 0.51%)1.76s (± 0.66%)~1.75s1.78sp=0.933 n=6
Check Time3.06s (± 0.67%)3.06s (± 0.34%)~3.04s3.07sp=0.802 n=6
Emit Time0.09s (± 0.00%)0.09s (± 4.45%)~0.09s0.10sp=0.405 n=6
Total Time8.92s (± 0.31%)8.91s (± 0.27%)~8.88s8.94sp=0.624 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-148-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
BenchmarkNameIterations
Current541696
Baselinemain6

Developer Information:

Download Benchmark

@DanielRosenwasser

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) cherry-pick this to release-5.1

@typescript-bot

TypeScript Bot (typescript-bot) commented May 26, 2023

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the task to cherry-pick this into release-5.1 on this PR at 98c0460. You can monitor the build here.

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Daniel Rosenwasser (@DanielRosenwasser), I've opened #54412 for you.

TypeScript Bot (typescript-bot) pushed a commit to typescript-bot/TypeScript that referenced this pull request May 26, 2023
Component commits:
a54709c Revise creation of composite union/intersection type predicates
98c0460 Add tests
Daniel Rosenwasser (DanielRosenwasser) pushed a commit that referenced this pull request May 29, 2023
…e-5.1 (#54412)
Co-authored-by: Anders Hejlsberg <andersh@microsoft.com>
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: TeamFor Milestone BugPRs that fix a bug with a specific milestone

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Type guard incorrectly erases union type

5 participants

@ahejlsberg@typescript-bot@DanielRosenwasser@jakebailey@sandersn