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

Filter return type inferences by constraint applicability #58910

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Filter return type inferences by constraint applicability #58910

wants to merge 2 commits into from

Conversation

Andarist
Copy link
Contributor

fixes #50719
fixes #41461

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Jun 18, 2024
@Andarist Andarist force-pushed the fix/filter-out-return-type-inference-by-constraint-applicability branch from 34d5fbe to b9936e3 Compare June 18, 2024 08:05
weswigham
weswigham reviewed Jun 18, 2024
View reviewed changes
src/compiler/checker.ts
}
else {
if (inference.priority! & InferencePriority.ReturnType) {
inference.inferredType = filterContextualInferredType(inferredType, instantiatedConstraint) ?? (fallbackType && filterContextualInferredType(fallbackType, instantiatedConstraint)) ?? instantiatedConstraint;
Copy link
Member

Choose a reason for hiding this comment

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

We only need to bother with this filtering if the constraint check for the whole union fails, right? (Since if it passes, no filtering will occur, as every union member must also pass.) This can probably be inside the comparison branch below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s right but it felt cleaner to have this in a separate branch. Shouldnt filterType already optimize for array/type allocations?

@Andarist Andarist requested a review from weswigham June 19, 2024 10:17
@jakebailey
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 19, 2024 •
edited
Loading

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

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

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

There were interesting changes:

Branch only errors:

Package: jest
Error:

Error: 
/mnt/vss/_work/1/DefinitelyTyped/types/jest/jest-tests.ts
  762:1  error  TypeScript@local compile error: 
Type 'SpyInstance<void, [number | void], any>' is not assignable to type 'SpiedSetter<number>'.
  Types of property 'mock' are incompatible.
    Type 'MockContext<void, [number | void], any>' is not assignable to type 'MockContext<void, [number], any>'.
      Type '[number | void]' is not assignable to type '[number]'  @definitelytyped/expect

✖ 1 problem (1 error, 0 warnings)

    at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/dist/index.js:194:28)
    at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/dist/index.js:186:20)

You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/58910/merge:

Something interesting changed - please have a look.

Details

webpack

tsconfig.types.json

@typescript-bot
Copy link
Collaborator

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

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,243 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 193,782k (± 0.94%) 192,726k (± 0.75%) ~ 192,087k 195,687k p=0.471 n=6
Parse Time 1.30s (± 0.93%) 1.29s (± 2.10%) ~ 1.25s 1.31s p=0.422 n=6
Bind Time 0.71s 0.71s ~ ~ ~ p=1.000 n=6
Check Time 9.40s (± 0.44%) 9.43s (± 0.28%) ~ 9.40s 9.46s p=0.258 n=6
Emit Time 2.74s (± 0.74%) 2.76s (± 0.50%) ~ 2.74s 2.78s p=0.105 n=6
Total Time 14.16s (± 0.17%) 14.19s (± 0.31%) ~ 14.12s 14.24s p=0.148 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,114 944,138 +24 (+ 0.00%) ~ ~ p=0.001 n=6
Types 407,051 407,082 +31 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 1,218,315k (± 0.00%) 1,218,358k (± 0.00%) ~ 1,218,299k 1,218,401k p=0.092 n=6
Parse Time 6.64s (± 0.57%) 6.68s (± 0.79%) ~ 6.64s 6.78s p=0.086 n=6
Bind Time 1.88s (± 1.38%) 1.87s (± 1.17%) ~ 1.85s 1.91s p=0.618 n=6
Check Time 30.59s (± 0.26%) 30.68s (± 0.42%) ~ 30.50s 30.88s p=0.199 n=6
Emit Time 13.56s (± 0.36%) 13.55s (± 0.27%) ~ 13.48s 13.58s p=0.627 n=6
Total Time 52.67s (± 0.15%) 52.78s (± 0.24%) ~ 52.59s 52.95s p=0.149 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 4 🔻+4 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 2,133,420 2,131,417 -2,003 (- 0.09%) ~ ~ p=0.001 n=6
Types 926,456 925,839 -617 (- 0.07%) ~ ~ p=0.001 n=6
Memory used 2,115,464k (± 0.00%) 2,114,668k (± 0.00%) -796k (- 0.04%) 2,114,572k 2,114,839k p=0.005 n=6
Parse Time 6.60s (± 0.17%) 6.60s (± 0.25%) ~ 6.58s 6.62s p=0.566 n=6
Bind Time 2.33s (± 0.22%) 2.34s (± 0.96%) ~ 2.30s 2.37s p=0.437 n=6
Check Time 70.66s (± 0.48%) 70.45s (± 1.32%) ~ 68.63s 71.17s p=0.873 n=6
Emit Time 0.14s (± 3.77%) 0.13s (± 3.87%) ~ 0.13s 0.14s p=0.311 n=6
Total Time 79.73s (± 0.42%) 79.52s (± 1.15%) ~ 77.73s 80.25s p=0.873 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,230,762 1,230,884 +122 (+ 0.01%) ~ ~ p=0.001 n=6
Types 261,187 261,272 +85 (+ 0.03%) ~ ~ p=0.001 n=6
Memory used 2,345,612k (± 0.04%) 2,346,043k (± 0.04%) ~ 2,345,013k 2,347,238k p=0.575 n=6
Parse Time 4.99s (± 1.28%) 5.00s (± 0.95%) ~ 4.93s 5.06s p=0.748 n=6
Bind Time 1.90s (± 0.87%) 1.90s (± 0.70%) ~ 1.89s 1.92s p=0.616 n=6
Check Time 33.74s (± 0.26%) 33.80s (± 0.18%) ~ 33.71s 33.86s p=0.170 n=6
Emit Time 2.71s (± 2.78%) 2.69s (± 2.56%) ~ 2.61s 2.81s p=0.298 n=6
Total Time 43.36s (± 0.38%) 43.41s (± 0.25%) ~ 43.25s 43.54s p=0.521 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,230,762 1,230,884 +122 (+ 0.01%) ~ ~ p=0.001 n=6
Types 261,187 261,272 +85 (+ 0.03%) ~ ~ p=0.001 n=6
Memory used 2,423,016k (± 0.03%) 2,422,796k (± 0.02%) ~ 2,421,965k 2,423,472k p=0.378 n=6
Parse Time 7.71s (± 0.56%) 7.71s (± 0.60%) ~ 7.67s 7.79s p=0.936 n=6
Bind Time 2.50s (± 0.86%) 2.51s (± 0.53%) ~ 2.49s 2.53s p=0.332 n=6
Check Time 49.53s (± 0.40%) 49.53s (± 0.25%) ~ 49.41s 49.74s p=0.936 n=6
Emit Time 3.86s (± 2.06%) 3.90s (± 2.78%) ~ 3.74s 4.03s p=0.689 n=6
Total Time 63.63s (± 0.30%) 63.66s (± 0.20%) ~ 63.53s 63.88s p=0.810 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,573 258,690 +117 (+ 0.05%) ~ ~ p=0.001 n=6
Types 104,819 104,890 +71 (+ 0.07%) ~ ~ p=0.001 n=6
Memory used 428,178k (± 0.01%) 428,374k (± 0.00%) +197k (+ 0.05%) 428,341k 428,397k p=0.005 n=6
Parse Time 3.31s (± 0.56%) 3.33s (± 0.58%) ~ 3.31s 3.36s p=0.120 n=6
Bind Time 1.32s (± 1.58%) 1.32s (± 1.41%) ~ 1.29s 1.34s p=1.000 n=6
Check Time 17.75s (± 0.20%) 17.77s (± 0.56%) ~ 17.67s 17.89s p=0.746 n=6
Emit Time 1.37s (± 1.81%) 1.37s (± 0.85%) ~ 1.35s 1.38s p=0.681 n=6
Total Time 23.73s (± 0.26%) 23.78s (± 0.38%) ~ 23.67s 23.89s p=0.467 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,675 +110 (+ 0.05%) ~ ~ p=0.001 n=6
Types 93,734 93,805 +71 (+ 0.08%) ~ ~ p=0.001 n=6
Memory used 369,473k (± 0.03%) 369,626k (± 0.04%) ~ 369,439k 369,828k p=0.066 n=6
Parse Time 2.78s (± 0.95%) 2.78s (± 1.00%) ~ 2.74s 2.81s p=0.871 n=6
Bind Time 1.58s (± 0.53%) 1.58s (± 1.17%) ~ 1.57s 1.62s p=0.325 n=6
Check Time 15.45s (± 0.29%) 15.47s (± 0.20%) ~ 15.42s 15.51s p=0.520 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 19.82s (± 0.16%) 19.83s (± 0.29%) ~ 19.72s 19.88s p=0.145 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=NaN n=0
Symbols 0 0 ~ ~ ~ p=NaN n=0
Types 0 0 ~ ~ ~ p=NaN n=0
Memory used 0k 0k ~ ~ ~ p=NaN n=0
Parse Time 0s 0s ~ ~ ~ p=NaN n=0
Bind Time 0s 0s ~ ~ ~ p=NaN n=0
Check Time 0s 0s ~ ~ ~ p=NaN n=0
Emit Time 0s 0s ~ ~ ~ p=NaN n=0
Total Time 0s 0s ~ ~ ~ p=NaN n=0
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,116 267,119 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 108,756 108,757 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 411,518k (± 0.01%) 411,609k (± 0.03%) ~ 411,523k 411,825k p=0.093 n=6
Parse Time 3.82s (± 0.33%) 3.81s (± 0.55%) ~ 3.79s 3.84s p=0.325 n=6
Bind Time 1.70s (± 0.69%) 1.69s (± 0.53%) ~ 1.68s 1.70s p=0.270 n=6
Check Time 16.74s (± 0.47%) 16.75s (± 0.57%) ~ 16.62s 16.88s p=0.873 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.26s (± 0.32%) 22.25s (± 0.47%) ~ 22.10s 22.40s p=1.000 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 523,765 523,005 -760 (- 0.15%) ~ ~ p=0.001 n=6
Types 178,055 178,065 +10 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 461,594k (± 0.07%) 461,628k (± 0.06%) ~ 461,208k 461,961k p=0.575 n=6
Parse Time 3.94s (± 0.27%) 3.93s (± 0.65%) ~ 3.90s 3.96s p=0.466 n=6
Bind Time 1.47s (± 0.71%) 1.47s (± 0.71%) ~ 1.46s 1.49s p=1.000 n=6
Check Time 22.20s (± 0.36%) 22.26s (± 0.53%) ~ 22.10s 22.44s p=0.295 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 27.61s (± 0.26%) 27.67s (± 0.47%) ~ 27.49s 27.84s p=0.574 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)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/58910/merge:

Something interesting changed - please have a look.

Details

alan2207/bulletproof-react

tsconfig.json

apollographql/apollo-client

1 of 12 projects failed to build with the old tsc and were ignored

tsconfig.json

src/tsconfig.json

invoke-ai/InvokeAI

invokeai/frontend/web/tsconfig.json

mattermost/mattermost

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

webapp/channels/tsconfig.json

@typescript-bot
Copy link
Collaborator

@jakebailey Here are some more interesting changes from running the top 400 repos suite

Details

microsoft/vscode

5 of 54 projects failed to build with the old tsc and were ignored

src/tsconfig.tsec.json

@typescript-bot
Copy link
Collaborator

@jakebailey Here are some more interesting changes from running the top 400 repos suite

Details

pixijs/pixijs

tsconfig.types.json

tsconfig.json

recharts/recharts

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

storybook/tsconfig.json

tailwindlabs/headlessui

1 of 5 projects failed to build with the old tsc and were ignored

packages/@headlessui-react/tsconfig.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabritto gabritto Awaiting requested review from gabritto

@weswigham weswigham Awaiting requested review from weswigham

At least 1 approving review is required to merge this pull request.

Assignees

@weswigham weswigham

Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
Status: Waiting on reviewers
Milestone
No milestone
4 participants
@Andarist @jakebailey @typescript-bot @weswigham