Skip to content

allow type narrowing with NonNullExpression #41075

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
Merged
merged 1 commit into from
Oct 28, 2020
Merged

allow type narrowing with NonNullExpression #41075

merged 1 commit into from
Oct 28, 2020

Conversation

uhyo
Copy link
Contributor

@uhyo uhyo commented Oct 13, 2020

Happy hacktoberfest! 🎃🥳

Fixes #36958.

Example

Code

const m = ''.match('');
m! && m[0];
m?.[0]! && m[0];

Current Behavior (TS 4.1 Beta)

src/36958.ts:2:7 - error TS2531: Object is possibly 'null'.

2 m! && m[0];
        ~

src/36958.ts:3:12 - error TS2531: Object is possibly 'null'.

3 m?.[0]! && m[0];
             ~


Found 2 errors.

New Behavior

(no errors)

Details & Discussion

This PR enables type narrowing when condition is a NonNullExpression.

The original issue includes following example, but this PR does not fix that case.

m?.[0].length! > 0 && m[0];

To fix this case, we have to take expr > 0 and similar expressions into consideration, which I think is too complex compared to the outcome. How do you think?

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Oct 13, 2020
@weswigham
Copy link
Member

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 27, 2020 •
edited
Loading

Heya @weswigham, I've started to run the perf test suite on this PR at 58781b0. You can monitor the build here.

Update: The results are in!

weswigham
weswigham approved these changes Oct 27, 2020
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.

Assuming the perf tests come back OK (always a good idea to check with control flow changes), this looks great to me.

@weswigham
Copy link
Member

@sandersn @ahejlsberg should this be 4.1 or 4.2 bound?

@sandersn
Copy link
Member

I'd lean 4.2, but it's a small change so if the perf results come back clean, 4.1 would be OK too.

@ahejlsberg
Copy link
Member

If perf looks good I'm fine with having this in 4.1.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..41075

Metric master 41075 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 350,582k (± 0.02%) 350,589k (± 0.02%) +7k (+ 0.00%) 350,422k 350,739k
Parse Time 2.07s (± 0.39%) 2.08s (± 0.64%) +0.01s (+ 0.34%) 2.04s 2.10s
Bind Time 0.84s (± 0.83%) 0.84s (± 0.59%) +0.00s (+ 0.48%) 0.84s 0.86s
Check Time 4.97s (± 0.47%) 4.98s (± 0.33%) +0.00s (+ 0.08%) 4.95s 5.03s
Emit Time 5.30s (± 0.61%) 5.31s (± 0.65%) +0.01s (+ 0.13%) 5.26s 5.40s
Total Time 13.19s (± 0.32%) 13.21s (± 0.26%) +0.02s (+ 0.17%) 13.15s 13.30s
Monaco - node (v10.16.3, x64)
Memory used 354,616k (± 0.02%) 354,630k (± 0.02%) +14k (+ 0.00%) 354,490k 354,755k
Parse Time 1.60s (± 0.51%) 1.60s (± 0.19%) -0.00s (- 0.31%) 1.59s 1.60s
Bind Time 0.72s (± 0.72%) 0.72s (± 0.62%) -0.00s (- 0.42%) 0.71s 0.73s
Check Time 5.13s (± 0.65%) 5.10s (± 0.44%) -0.03s (- 0.58%) 5.05s 5.16s
Emit Time 2.82s (± 0.79%) 2.80s (± 0.77%) -0.02s (- 0.57%) 2.75s 2.86s
Total Time 10.27s (± 0.43%) 10.21s (± 0.37%) -0.05s (- 0.51%) 10.14s 10.28s
TFS - node (v10.16.3, x64)
Memory used 307,880k (± 0.02%) 307,845k (± 0.02%) -35k (- 0.01%) 307,733k 308,004k
Parse Time 1.24s (± 0.54%) 1.24s (± 0.54%) -0.00s (- 0.24%) 1.22s 1.25s
Bind Time 0.68s (± 0.88%) 0.67s (± 0.54%) -0.00s (- 0.30%) 0.67s 0.68s
Check Time 4.58s (± 0.47%) 4.56s (± 0.33%) -0.03s (- 0.61%) 4.52s 4.59s
Emit Time 2.92s (± 0.69%) 2.93s (± 0.85%) +0.01s (+ 0.41%) 2.87s 2.96s
Total Time 9.42s (± 0.35%) 9.40s (± 0.30%) -0.02s (- 0.22%) 9.33s 9.45s
material-ui - node (v10.16.3, x64)
Memory used 489,312k (± 0.01%) 489,331k (± 0.02%) +19k (+ 0.00%) 489,161k 489,527k
Parse Time 2.07s (± 0.46%) 2.06s (± 0.44%) -0.01s (- 0.34%) 2.04s 2.08s
Bind Time 0.65s (± 0.72%) 0.65s (± 0.68%) +0.00s (+ 0.31%) 0.64s 0.66s
Check Time 13.51s (± 0.77%) 13.48s (± 0.45%) -0.02s (- 0.17%) 13.35s 13.65s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.22s (± 0.68%) 16.20s (± 0.39%) -0.02s (- 0.15%) 16.04s 16.37s
Angular - node (v12.1.0, x64)
Memory used 327,745k (± 0.03%) 327,770k (± 0.02%) +26k (+ 0.01%) 327,658k 327,946k
Parse Time 2.06s (± 0.87%) 2.08s (± 0.70%) +0.02s (+ 1.22%) 2.05s 2.12s
Bind Time 0.81s (± 0.69%) 0.81s (± 0.92%) +0.01s (+ 0.62%) 0.80s 0.83s
Check Time 4.90s (± 0.31%) 4.94s (± 0.92%) +0.04s (+ 0.76%) 4.84s 5.05s
Emit Time 5.48s (± 0.57%) 5.53s (± 0.54%) +0.05s (+ 0.88%) 5.46s 5.59s
Total Time 13.24s (± 0.29%) 13.36s (± 0.54%) +0.11s (+ 0.84%) 13.20s 13.46s
Monaco - node (v12.1.0, x64)
Memory used 336,763k (± 0.01%) 336,779k (± 0.03%) +15k (+ 0.00%) 336,624k 337,086k
Parse Time 1.58s (± 0.81%) 1.59s (± 0.82%) +0.01s (+ 0.38%) 1.55s 1.61s
Bind Time 0.70s (± 0.74%) 0.70s (± 0.95%) +0.00s (+ 0.57%) 0.69s 0.72s
Check Time 4.92s (± 0.35%) 4.91s (± 0.66%) -0.01s (- 0.22%) 4.84s 4.97s
Emit Time 2.87s (± 0.93%) 2.88s (± 0.92%) +0.00s (+ 0.03%) 2.82s 2.94s
Total Time 10.07s (± 0.34%) 10.08s (± 0.50%) +0.00s (+ 0.03%) 9.98s 10.17s
TFS - node (v12.1.0, x64)
Memory used 292,055k (± 0.02%) 292,057k (± 0.01%) +1k (+ 0.00%) 291,971k 292,161k
Parse Time 1.25s (± 0.56%) 1.25s (± 0.72%) -0.00s (- 0.40%) 1.23s 1.27s
Bind Time 0.65s (± 1.03%) 0.64s (± 0.53%) -0.00s (- 0.62%) 0.64s 0.65s
Check Time 4.51s (± 0.59%) 4.49s (± 0.35%) -0.02s (- 0.40%) 4.46s 4.53s
Emit Time 2.95s (± 0.57%) 2.96s (± 0.81%) +0.01s (+ 0.31%) 2.90s 3.02s
Total Time 9.35s (± 0.34%) 9.34s (± 0.40%) -0.02s (- 0.16%) 9.26s 9.45s
material-ui - node (v12.1.0, x64)
Memory used 467,316k (± 0.01%) 467,269k (± 0.05%) -48k (- 0.01%) 466,279k 467,525k
Parse Time 2.09s (± 0.42%) 2.08s (± 0.63%) -0.01s (- 0.38%) 2.06s 2.11s
Bind Time 0.64s (± 0.69%) 0.65s (± 0.58%) +0.00s (+ 0.47%) 0.64s 0.65s
Check Time 11.99s (± 0.48%) 12.05s (± 0.62%) +0.06s (+ 0.50%) 11.93s 12.24s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.72s (± 0.39%) 14.77s (± 0.46%) +0.05s (+ 0.35%) 14.67s 14.94s
Angular - node (v8.9.0, x64)
Memory used 353,022k (± 0.02%) 352,946k (± 0.02%) -76k (- 0.02%) 352,786k 353,098k
Parse Time 2.65s (± 0.38%) 2.64s (± 0.20%) -0.01s (- 0.26%) 2.63s 2.65s
Bind Time 0.88s (± 0.80%) 0.88s (± 0.75%) +0.00s (+ 0.11%) 0.87s 0.90s
Check Time 5.60s (± 0.51%) 5.61s (± 0.44%) +0.01s (+ 0.14%) 5.55s 5.67s
Emit Time 6.36s (± 0.79%) 6.41s (± 0.91%) +0.05s (+ 0.82%) 6.26s 6.54s
Total Time 15.49s (± 0.46%) 15.54s (± 0.42%) +0.05s (+ 0.33%) 15.35s 15.69s
Monaco - node (v8.9.0, x64)
Memory used 358,439k (± 0.02%) 358,435k (± 0.01%) -4k (- 0.00%) 358,314k 358,580k
Parse Time 1.93s (± 0.29%) 1.92s (± 0.31%) -0.00s (- 0.21%) 1.91s 1.94s
Bind Time 0.90s (± 1.15%) 0.90s (± 1.04%) -0.00s (- 0.22%) 0.88s 0.92s
Check Time 5.66s (± 0.73%) 5.63s (± 0.66%) -0.03s (- 0.55%) 5.56s 5.71s
Emit Time 3.36s (± 1.46%) 3.36s (± 1.20%) +0.00s (+ 0.06%) 3.24s 3.44s
Total Time 11.85s (± 0.68%) 11.82s (± 0.57%) -0.04s (- 0.31%) 11.63s 11.95s
TFS - node (v8.9.0, x64)
Memory used 310,265k (± 0.02%) 310,249k (± 0.01%) -16k (- 0.01%) 310,128k 310,327k
Parse Time 1.57s (± 0.49%) 1.57s (± 0.49%) -0.01s (- 0.38%) 1.55s 1.58s
Bind Time 0.68s (± 0.54%) 0.68s (± 0.53%) -0.00s (- 0.15%) 0.68s 0.69s
Check Time 5.35s (± 0.49%) 5.33s (± 0.60%) -0.02s (- 0.37%) 5.27s 5.43s
Emit Time 2.98s (± 0.53%) 2.97s (± 1.55%) -0.01s (- 0.30%) 2.89s 3.12s
Total Time 10.59s (± 0.34%) 10.55s (± 0.53%) -0.04s (- 0.39%) 10.46s 10.70s
material-ui - node (v8.9.0, x64)
Memory used 496,221k (± 0.02%) 496,190k (± 0.02%) -32k (- 0.01%) 496,028k 496,374k
Parse Time 2.49s (± 0.52%) 2.49s (± 0.35%) -0.00s (- 0.08%) 2.47s 2.51s
Bind Time 0.82s (± 0.99%) 0.81s (± 1.61%) -0.01s (- 1.10%) 0.78s 0.84s
Check Time 17.99s (± 1.03%) 17.93s (± 1.19%) -0.07s (- 0.38%) 17.48s 18.36s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.31s (± 0.91%) 21.23s (± 0.94%) -0.08s (- 0.37%) 20.81s 21.65s
Angular - node (v8.9.0, x86)
Memory used 202,046k (± 0.01%) 202,098k (± 0.02%) +52k (+ 0.03%) 201,974k 202,199k
Parse Time 2.54s (± 0.50%) 2.54s (± 0.76%) +0.00s (+ 0.04%) 2.51s 2.59s
Bind Time 1.01s (± 0.59%) 1.02s (± 0.95%) +0.01s (+ 0.49%) 1.00s 1.04s
Check Time 5.08s (± 0.68%) 5.04s (± 0.52%) -0.03s (- 0.63%) 4.98s 5.09s
Emit Time 6.12s (± 1.11%) 6.09s (± 0.52%) -0.03s (- 0.44%) 6.01s 6.17s
Total Time 14.75s (± 0.57%) 14.70s (± 0.35%) -0.05s (- 0.34%) 14.60s 14.81s
Monaco - node (v8.9.0, x86)
Memory used 202,968k (± 0.01%) 202,939k (± 0.03%) -29k (- 0.01%) 202,775k 203,048k
Parse Time 1.96s (± 0.42%) 1.97s (± 0.68%) +0.00s (+ 0.10%) 1.94s 1.99s
Bind Time 0.72s (± 0.77%) 0.71s (± 0.83%) -0.00s (- 0.56%) 0.70s 0.73s
Check Time 5.75s (± 1.42%) 5.72s (± 1.63%) -0.03s (- 0.50%) 5.46s 5.86s
Emit Time 2.77s (± 2.22%) 2.81s (± 3.32%) +0.04s (+ 1.41%) 2.71s 3.06s
Total Time 11.20s (± 0.37%) 11.21s (± 0.25%) +0.01s (+ 0.09%) 11.15s 11.27s
TFS - node (v8.9.0, x86)
Memory used 177,412k (± 0.02%) 177,352k (± 0.01%) -60k (- 0.03%) 177,319k 177,414k
Parse Time 1.61s (± 1.05%) 1.61s (± 0.55%) -0.00s (- 0.06%) 1.59s 1.63s
Bind Time 0.65s (± 1.19%) 0.65s (± 0.56%) -0.01s (- 0.77%) 0.64s 0.65s
Check Time 4.87s (± 0.59%) 4.85s (± 0.57%) -0.02s (- 0.33%) 4.78s 4.92s
Emit Time 2.82s (± 1.15%) 2.82s (± 0.66%) -0.00s (- 0.04%) 2.76s 2.85s
Total Time 9.95s (± 0.54%) 9.92s (± 0.42%) -0.02s (- 0.24%) 9.84s 9.99s
material-ui - node (v8.9.0, x86)
Memory used 279,261k (± 0.01%) 279,281k (± 0.01%) +20k (+ 0.01%) 279,228k 279,349k
Parse Time 2.55s (± 0.66%) 2.55s (± 0.75%) +0.00s (+ 0.04%) 2.52s 2.61s
Bind Time 0.72s (± 4.79%) 0.72s (± 4.81%) -0.00s (- 0.00%) 0.69s 0.85s
Check Time 16.40s (± 0.70%) 16.48s (± 0.66%) +0.07s (+ 0.44%) 16.23s 16.76s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.68s (± 0.61%) 19.75s (± 0.51%) +0.07s (+ 0.38%) 19.54s 20.00s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 41075 10
Baseline master 10

@weswigham weswigham merged commit 6acce0c into microsoft:master Oct 28, 2020
@uhyo uhyo deleted the fix-36958 branch October 28, 2020 12:14
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

@ahejlsberg ahejlsberg ahejlsberg approved these changes

Assignees

@weswigham weswigham

Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

non-null assertions break CFA
5 participants
@uhyo @weswigham @typescript-bot @sandersn @ahejlsberg