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

Fix nullability intersections in CFA and relations #57724

Merged
merged 6 commits into from
Mar 11, 2024
Merged

Fix nullability intersections in CFA and relations #57724

merged 6 commits into from
Mar 11, 2024

Conversation

ahejlsberg
Copy link
Member

Fixes #57693.

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

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 11, 2024 •
edited
Loading

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

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

@Andarist Andarist mentioned this pull request Mar 11, 2024
Closed
@typescript-bot
Copy link
Collaborator

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

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/57724/merge:

There were infrastructure failures potentially unrelated to your change:

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

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot
Copy link
Collaborator

@ahejlsberg
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
Angular - node (v18.15.0, x64)
Memory used 295,545k (± 0.01%) 295,559k (± 0.01%) ~ 295,534k 295,594k p=0.423 n=6
Parse Time 2.66s (± 0.15%) 2.67s (± 0.28%) ~ 2.66s 2.68s p=0.100 n=6
Bind Time 0.83s (± 0.62%) 0.83s (± 0.00%) ~ 0.83s 0.83s p=0.174 n=6
Check Time 8.20s (± 0.43%) 8.23s (± 0.21%) ~ 8.21s 8.25s p=0.134 n=6
Emit Time 7.13s (± 0.19%) 7.13s (± 0.15%) ~ 7.11s 7.14s p=0.867 n=6
Total Time 18.81s (± 0.22%) 18.85s (± 0.16%) ~ 18.82s 18.89s p=0.294 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 193,248k (± 1.01%) 192,651k (± 0.92%) ~ 191,432k 194,953k p=0.471 n=6
Parse Time 1.36s (± 0.40%) 1.36s (± 0.86%) ~ 1.34s 1.37s p=0.498 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.37s (± 0.28%) 9.31s (± 0.29%) -0.06s (- 0.62%) 9.27s 9.35s p=0.012 n=6
Emit Time 2.61s (± 0.42%) 2.60s (± 0.86%) ~ 2.58s 2.64s p=0.318 n=6
Total Time 14.05s (± 0.23%) 13.99s (± 0.31%) -0.07s (- 0.47%) 13.91s 14.04s p=0.020 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,321k (± 0.00%) 347,316k (± 0.01%) ~ 347,291k 347,340k p=0.936 n=6
Parse Time 2.48s (± 0.44%) 2.48s (± 0.66%) ~ 2.45s 2.49s p=0.933 n=6
Bind Time 0.93s (± 0.44%) 0.92s (± 0.56%) -0.01s (- 0.89%) 0.92s 0.93s p=0.022 n=6
Check Time 6.95s (± 0.20%) 6.96s (± 0.34%) ~ 6.93s 6.99s p=0.570 n=6
Emit Time 4.06s (± 0.29%) 4.06s (± 0.38%) ~ 4.05s 4.09s p=0.457 n=6
Total Time 14.42s (± 0.17%) 14.43s (± 0.25%) ~ 14.39s 14.49s p=0.936 n=6
TFS - node (v18.15.0, x64)
Memory used 302,753k (± 0.00%) 302,794k (± 0.01%) +41k (+ 0.01%) 302,765k 302,842k p=0.006 n=6
Parse Time 2.01s (± 0.58%) 1.99s (± 1.48%) ~ 1.96s 2.03s p=0.366 n=6
Bind Time 1.00s (± 1.03%) 1.01s (± 1.04%) ~ 0.99s 1.02s p=0.801 n=6
Check Time 6.34s (± 0.22%) 6.34s (± 0.44%) ~ 6.31s 6.39s p=0.371 n=6
Emit Time 3.59s (± 0.27%) 3.60s (± 0.29%) ~ 3.58s 3.61s p=0.547 n=6
Total Time 12.95s (± 0.12%) 12.94s (± 0.22%) ~ 12.89s 12.96s p=0.681 n=6
material-ui - node (v18.15.0, x64)
Memory used 511,231k (± 0.00%) 511,239k (± 0.00%) ~ 511,224k 511,255k p=0.378 n=6
Parse Time 2.65s (± 0.62%) 2.66s (± 0.51%) ~ 2.63s 2.67s p=1.000 n=6
Bind Time 0.98s (± 0.64%) 0.99s (± 0.83%) ~ 0.98s 1.00s p=0.177 n=6
Check Time 17.29s (± 0.36%) 17.31s (± 0.35%) ~ 17.22s 17.37s p=0.627 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.93s (± 0.35%) 20.95s (± 0.33%) ~ 20.86s 21.02s p=0.419 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,789,710k (± 0.00%) 1,789,729k (± 0.00%) ~ 1,789,686k 1,789,781k p=0.936 n=6
Parse Time 6.62s (± 0.48%) 6.63s (± 0.51%) ~ 6.60s 6.69s p=0.808 n=6
Bind Time 2.40s (± 0.35%) 2.38s (± 0.51%) ~ 2.36s 2.39s p=0.062 n=6
Check Time 58.81s (± 0.22%) 59.07s (± 0.28%) +0.26s (+ 0.45%) 58.87s 59.27s p=0.031 n=6
Emit Time 0.17s (± 3.32%) 0.17s (± 3.10%) ~ 0.16s 0.17s p=0.640 n=6
Total Time 67.99s (± 0.20%) 68.25s (± 0.23%) +0.26s (+ 0.38%) 68.03s 68.45s p=0.031 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,394,063k (± 0.02%) 2,394,116k (± 0.03%) ~ 2,393,155k 2,394,884k p=1.000 n=6
Parse Time 5.04s (± 0.88%) 5.07s (± 0.46%) ~ 5.04s 5.10s p=0.230 n=6
Bind Time 1.89s (± 0.86%) 1.89s (± 1.51%) ~ 1.85s 1.92s p=0.745 n=6
Check Time 33.66s (± 0.51%) 33.71s (± 0.31%) ~ 33.58s 33.83s p=0.872 n=6
Emit Time 2.69s (± 1.82%) 2.65s (± 0.81%) ~ 2.63s 2.69s p=0.128 n=6
Total Time 43.29s (± 0.52%) 43.33s (± 0.26%) ~ 43.21s 43.48s p=1.000 n=6
self-compiler - node (v18.15.0, x64)
Memory used 414,469k (± 0.01%) 414,563k (± 0.01%) +94k (+ 0.02%) 414,478k 414,665k p=0.031 n=6
Parse Time 2.80s (± 0.71%) 2.79s (± 1.00%) ~ 2.76s 2.83s p=0.627 n=6
Bind Time 1.07s (± 0.48%) 1.07s (± 0.51%) ~ 1.06s 1.07s p=0.640 n=6
Check Time 15.21s (± 0.39%) 15.23s (± 0.40%) ~ 15.16s 15.32s p=0.627 n=6
Emit Time 1.10s (± 1.56%) 1.11s (± 0.76%) ~ 1.09s 1.11s p=0.933 n=6
Total Time 20.18s (± 0.28%) 20.19s (± 0.23%) ~ 20.16s 20.26s p=0.572 n=6
vscode - node (v18.15.0, x64)
Memory used 2,866,869k (± 0.00%) 2,866,894k (± 0.00%) ~ 2,866,799k 2,866,939k p=0.521 n=6
Parse Time 10.76s (± 0.36%) 10.78s (± 0.35%) ~ 10.73s 10.83s p=0.469 n=6
Bind Time 3.44s (± 0.22%) 3.44s (± 0.30%) ~ 3.43s 3.46s p=0.931 n=6
Check Time 60.97s (± 0.41%) 60.97s (± 0.19%) ~ 60.80s 61.14s p=0.936 n=6
Emit Time 16.26s (± 0.60%) 16.27s (± 0.44%) ~ 16.19s 16.39s p=0.574 n=6
Total Time 91.43s (± 0.26%) 91.46s (± 0.11%) ~ 91.26s 91.56s p=0.873 n=6
webpack - node (v18.15.0, x64)
Memory used 399,637k (± 0.00%) 399,613k (± 0.02%) ~ 399,504k 399,679k p=0.521 n=6
Parse Time 3.23s (± 0.37%) 3.22s (± 0.50%) ~ 3.21s 3.25s p=0.165 n=6
Bind Time 1.42s (± 1.07%) 1.41s (± 1.83%) ~ 1.37s 1.43s p=0.742 n=6
Check Time 14.05s (± 0.35%) 14.05s (± 0.28%) ~ 13.99s 14.10s p=0.808 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 18.70s (± 0.19%) 18.68s (± 0.33%) ~ 18.60s 18.78s p=0.377 n=6
xstate - node (v18.15.0, x64)
Memory used 513,142k (± 0.01%) 513,151k (± 0.01%) ~ 513,105k 513,224k p=0.936 n=6
Parse Time 3.28s (± 0.23%) 3.28s (± 0.31%) ~ 3.26s 3.29s p=0.931 n=6
Bind Time 1.54s (± 0.49%) 1.54s (± 0.41%) ~ 1.53s 1.55s p=0.718 n=6
Check Time 2.85s (± 0.52%) 2.86s (± 0.48%) ~ 2.84s 2.88s p=0.290 n=6
Emit Time 0.08s (± 4.99%) 0.08s (± 0.00%) ~ 0.08s 0.08s p=0.405 n=6
Total Time 7.75s (± 0.25%) 7.76s (± 0.39%) ~ 7.72s 7.81s p=0.747 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/57724/merge:

Everything looks good!

RyanCavanaugh
RyanCavanaugh reviewed Mar 11, 2024
View reviewed changes
src/compiler/checker.ts Outdated
Comment on lines 21194 to 21196
hasNullableOrEmpty ||= !!(t.flags & TypeFlags.Nullable) || isEmptyAnonymousObjectType(t);
}
return hasInstantiable && hasNullableOrEmpty;
Copy link
Member

Choose a reason for hiding this comment

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

Potential to bail early from this loop

Suggested change
hasNullableOrEmpty ||= !!(t.flags & TypeFlags.Nullable) || isEmptyAnonymousObjectType(t);
}
return hasInstantiable && hasNullableOrEmpty;
hasNullableOrEmpty ||= !!(t.flags & TypeFlags.Nullable) || isEmptyAnonymousObjectType(t);
if (hasInstantiable && hasNullableOrEmpty) return true;
}
return false;

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, why not.

@ahejlsberg ahejlsberg merged commit ef6a4ab into main Mar 11, 2024
19 checks passed
@ahejlsberg ahejlsberg deleted the fix57693 branch March 11, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RyanCavanaugh RyanCavanaugh RyanCavanaugh 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
Development

Successfully merging this pull request may close these issues.

null gets accidentally eliminated when narrowing by undefined's equality
3 participants
@ahejlsberg @typescript-bot @RyanCavanaugh